Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

1. If you don't have an account on b.p.o, please create one
2. Make sure your GitHub username is listed in "Your Details" at b.p.o
3. If you have not already done so, please sign the PSF contributor agreement
4. If you just signed the CLA, please wait at least a day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

Waiting on CLA approval.

bpo-19675: Fix pool cleanup issue would be the ideal commit message/title of the PR, you just needed to prefix bpo- for it to be good :)

Per continued discussion on https://bugs.python.org/issue19675 this patch still also needs tests. If you want to close this PR and submit another or continue iterating here, I'll leave that to you.

I just added the following note about testing to the issue:

(http://bugs.python.org/issue19675)

I don't think this can be tested. Throwing exceptions in the remote process causes exceptions that can't be caught in the same way (when the initializer fails the pool just attempts to recreate the process over and over) and I don't think it'd be acceptable to try to spawn too many processes in order to induce the original problem. There's not a lot of surface area to what we're doing here. We can't simulate failures any other way.

Once we get to the point where we can merge, I'll close this PR, cherry-pick the commit into the version-specific branches, and create new PRs.

Codecov Report

❗ No coverage uploaded for pull request base (master@c33ee85). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master      #57   +/-   ##
=========================================
  Coverage          ?   82.37%           
=========================================
  Files             ?     1427           
  Lines             ?   350958           
  Branches          ?        0           
=========================================
  Hits              ?   289088           
  Misses            ?    61870           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c33ee85...007f134. Read the comment docs.

@applio @briancurtin I am currently facing the limitations that would be resolved by this PR and would like to help in any way possible to get this merged. Do either of you have recommendations as to how we can get this PR passing all tests?

I'm, not fan of the actual patch: if an exception happen during _handle_workers, in a case it needs to respawn just one worker, you'll terminate all correctly running workers, which look bad.

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.

@brettcannon, are you aware of your duplicate comments?

@encukou yep, this one issue triggered an odd bug in my script; should be the only case.

Merged #5614 so this should be fixed now.