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

test_import.test_concurrency leaks ref on Windows #106176

Closed
sunmy2019 opened this issue Jun 28, 2023 · 13 comments
Closed

test_import.test_concurrency leaks ref on Windows #106176

sunmy2019 opened this issue Jun 28, 2023 · 13 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows release-blocker topic-importlib type-bug An unexpected behavior, bug, or error

Comments

@sunmy2019
Copy link
Member

sunmy2019 commented Jun 28, 2023

On Python 3.12, test_import.test_concurrency() leaks on Windows:

vstinner@WIN C:\victor\python\3.12>python -m test test_import -m test_concurrency -R 3:3
(...)
test_import leaked [22, 36, 28] references, sum=86

bisected to #94504

Potentially Related: #104702

See #102251 (comment) #104796 (comment)

Linked PRs

@sunmy2019 sunmy2019 added type-bug An unexpected behavior, bug, or error OS-windows labels Jun 28, 2023
@vstinner
Copy link
Member

This leak can be seen on the "AMD64 Windows11 Refleaks 3.12" buildbot for example: https://buildbot.python.org/all/#/builders/1103/builds/36

I reproduced it locally.

@vstinner
Copy link
Member

cc @exarkun @brettcannon

@Eclips4
Copy link
Member

Eclips4 commented Jun 28, 2023

On Python 3.12, test_import.test_concurrency() leaks on Windows:

Did you mean that this test leaks only on 3.12?
I can still reproduce it on current main branch (3.13).

More info
./python -m test test_import -m test_concurrency -R 3:3
Running debug|x64 interpreter...
0:00:00 Run tests sequentially
0:00:00 [1/1] test_import
beginning 6 repetitions
123456
......
test_import leaked [40, 40, 40] references, sum=120
test_import leaked [40, 40, 40] memory blocks, sum=120
test_import failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_import

Total duration: 7.0 sec
Tests result: FAILURE

@sunmy2019
Copy link
Member Author

Did you mean that this test leaks only on 3.12?

No. It also affects 3.12

@AlexWaygood AlexWaygood added 3.12 bugs and security fixes 3.13 bugs and security fixes labels Jun 28, 2023
@exarkun
Copy link
Contributor

exarkun commented Jun 28, 2023

I'm not familiar with the leak checking system and I'm not likely to have much time to learn about it in the foreseeable future.

@brettcannon
Copy link
Member

@exarkun it just means some things are not getting cleaned up.

@exarkun
Copy link
Contributor

exarkun commented Jun 29, 2023

Just trying to signal my likelihood of pitching on here: low.

@vstinner
Copy link
Member

Oops, I just reported a duplicate of this bug: #107086 (closed).

I also found the same commit with git bisect: commit 3325f05

commit 3325f054e33b318aa56b74472f76a56b8afc0510
Author: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date:   Fri Jan 20 19:00:39 2023 -0500

    gh-91351: Fix some bugs in importlib handling of re-entrant imports (GH-94504)

    Co-authored-by: Brett Cannon <brett@python.org>

@vstinner
Copy link
Member

A fix was proposed: PR #106207.

@erlend-aasland
Copy link
Contributor

@Yhg1s: IMO, this is also a release blocker (in my opinion, any ref leak is a release blocker).

@Yhg1s
Copy link
Member

Yhg1s commented Aug 1, 2023

(Yes, this looks like a real leak, not an artifact of how the refleak checker reruns things, so it should be a release blocker.)

@brettcannon
Copy link
Member

We could revert the fix that leads to the leak to unblock the release as I'm not sure if anyone has come up with a good solution to avoid the memory leak and the bugs that get fixed are not fundamental to import (although it would still be nice to have the bugs fixed).

@brettcannon
Copy link
Member

I created #108497 to try out an idea that @Eclips4 had for using weakrefs.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 29, 2023
…cross multiple threads (pythonGH-108497)

(cherry picked from commit 5f85b44)

Co-authored-by: Brett Cannon <brett@python.org>
Yhg1s pushed a commit that referenced this issue Aug 29, 2023
… multiple threads (GH-108497) (#108612)

GH-106176, GH-104702: Fix reference leak when importing across multiple threads (GH-108497)
(cherry picked from commit 5f85b44)

Co-authored-by: Brett Cannon <brett@python.org>
@Eclips4 Eclips4 closed this as completed Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows release-blocker topic-importlib type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

8 participants