Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spurious failures in test_collections in releak hunting mode after typing is imported #73824

Closed
ilevkivskyi opened this issue Feb 23, 2017 · 13 comments
Assignees
Labels
3.7 (EOL) end of life performance Performance or resource usage tests Tests in the Lib/test dir

Comments

@ilevkivskyi
Copy link
Member

BPO 29638
Nosy @gvanrossum, @rhettinger, @vstinner, @serhiy-storchaka, @ilevkivskyi
PRs
  • bpo-29638: Fix spurious refleaks after typing is imported #469
  • [3.5] bpo-29638: Fix spurious refleaks after typing is imported #482
  • [3.6] bpo-29638: Fix spurious refleaks after typing is imported #483
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ilevkivskyi'
    closed_at = <Date 2017-03-18.05:38:56.479>
    created_at = <Date 2017-02-23.23:47:47.939>
    labels = ['3.7', 'tests', 'performance']
    title = 'Spurious failures in test_collections in releak hunting mode after typing is imported'
    updated_at = <Date 2017-03-24.22:53:30.573>
    user = 'https://github.com/ilevkivskyi'

    bugs.python.org fields:

    activity = <Date 2017-03-24.22:53:30.573>
    actor = 'serhiy.storchaka'
    assignee = 'levkivskyi'
    closed = True
    closed_date = <Date 2017-03-18.05:38:56.479>
    closer = 'serhiy.storchaka'
    components = ['Tests']
    creation = <Date 2017-02-23.23:47:47.939>
    creator = 'levkivskyi'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29638
    keywords = []
    message_count = 13.0
    messages = ['288495', '289014', '289015', '289016', '289017', '289018', '289019', '289025', '289032', '289036', '289037', '290295', '290296']
    nosy_count = 6.0
    nosy_names = ['gvanrossum', 'rhettinger', 'vstinner', 'stutzbach', 'serhiy.storchaka', 'levkivskyi']
    pr_nums = ['469', '482', '483']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue29638'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @ilevkivskyi
    Copy link
    Member Author

    This command:

    ./python -c 'import runpy, typing; runpy.run_module("test")' -R 5:5 test_collections

    Sometimes gives spurious failures like this:

    test_collections leaked [0, 0, 3, -3, 3] memory blocks, sum=3

    I think this is because ABC caches of typing.ChainMap, typing.Counter, and typing.DefaultDict are not cleared by refleak.py/dash_R_cleanup (presumably because inspect.isabstract returns False on those)

    Adding a manual clean-up of these cashes to cleanup_cashes() fixes the "leak".

    @ilevkivskyi ilevkivskyi self-assigned this Feb 23, 2017
    @ilevkivskyi ilevkivskyi added tests Tests in the Lib/test dir performance Performance or resource usage labels Feb 23, 2017
    @serhiy-storchaka
    Copy link
    Member

    If there are problems with ABC caches of typing.ChainMap, typing.Counter, and typing.DefaultDict, why not add corresponding clearing methods obj._abc_cache.clear and obj._abc_negative_cache.clear to typing._cleanups ?

    Why there are problems only with ABC caches of these three types?

    @ilevkivskyi
    Copy link
    Member Author

    why not add corresponding clearing methods obj._abc_cache.clear and obj._abc_negative_cache.clear to typing._cleanups ?

    OK, this is another possible solution. I didn't think about this, because now typing._cleanups only clear generic caches (not ABC caches).

    Why there are problems only with ABC caches of these three types?

    Normally, ABC caches are cleared in dash_R_cleanup, but I think that the problem is that inspect.isabstract returns False for these three types (unlike other types in typing module), see line 147 in refleak.py:

            if not isabstract(abc):
                continue

    Actually now I think that the code in dash_R_cleanup might not clear some other caches also. So maybe we just need to add all ABC caches to typing._cleanups (just in case). What do you think, Serhiy?

    @serhiy-storchaka
    Copy link
    Member

    So maybe we just need to add all ABC caches to typing._cleanups (just in case).

    I don't know. Sorry, I didn't remember exactly reasons why I didn't move the clearing of all ABC caches into clear_caches(). Perhaps I suspected that this can have unwanted consequences or decrease performance too much.

    Are typing.ChainMap and others actually abstract classes? If yes, then perhaps there is something wrong with inspect.isabstract(). If no, then why ABC caches for these classes exist and non-empty?

    @ilevkivskyi
    Copy link
    Member Author

    Are typing.ChainMap and others actually abstract classes?

    They are abstract classes in the sense that they are instances of abc.ABCMeta. However, for some reasons inspect checks __flags__ attribute. The latter probably reflects the fact that Deque etc. do not have abstract methods (they still need to be instances of abc.ABCMeta because they need to be generic).

    I don't think that we need to change behaviour of inspect because it could be backward incompatible. I would either make changes to refleak or typing (adding few more cleanups in either place).

    @serhiy-storchaka
    Copy link
    Member

    What if explicitly set __abstractmethods__ = True for these types?

    @ilevkivskyi
    Copy link
    Member Author

    What if explicitly set __abstractmethods__ = True for these types?

    Unfortunately this does not help. I think this is because dash_R_cleanup only clears caches for classes in collections.abc.__all__ and their immediate .__subclasses__(). Making this recursive (i.e. also clearing caches of __subclasses__() of __subclasses__() etc) will probably fix the problem (provided we also add __abstractmethods__ = True). However, this looks a bit too complex for me. I would rather add few more typing._cleanups (anyway this failure only happens with typing ABCs)

    @serhiy-storchaka
    Copy link
    Member

    I just wonder is there any other negative consequences? inspect.isabstract() returning False for abstract class looks a bug to me.

    If you just want to add a workaround in dash_R_cleanup, I think it would be better to generate the list of all abstract classes and add three typing classes to it.

    clear_caches() is called before running every test, but ABC caches are cleared not so often.

    @ilevkivskyi
    Copy link
    Member Author

    If you just want to add a workaround in dash_R_cleanup, I think it would be better to generate the list of all abstract classes and add three typing classes to it.

    Yes, I just updated the PR.

    I have found something else unrelated to typing, this test (in exactly this order):

    ./python -m test -R 5:5 test_abc test_functools

    fails with a reproducible pattern:

    test_functools leaked [0, 3, 1, 0, 0] memory blocks, sum=4

    If you are happy with the PR now, we could open two separate issues for possible bug in inspect.isabstract and refleak in test_functools after test_abc. (the latter however might be potentially somehow related to abc caches)

    @serhiy-storchaka
    Copy link
    Member

    Thank you Ivan! Seems this has fixed bpo-25744. Don't you mind to backport your changes to 3.6 and 3.5?

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Mar 5, 2017
    @ilevkivskyi
    Copy link
    Member Author

    Seems this has fixed bpo-25744.

    This is interesting, if I remember correctly the relevant typing classes were added only recently. I will take a look at how to back-port this (probably this will require some code changes)

    @serhiy-storchaka
    Copy link
    Member

    New changeset b414e34 by Serhiy Storchaka (Ivan Levkivskyi) in branch '3.6':
    bpo-29638: Fix spurious refleaks after typing is imported (#469) (#483)
    b414e34

    @serhiy-storchaka
    Copy link
    Member

    New changeset 7acffa2 by Serhiy Storchaka (Ivan Levkivskyi) in branch 'master':
    bpo-29638: Fix spurious refleaks after typing is imported (#469)
    7acffa2

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life performance Performance or resource usage tests Tests in the Lib/test dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants