@brandtbucher Thanks for adding the labels. :) Do you mind adding the "skip news" label to this pull request and the bugfix one #4819, too? It's the only CI check missing from #4819 and as far as I understand, a news entry for that small patch is not necessary. The failed CI check might be the only thing missing from getting #4819 merged.

Thanks for the response :) Sorry, you're right - I had a look at only the most recent pull request which introduced a change to _test_multiprocessing.py which doesn't have a NEWS entry and makes use of the "skip news" label. I should have also looked at a few old pull requests which all do have a NEWS entry. ;) So 3f6cd15 should fix that now, I think.

Sorry for the delay. @finefoot is it possible for you to integrate the #4819 fix in this PR? It will make it easier to review and merge.

@pitrou I thought, I'd just adopt the changes directly from master once #4819 got merged. (To show the tests will pass, then.) But you want me to add the changes to managers.py with an individual commit before that, yes?

The fix for a bug and the tests for it should be logically contained in a single PR, so I'd like this PR (or the other one) to contain both.

@pitrou 👍 Done, I've merged the changes from #4819 and the tests just passed.

Codecov Report

Merging #16341 into master will increase coverage by 2.50%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #16341       +/-   ##
===========================================
+ Coverage   79.56%   82.06%    +2.50%     
===========================================
  Files         381     1955     +1574     
  Lines      168109   584066   +415957     
  Branches        0    44458    +44458     
===========================================
+ Hits       133753   479337   +345584     
- Misses      34356    95105    +60749     
- Partials        0     9624     +9624     

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 bfd0c96...b5877a0. Read the comment docs.

@finefoot You can leave the codecov failure alone, it's not relevant. The patch is in my todo list, it's just that I need to familiarize a bit with the code around that feature.

I first left his comment on #4819 but I perhaps should have added it here instead:

When testing this fix, this post on StackOverflow suggests they experienced a seg fault: https://stackoverflow.com/questions/56716470/multiprocessing-manager-nested-shared-objects-doesnt-work-with-queue

Has this been investigated and resolved?

Note that the solution I referenced in comments in @Darkonaut's answer produced seg faults in my own testing, but this solution did not.

From https://stackoverflow.com/a/58668088

Thank you @finefoot for creating this MR with additional tests. As I mentioned in #4819 and @finefoot noted above, it seems to me that the noted stackoverflow comment was not suggesting that this change causes crashes. And, two other commenters in #4819 including myself are unable to reproduce it. It'd be really, really, really nice to have this in 3.10.

I'll close and reopen this PR to force the tests to re-run, otherwise I can't merge it.

...Aaaaand we're back!

• What I should add:

Co-Authored-By:  Jordan Speicher <[email protected]>

• Waiting for tests to pass.
• Hopefully I can merge despite the failing codecov test.

Thanks @finefoot for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖

GH-26987 is a backport of this pull request to the 3.10 branch.

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

GH-26989 is a backport of this pull request to the 3.9 branch.