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
Comments
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". |
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? |
OK, this is another possible solution. I didn't think about this, because now typing._cleanups only clear generic caches (not ABC caches).
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? |
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? |
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). |
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) |
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. |
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) |
Thank you Ivan! Seems this has fixed bpo-25744. Don't you mind to backport your changes to 3.6 and 3.5? |
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) |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: