msg288495 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-02-23 23:47 |
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".
|
msg289014 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-05 15:11 |
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?
|
msg289015 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-03-05 15:21 |
> 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?
|
msg289016 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-05 15:45 |
> 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?
|
msg289017 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-03-05 16:03 |
> 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).
|
msg289018 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-05 16:12 |
What if explicitly set __abstractmethods__ = True for these types?
|
msg289019 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-03-05 16:30 |
> 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)
|
msg289025 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-05 16:57 |
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.
|
msg289032 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-03-05 17:56 |
> 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)
|
msg289036 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-05 18:18 |
Thank you Ivan! Seems this has fixed issue25744. Don't you mind to backport your changes to 3.6 and 3.5?
|
msg289037 - (view) |
Author: Ivan Levkivskyi (levkivskyi) * |
Date: 2017-03-05 18:23 |
> Seems this has fixed issue25744.
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)
|
msg290295 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-24 22:53 |
New changeset b414e349eb6a6140a81ccec249bae3abe939e836 by Serhiy Storchaka (Ivan Levkivskyi) in branch '3.6':
bpo-29638: Fix spurious refleaks after typing is imported (#469) (#483)
https://github.com/python/cpython/commit/b414e349eb6a6140a81ccec249bae3abe939e836
|
msg290296 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-24 22:53 |
New changeset 7acffa23c9e7866e5b4b98b8f3c28b14ec1c688c by Serhiy Storchaka (Ivan Levkivskyi) in branch 'master':
bpo-29638: Fix spurious refleaks after typing is imported (#469)
https://github.com/python/cpython/commit/7acffa23c9e7866e5b4b98b8f3c28b14ec1c688c
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:43 | admin | set | github: 73824 |
2017-03-24 22:53:30 | serhiy.storchaka | set | messages:
+ msg290296 |
2017-03-24 22:53:17 | serhiy.storchaka | set | messages:
+ msg290295 |
2017-03-18 05:38:56 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: resolved |
2017-03-05 19:15:41 | levkivskyi | set | pull_requests:
+ pull_request395 |
2017-03-05 19:00:10 | levkivskyi | set | pull_requests:
+ pull_request394 |
2017-03-05 18:23:54 | levkivskyi | set | messages:
+ msg289037 |
2017-03-05 18:18:15 | serhiy.storchaka | set | messages:
+ msg289036 versions:
+ Python 3.5, Python 3.6, Python 3.7 |
2017-03-05 18:09:38 | serhiy.storchaka | link | issue25744 superseder |
2017-03-05 17:56:02 | levkivskyi | set | messages:
+ msg289032 |
2017-03-05 16:57:39 | serhiy.storchaka | set | messages:
+ msg289025 |
2017-03-05 16:30:42 | levkivskyi | set | messages:
+ msg289019 |
2017-03-05 16:12:59 | serhiy.storchaka | set | messages:
+ msg289018 |
2017-03-05 16:03:27 | levkivskyi | set | messages:
+ msg289017 |
2017-03-05 15:45:50 | serhiy.storchaka | set | nosy:
+ rhettinger, vstinner, stutzbach messages:
+ msg289016
|
2017-03-05 15:21:32 | levkivskyi | set | messages:
+ msg289015 |
2017-03-05 15:11:12 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg289014
|
2017-03-04 23:38:18 | levkivskyi | set | pull_requests:
+ pull_request387 |
2017-02-23 23:47:47 | levkivskyi | create | |