Skip to content

bpo-34172: multiprocessing.Pool leaks resources after being deleted #8450

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

Merged
merged 2 commits into from
Oct 2, 2018

Conversation

tzickel
Copy link
Contributor

@tzickel tzickel commented Jul 24, 2018

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

@tzickel
Copy link
Contributor Author

tzickel commented Jul 28, 2018

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) ?

@tzickel
Copy link
Contributor Author

tzickel commented Sep 12, 2018

@pitrou any chance to get this in the next python 2.7 / 3.7 releases ?

Copy link
Member

@pitrou pitrou left a comment

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.
@tzickel
Copy link
Contributor Author

tzickel commented Sep 14, 2018

Done.

@tzickel
Copy link
Contributor Author

tzickel commented Sep 29, 2018

Guess this won't make it to 3.7.1

@pitrou
Copy link
Member

pitrou commented Oct 2, 2018

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 :-/

@miss-islington
Copy link
Contributor

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 Oct 2, 2018
…ythonGH-8450)

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 <[email protected]>
@bedevere-bot
Copy link

GH-9676 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2018
…ythonGH-8450)

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 <[email protected]>
@bedevere-bot
Copy link

GH-9677 is a backport of this pull request to the 3.6 branch.

@tzickel
Copy link
Contributor Author

tzickel commented Oct 2, 2018 via email

@pitrou
Copy link
Member

pitrou commented Oct 2, 2018

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 Oct 2, 2018
…H-8450) (GH-9676)

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 <[email protected]>
pitrou pushed a commit that referenced this pull request Oct 2, 2018
…H-8450) (GH-9677)

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 <[email protected]>
tzickel added a commit to tzickel/cpython that referenced this pull request Oct 3, 2018
@tzickel
Copy link
Contributor Author

tzickel commented Oct 3, 2018

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 Dec 6, 2018
vstinner added a commit that referenced this pull request Dec 6, 2018
vstinner added a commit that referenced this pull request Dec 6, 2018
pablogsal added a commit that referenced this pull request Feb 11, 2019
Changes in this commit:

1. Use a _strong_ reference between the Pool and associated iterators
2. 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants