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.

Sign up for GitHub

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

Jump to bottom

gh-94777: Fix deadlock in ProcessPoolExecutor #94784

Merged
merged 7 commits into from Jul 10, 2023
Merged

gh-94777: Fix deadlock in ProcessPoolExecutor #94784

merged 7 commits into from Jul 10, 2023

Conversation

lpaulot
Copy link
Contributor

@lpaulot lpaulot commented Jul 12, 2022 •
edited by bedevere-bot

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jul 12, 2022 •
edited

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@lpaulot lpaulot mentioned this pull request Jul 12, 2022
Closed
@lpaulot lpaulot changed the title gh-94777: Drain call_queue to avoid hanging in ProcessPoolExecutor gh-94777: Fix deadlock in ProcessPoolExecutor Jul 13, 2022
itamaro
itamaro approved these changes Jul 8, 2023
View reviewed changes
Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the report and the fix!

I was able to repro the issue (on MacOS, 3.13 main branch as of today) and confirmed this patch fixes it.

@carljm carljm requested review from gpshead and pitrou July 10, 2023 16:44
@carljm
Copy link
Member

carljm commented Jul 10, 2023

@gpshead @pitrou You are listed in devguide as experts for multiprocessing module; any concerns about merging this fix?

We ran into this deadlock in an internal codebase and @mpage independently investigated and produced this exact same fix, before we found this issue and PR. So I'm inclined to merge, but not a multiprocessing expert.

@pitrou
Copy link
Member

pitrou commented Jul 10, 2023

@carljm Thanks for the ping. No objection from me.

@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes topic-multiprocessing labels Jul 10, 2023
@gpshead gpshead enabled auto-merge (squash) July 10, 2023 21:16
@gpshead
Copy link
Member

gpshead commented Jul 10, 2023

Oh nice, LGTM. Thanks! I've tweaked the NEWS entry and hit the auto-merge button.

@gpshead gpshead merged commit 6782fc0 into python:main Jul 10, 2023
27 of 28 checks passed
@miss-islington
Copy link
Contributor

Thanks @lpaulot for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @lpaulot and @gpshead, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 6782fc050281205734700a1c3e13b123961ed15b 3.12

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2023
@lpaulot @miss-islington
pythongh-94777: Fix deadlock in ProcessPoolExecutor (pythonGH-94784)
fc537a3 
Fixes a hang in multiprocessing process pool executor when a child process crashes and code could otherwise block on writing to the pipe.  See pythonGH-94777 for more details.
(cherry picked from commit 6782fc0)

Co-authored-by: Louis Paulot <55740424+lpaulot@users.noreply.github.com>
@bedevere-bot
Copy link

GH-106607 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 10, 2023
@carljm carljm added needs backport to 3.12 bug and security fixes and removed needs backport to 3.12 bug and security fixes labels Jul 10, 2023
@miss-islington
Copy link
Contributor

Thanks @lpaulot for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2023
@lpaulot @miss-islington
pythongh-94777: Fix deadlock in ProcessPoolExecutor (pythonGH-94784)
25edb68 
Fixes a hang in multiprocessing process pool executor when a child process crashes and code could otherwise block on writing to the pipe.  See pythonGH-94777 for more details.
(cherry picked from commit 6782fc0)

Co-authored-by: Louis Paulot <55740424+lpaulot@users.noreply.github.com>
@bedevere-bot
Copy link

GH-106609 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Jul 10, 2023
carljm pushed a commit that referenced this pull request Jul 10, 2023
@miss-islington @lpaulot
[3.11] gh-94777: Fix deadlock in ProcessPoolExecutor (GH-94784) (#106607
d63953d 
)

gh-94777: Fix deadlock in ProcessPoolExecutor (GH-94784)

Fixes a hang in multiprocessing process pool executor when a child process crashes and code could otherwise block on writing to the pipe.  See GH-94777 for more details.
(cherry picked from commit 6782fc0)

Co-authored-by: Louis Paulot <55740424+lpaulot@users.noreply.github.com>
carljm pushed a commit that referenced this pull request Jul 10, 2023
@miss-islington @lpaulot
[3.12] gh-94777: Fix deadlock in ProcessPoolExecutor (GH-94784) (#106609
90ea3be 
)

gh-94777: Fix deadlock in ProcessPoolExecutor (GH-94784)

Fixes a hang in multiprocessing process pool executor when a child process crashes and code could otherwise block on writing to the pipe.  See GH-94777 for more details.
(cherry picked from commit 6782fc0)

Co-authored-by: Louis Paulot <55740424+lpaulot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpage mpage mpage left review comments

@pitrou pitrou pitrou left review comments

@gpshead gpshead gpshead approved these changes

@itamaro itamaro itamaro approved these changes

Assignees

@gpshead gpshead

Labels
topic-multiprocessing type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ProcessPoolExecutor deadlock when a child process crashes while data is being sent in call queue
9 participants
@lpaulot @bedevere-bot @carljm @pitrou @gpshead @miss-islington @itamaro @mpage @AlexWaygood