bpo-34172: multiprocessing.Pool leaks resources after being deleted by tzickel · Pull Request #8450 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation18 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Due to a circular reference inside the Pool implementation, there
was a resource leak if the object was not closed/terminated.
This patch removes the circular reference, and now the code acts
like the documentation.
https://bugs.python.org/issue34172
Maybe we should rename the Process method inside Pool to be _Process, since it doesn't have a usage outside (and I had to change it's definition to remove the circular reference) ?
@pitrou any chance to get this in the next python 2.7 / 3.7 releases ?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in looking at this. I posted a couple comments below, seems mostly good.
Due to a circular reference inside the Pool implementation, there was a resource leak if the object was not closed/terminated. This patch removes the circular reference, and now the code acts like the documentation.
Guess this won't make it to 3.7.1
Wow, I'm really sorry about that. I'm not sure about the rules post-rc but it probably will have to wait for 3.7.2 indeed :-/
Thanks @tzickel for the PR, and @pitrou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
Fix a reference issue inside multiprocessing.Pool that caused the pool to remain alive if it was deleted without being closed or terminated explicitly. (cherry picked from commit 97bfe8d)
Co-authored-by: tzickel tzickel@users.noreply.github.com
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
Fix a reference issue inside multiprocessing.Pool that caused the pool to remain alive if it was deleted without being closed or terminated explicitly. (cherry picked from commit 97bfe8d)
Co-authored-by: tzickel tzickel@users.noreply.github.com
What about the 2.7 back port ? I should manually do another PR for it ?
If you care about 2.7, that will probably be needed. I could push the "2.7 backport button" but the backport would almost certainly fail due to conflicts.
But I wouldn't bother with 2.7 if I were you.
pitrou pushed a commit that referenced this pull request
Fix a reference issue inside multiprocessing.Pool that caused the pool to remain alive if it was deleted without being closed or terminated explicitly. (cherry picked from commit 97bfe8d)
Co-authored-by: tzickel tzickel@users.noreply.github.com
pitrou pushed a commit that referenced this pull request
Fix a reference issue inside multiprocessing.Pool that caused the pool to remain alive if it was deleted without being closed or terminated explicitly. (cherry picked from commit 97bfe8d)
Co-authored-by: tzickel tzickel@users.noreply.github.com
tzickel added a commit to tzickel/cpython that referenced this pull request
If you care about 2.7, that will probably be needed. I could push the "2.7 backport button" but the backport would almost certainly fail due to conflicts.
But I wouldn't bother with 2.7 if I were you.
2.7 is still supported, and this an bug fix, not something new (also this bug was detected on code still written in python 2...). PR #9686 is a manual backport.
vstinner added a commit that referenced this pull request
vstinner added a commit that referenced this pull request
vstinner added a commit that referenced this pull request
pablogsal added a commit that referenced this pull request
Changes in this commit:
- Use a strong reference between the Pool and associated iterators
- Rework PR #8450 to eliminate a cycle in the Pool.
There is no test in this commit because any test that automatically tests this behaviour needs to eliminate the pool before joining the pool to check that the pool object is garbaged collected/does not hang. But doing this will potentially leak threads and processes (see https://bugs.python.org/issue35413).
