[issue12167] test_packaging reference leak

Changes by Éric Araujo : -- resolution: -> fixed stage: needs patch -> committed/rejected status: open -> closed ___ Python tracker ___ _

[issue12167] test_packaging reference leak

Roundup Robot added the comment: New changeset e76c6aaff135 by Éric Araujo in branch 'default': Add regrtest check for caches in packaging.database (see #12167) http://hg.python.org/cpython/rev/e76c6aaff135 -- ___ Python tracker

[issue12167] test_packaging reference leak

Éric Araujo added the comment: I edited my patch to use a copy instead of an explicit empty dict, but I found a bug. The restore code unpacks the saved_caches object to (cache, items), but saved_caches is (id(cache), cache, cache.copy()). I’m surprised the unpacking works; I don’t want to c

[issue12167] test_packaging reference leak

Éric Araujo added the comment: > I would call .copy() on the original dicts rather than remembering an > explicit empty dict. I thought about that and decided to use an empty dict as a way to add a check that the caches should start empty. Maybe it was misguided and I should instead add a un

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: > I’ve also made a patch to register the caches used by > packaging.database with the regrtest unclean environment warning; can > someone review it? I would call .copy() on the original dicts rather than remembering an explicit empty dict. --

[issue12167] test_packaging reference leak

Éric Araujo added the comment: It looks like we don’t have refleaks anymore! I still have one question. Victor reported some time ago that packaging.util._path_created was a cache that was never cleared; I fixed that in 27a70dfc38cc, but recently I’ve found that regrtest itself clears the si

[issue12167] test_packaging reference leak

Vinay Sajip added the comment: I've created #12536 to cover the lib2to3 issue. -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue12167] test_packaging reference leak

Éric Araujo added the comment: Let me clarify my position: I think there is a bug in lib2to3 that should be fixed, after what the refleak would go. (I’ll report it soon if nobody does it before.) If Benjamin rejects the bug, then I’ll agree with the monkey-patch. -- assignee: tarek

[issue12167] test_packaging reference leak

Vinay Sajip added the comment: I can confirm what Andreas said - I just removed the two lines referencing the logger in BaseFix, and ran the regression tests - with no failures. So I, too, think those lines should go. -- nosy: +vinay.sajip ___ Pyth

[issue12167] test_packaging reference leak

Éric Araujo added the comment: Thanks for clarifying, I had misread. IMO, using one logger per file is a lib2to3 bug. -- ___ Python tracker ___ ___

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: > See http://hg.python.org/cpython/file/tip/Lib/packaging/tests/support.py#l81 AFAIU, the problem is that 2to3 creates a whole new logger for each different file. So it's not about cleaning the handlers, but cleaning up the loggers as well. And logging (deliber

[issue12167] test_packaging reference leak

Éric Araujo added the comment: See http://hg.python.org/cpython/file/tip/Lib/packaging/tests/support.py#l81 -- ___ Python tracker ___ ___

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: > To fix it, I don’t find the monkey-patching approach very good, I’d > prefer properly using the logging API to remove handlers upon cleanup. I don't think such stuff exists. -- ___ Python tracker

[issue12167] test_packaging reference leak

Éric Araujo added the comment: Thanks a lot for the diagnosis. To fix it, I don’t find the monkey-patching approach very good, I’d prefer properly using the logging API to remove handlers upon cleanup. Would you like to look into that? -- ___ Pyt

[issue12167] test_packaging reference leak

Andreas Stührk added the comment: Attached is a patch that replaces `lib2to3.fixer_Base.BaseFix.set_filename()` during tests. With the patch applied, I don't get any refleaks for packaging. Another approach would be to simply remove the logging attribute of lib2to3 fixers, as it isn't used an

[issue12167] test_packaging reference leak

Andreas Stührk added the comment: At least some of the remaining refleaks are caused by lib2to3. lib2to3 uses a logger with the filename as logger name (see `lib2to3.fixer_base.BaseFix.set_filename()`), but as the tests use a temporary file with an arbitrary name, a new logger is created on e

[issue12167] test_packaging reference leak

Éric Araujo added the comment: > I changed test test to use a subprocess: Yes, in packaging. I replied to an earlier question about distutils: >>> I could not find any test in distutils/tests that imports extension modules. >> test_build_ext builds and imports xx. -- ___

[issue12167] test_packaging reference leak

STINNER Victor added the comment: > test_build_ext builds and imports xx. I changed test test to use a subprocess: New changeset 144cea8db9a5 by Victor Stinner in branch 'default': Issue #12333: run tests on the new module in a subprocess http://hg.python.org/cpython/rev/144cea8db9a5 It shoul

[issue12167] test_packaging reference leak

R. David Murray added the comment: It means that objects were garbage collected. The refleak test runs the test multiple times, and ignores the first N runs to allow the object count to "settle". But sometimes it either doesn't settle, or later runs end up with objects getting garbage colle

[issue12167] test_packaging reference leak

Éric Araujo added the comment: We’re down to 100 refleaks. Thanks to the negative refleaks of test_pydoc, we now have a few refleaks to spare! Seriously, what does a negative refleak mean? -- ___ Python tracker

[issue12167] test_packaging reference leak

Éric Araujo added the comment: > I could not find any test in distutils/tests that imports extension modules. test_build_ext builds and imports xx. -- ___ Python tracker ___ __

[issue12167] test_packaging reference leak

Roundup Robot added the comment: New changeset 27a70dfc38cc by Éric Araujo in branch 'default': Packaging tests: don’t let an internal cache grow indefinitely. http://hg.python.org/cpython/rev/27a70dfc38cc -- ___ Python tracker

[issue12167] test_packaging reference leak

STINNER Victor added the comment: Note: #12133 has a patch to fix the ResourceWarning of test_pypi_simple. -- ___ Python tracker ___

[issue12167] test_packaging reference leak

STINNER Victor added the comment: test_dist and test_bdist_dumb leak because of the _path_created global variable of packaging.util, used indirectly by copy_tree(). # cache for by mkpath() -- in addition to cheapening redundant calls, # eliminates redundant "creating /foo/bar/baz" messages in

[issue12167] test_packaging reference leak

Roundup Robot added the comment: New changeset fd6446a88fe3 by Victor Stinner in branch 'default': Issue #12167: Fix a reafleak in packaging.tests.PyPIServer constructor http://hg.python.org/cpython/rev/fd6446a88fe3 -- ___ Python tracker

[issue12167] test_packaging reference leak

Éric Araujo added the comment: Ah, it’s just refcounting issues? I can probably use the gc module to find them, and add del statements where appropriate to release objects. -- ___ Python tracker

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: > I’m afraid I don’t understand enough to fix this. I looked at > test_simple_built in test_command_bdist_dumb and found no C code > involved. It means that either packaging or test_packaging keeps references to more and more objects. You don't need to know C,

[issue12167] test_packaging reference leak

Éric Araujo added the comment: I’m afraid I don’t understand enough to fix this. I looked at test_simple_built in test_command_bdist_dumb and found no C code involved. -- ___ Python tracker _

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: Is someone investigating this? -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscr

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: In test_pypi_dist, the leak is shared between test_download() and test_unpack(). -- ___ Python tracker ___

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: In test_mixin2to3.py, the leak is shared between the 3 test methods. -- ___ Python tracker ___ ___

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: In test_command_bdist, the leak is in test_simple_built(). -- ___ Python tracker ___ ___ Python-bug

[issue12167] test_packaging reference leak

Roundup Robot added the comment: New changeset 70675864717b by Victor Stinner in branch 'default': Issue #12167: packaging.tests.support, LoggingCatcher restores correctly the http://hg.python.org/cpython/rev/70675864717b New changeset 28c1f8480090 by Victor Stinner in branch 'default': Issue #

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: Let's see: -> test_command_bdist_dumb.py leaks: test_packaging leaked [7, 7] references, sum=14 -> test_dist.py leaks: test_packaging leaked [65, 65] references, sum=130 -> test_mixin2to3.py leaks: test_packaging leaked [60, 60] references, sum=120 -> test_p

[issue12167] test_packaging reference leak

STINNER Victor added the comment: DistributionTestCase.test_hooks_get_run() leaks some references, I'm trying to understand where exactly. -- nosy: +haypo ___ Python tracker __

[issue12167] test_packaging reference leak

Amaury Forgeot d'Arc added the comment: I could not find any test in distutils/tests that imports extension modules. -- ___ Python tracker ___ __

[issue12167] test_packaging reference leak

Antoine Pitrou added the comment: > Probably because new extension modules are built and imported on every run. Well, test_distutils does the same, doesn't it? -- ___ Python tracker __

[issue12167] test_packaging reference leak

Amaury Forgeot d'Arc added the comment: Probably because new extension modules are built and imported on every run. -- nosy: +amaury.forgeotdarc ___ Python tracker ___ _

[issue12167] test_packaging reference leak

Changes by Nadeem Vawda : -- nosy: +nadeem.vawda ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.py

[issue12167] test_packaging reference leak

New submission from Antoine Pitrou : Looks like either packaging or test_packaging forgets to clean up after itself: results for 9a16fa0c9548 on branch "default" test_packaging leaked [193, 193, 193] references, sum=579 -- assignee: tarek co