[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: I think tearDown() is not necessary. But on the other hand it could be nice to have a method in tests to force cache clear. I would propose it not as a default, but as an opt-in to check that cache works well, or that a certain tested feature is "cache indepe

[issue28649] refleak in typing.py

Guido van Rossum added the comment: BTW, do we still want the tearDown()? Or is that no longer necessary? -- ___ Python tracker ___ __

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Thank you Guido and Ivan! > Yury, when you're happy with the situation can you close this issue? I'll close it now, will reopen if something comes up. -- resolution: -> fixed status: open -> closed ___ Python track

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Thanks a bundle, Ivan! I've merged those two into CPython 3.5, 3.6, 3.7. We can now watch the build bots for a while and double-check the refleaks. Yury, when you're happy with the situation can you close this issue? -- _

[issue28649] refleak in typing.py

Roundup Robot added the comment: New changeset 555f0ca31587 by Guido van Rossum in branch '3.5': Issue #28649: fix second issue with _ForwardRef (#328) https://hg.python.org/cpython/rev/555f0ca31587 New changeset 0f863906cf2e by Guido van Rossum in branch '3.6': Issue #28649: fix second issue wi

[issue28649] refleak in typing.py

Roundup Robot added the comment: New changeset 0da2e381ad71 by Guido van Rossum in branch '3.5': Issue #28649: fix first issue with _ForwardRef (#327) https://hg.python.org/cpython/rev/0da2e381ad71 New changeset 249a1f0b2857 by Guido van Rossum in branch '3.6': Issue #28649: fix first issue with

[issue28649] refleak in typing.py

Guido van Rossum added the comment: We discussed cache size previously and 128 is fine. --Guido (mobile) -- ___ Python tracker ___ __

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Good work, Ivan! > Those two seem to fix everything, I tried various cache sizes with both C and > Python versions of lru_cache and found no refleaks. Speaking of lru_cache: typing currently doesn't configure maxsize, which means that the cache is limited to

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: OK, here are the PRs: https://github.com/python/typing/pull/327 https://github.com/python/typing/pull/328 Those two seem to fix everything, I tried various cache sizes with both C and Python versions of lru_cache and found no refleaks. -- __

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: > BTW, if I set maxsize=10 for typing lru_cache, test_generic_forward_ref > crashes in refleak-test mode. Maybe this is another bug... This one is also related to the mentioned in my previous message. Namely, forward references are only evaluated once. Ho

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: Guido, Yury, it looks like I solved the puzzle. All the remaining problems are because of forward references. In particular, _ForwardRef keeps a reference to the frame where it was defined: typing_globals = globals() frame = sys._getframe(1)

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: I think Yury is right here. The good plan would be to add the tearDown(), but also fix the problem with test_generic_forward_ref. I could work on it tomorrow (really don't have time today, sorry). -- ___ Python tra

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: It looks like the tearDown() works really well. It also somehow fixes the problem with test_generic_forward_ref -- ___ Python tracker ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > Segfault or traceback? Traceback. I should have said failed, not crashed :( > I'm not sure a crash the Python version of lru_cache with maxsize=10 is high on my list of worries. The problem is that the test fails depending on cache size, and caching code

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: > * It looks like that some types in the cache are being *reused* by different > tests. I also noticed this some time ago, ideally we need to make all tests more isolated between each other. > Now this all of this is just my guess of what's going on here.

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Segfault or traceback? Anyway, it implements a doubly-linked list where each node is implemented as a list of 4 items... Maybe there's something in the GC code that recurses down at the C level??? I'm not sure a crash the Python version of lru_cache with maxsi

[issue28649] refleak in typing.py

Yury Selivanov added the comment: >> But why is this so different from the C implementation of lru_cache? > I don't know. Maybe C version of lru cache creates a bit less pressure on GC. Actually test/refleak.py now cleans up typing's lru cache & calls "gc.collect()". It seems that the typing

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > But why is this so different from the C implementation of lru_cache? I don't know. Maybe C version of lru cache creates a bit less pressure on GC. BTW, if I set maxsize=10 for typing lru_cache, test_generic_forward_ref crashes in refleak-test mode. Mayb

[issue28649] refleak in typing.py

Guido van Rossum added the comment: But why is this so different from the C implementation of lru_cache? -- ___ Python tracker ___ ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: So Ivan made an interesting observation: if we use Python version of functools.lru_cache, typing tests start to leak like crazy: beginning 6 repetitions 123456 .. test_typing leaked [24980, 24980, 24980] references, sum=74940 I experimented

[issue28649] refleak in typing.py

Yury Selivanov added the comment: See http://bugs.python.org/issue28653 for details. -- ___ Python tracker ___ ___ Python-bugs-list ma

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Found it, will open a separate issue. -- resolution: -> fixed stage: needs patch -> resolved status: open -> closed ___ Python tracker ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: reflesk with only functools: def test_lru_type_error(self): @functools.lru_cache(maxsize=None) def foo(o): raise TypeError with self.assertRaises(TypeError): foo([]) -- __

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: A very simple repro: with self.assertRaises(TypeError): Union[[]] It looks like the problem happens when a non-hashable argument is passed to cached function -- ___ Python tracker

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: My impression is like something wrong happens when TypeError is raised by a cached function. -- ___ Python tracker ___ ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Oh. This is a bug in C implementation of functools.lru_cache. I'll take a look. -- ___ Python tracker ___ __

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Ivan, this is the part of the test that leaks: def test_extended_generic_rules_eq(self): T = TypeVar('T') U = TypeVar('U') with self.assertRaises(TypeError): Callable[[T], U][[], int] -- _

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: Yury, yes I will take a look at this now. -- ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue28649] refleak in typing.py

Yury Selivanov added the comment: The specific test that leaks is test_extended_generic_rules_eq -- ___ Python tracker ___ ___ Python-

[issue28649] refleak in typing.py

Yury Selivanov added the comment: With all commits pushed, test_typing still seems to leak. Ivan, do you have time to see what's going on? test_typing leaked [125, 125, 125] references, sum=375 test_typing leaked [49, 49, 49] memory blocks, sum=147 --

[issue28649] refleak in typing.py

Roundup Robot added the comment: New changeset d926b484d33a by Serhiy Storchaka in branch '3.5': Issue #28649: Clear the typing module caches when search for reference leaks. https://hg.python.org/cpython/rev/d926b484d33a New changeset caf3ceb93307 by Serhiy Storchaka in branch '3.6': Issue #286

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Serhiy can you apply the non-typing.py part of typing-clear-caches.patch? The diff I applied only has the typing.py part (because I've done this upstream first). -- ___ Python tracker

[issue28649] refleak in typing.py

Roundup Robot added the comment: New changeset d920bfa5a71a by Guido van Rossum in branch '3.5': Issue #28649: typing-clear-caches.patch https://hg.python.org/cpython/rev/d920bfa5a71a New changeset bd2ec9965f47 by Guido van Rossum in branch '3.6': Issue #28649: typing-clear-caches.patch (3.5->3.

[issue28649] refleak in typing.py

Roundup Robot added the comment: New changeset d790078797bd by Guido van Rossum in branch '3.5': Issue #28649: fix-typing-test-v2.diff https://hg.python.org/cpython/rev/d790078797bd New changeset 43be7891b1f5 by Guido van Rossum in branch '3.6': Issue #28649: fix-typing-test-v2.diff (3.5->3.6) h

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: Here is the corrected patch to avoid crash. -- Added file: http://bugs.python.org/file45416/fix-typing-test-v2.diff ___ Python tracker ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Serhiy's patch typing-clear-caches.patch looks good, we should apply it (fixing the code style a little bit). -- ___ Python tracker ___ ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > Yury, _tp_cache should be the only one (of course most generic classes have > _abc_cache since GenericMeta inherits from ABCMeta) Then there is at least one more bug not related to lru_cache (or any kind of cache in typing). And it's not classes-with-empty-

[issue28649] refleak in typing.py

Guido van Rossum added the comment: > It fixes 96% of refleaks. In typing.py? Or generally? -- ___ Python tracker ___ ___ Python-bugs

[issue28649] refleak in typing.py

Guido van Rossum added the comment: FYI I've opened https://github.com/python/typing/issues/323 to track this upstream. -- ___ Python tracker ___ ___

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: Yury, _tp_cache should be the only one (of course most generic classes have _abc_cache since GenericMeta inherits from ABCMeta) -- ___ Python tracker

[issue28649] refleak in typing.py

Serhiy Storchaka added the comment: > Does that patch fix the refleak completely or only partially? It fixes 96% of refleaks. -- ___ Python tracker ___ _

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > Does that patch fix the refleak completely or only partially? I tried to empty the cache in test_typing.BaseTestCase.setUp and tearDown; this doesn't fix all refleaks. Do we have some other caches in typing.py? -- __

[issue28649] refleak in typing.py

Ivan Levkivskyi added the comment: Yury, Guido, I think I understand why test_typing crashes in refleak hunting mode. The test relies on caching, namely type variables are compared by id, so that if cache is cleared between those two lines, then test fails. I think we should just update those

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > Good sleuthing! So the bug is in _lru_cache? I don't think it's a bug of lru_cache. It stores strong references for arguments and return value of the decorated function (which btw isn't documented). I don't think it's possible to write efficient generic lru

[issue28649] refleak in typing.py

Guido van Rossum added the comment: I have no objections against typing-clear-caches.patch (though we'll have to make sure to apply it to the GitHub upstream as well -- I can take care of that once you merge it to CPython). Does that patch fix the refleak completely or only partially? I have

[issue28649] refleak in typing.py

Serhiy Storchaka added the comment: > Serhiy, why is `gc.collect()` returning 2 after `del A` proof of a leak? Sorry, it was bad example. I don't remember all details. > The problem is that functools.lru_cache is used in _tp_cache. If I remove > type caching, the original "refleak" is fixed.

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Good sleuthing! So the bug is in _lru_cache? On Wed, Nov 9, 2016 at 10:37 AM, Guido van Rossum wrote: > > Guido van Rossum added the comment: > > I could use a hint on a complete program that establishes whether there is > or is not a refleak. > > --

[issue28649] refleak in typing.py

Guido van Rossum added the comment: I could use a hint on a complete program that establishes whether there is or is not a refleak. -- ___ Python tracker ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Josh, Serhiy, I've created an issue for tracking empty slots types: #28651. This issue is about typing.py using functools.lru_cache for internal cache That is what causing the refleaks, and inability of test_typing.py to be run in refleak hunting mode. -

[issue28649] refleak in typing.py

Josh Rosenberg added the comment: Serhiy: I think you forgot to make a global instance of the type for your demonstration. You mentioned in msg253899 that the loop is: global instance -> type -> method -> module globals -> global instance The example you gave doesn't instantiate class A, and i

[issue28649] refleak in typing.py

Yury Selivanov added the comment: The problem is that functools.lru_cache is used in _tp_cache. If I remove type caching, the original "refleak" is fixed. -- ___ Python tracker ___

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Wow. In default CPython branch, with no modifications, test_typing.py simply crashes in debug mode: yury@ysmbp ~/dev/py/cpython $ ./python.exe -m test -R3:3 test_typing Run tests sequentially 0:00:00 [1/1] test_typing beginning 6 repetitions 123456 .test test_t

[issue28649] refleak in typing.py

Yury Selivanov added the comment: I removed all slots from typing.py, the refleak is still there. -- ___ Python tracker ___ ___ Python

[issue28649] refleak in typing.py

Yury Selivanov added the comment: Attached patch (slots_gc.patch) makes objects with empty slots tracked by GC: class A: __slots__ = () gc.is_tracked(A()) == False # before gc.is_tracked(A()) == True # with the patch This doesn't fix the refleak though. -- keywords: +patch

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Serhiy, why is `gc.collect()` returning 2 after `del A` proof of a leak? Yes, there's a cycle, but it's being GC'ed -- isn't that fine? Without the slots the collect() call returns a larger number. -- ___ Python tra

[issue28649] refleak in typing.py

Serhiy Storchaka added the comment: See also https://mail.python.org/pipermail/python-dev/2015-October/141993.html. -- ___ Python tracker ___

[issue28649] refleak in typing.py

Serhiy Storchaka added the comment: >>> import gc >>> gc.collect() 23 >>> gc.collect() 0 >>> class A: ... __slots__ = () ... >>> del A >>> gc.collect() 2 ^ leak -- ___ Python tracker __

[issue28649] refleak in typing.py

Guido van Rossum added the comment: Ivan, this was an early typing.py update (merged into CPython on Sept. 27). But the refleak is still with us. Do you have any intuition on what could be going on here? My own intuition here is lacking, other than thinking there's a lot of metaclass magic go

[issue28649] refleak in typing.py

Yury Selivanov added the comment: > Could it be related to leaks in objects with empty __slots__? See issue24379 > for details. Not sure... Do we have a test/patch for objects-with-empty-slots bug? -- ___ Python tracker

[issue28649] refleak in typing.py

Serhiy Storchaka added the comment: Could it be related to leaks in objects with empty __slots__? See issue24379 for details. -- ___ Python tracker ___ _

[issue28649] refleak in typing.py

New submission from Yury Selivanov: Looks like we have a refleak somewhere in the system that typing.py is exposing. The following code exposes the refleak: def test_refleak(self): T = typing.TypeVar('T') class C(typing.Generic[T], typing.Mapping[int, str]): ... The change