GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-108520: Fix bad fork detection in nested multiprocessing use case by albanD · Pull Request #108568 · python/cpython

Conversation

@albanD

@albanD albanD commented Aug 28, 2023 •
edited by pitrou
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

gh-107275 introduced a regression where a SemLock would fail being passed along nested child processes, as the is_fork_ctx attribute would be left missing after the first deserialization.

  • Issue: Nested multiprocessing leads to AttributeError: is_fork_ctx with forkserver or spawn methods #108520

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot mentioned this pull request Aug 28, 2023
Nested multiprocessing leads to AttributeError: is_fork_ctx with forkserver or spawn methods #108520
Closed
2 tasks done
@bedevere-bot bedevere-bot added the awaiting review label Aug 28, 2023
@albanD

albanD commented Aug 28, 2023

Copy link
Copy Markdown
Contributor Author

I'm not sure how to read the test failure? Are they actually relevant?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@albanD
Support nesting and add test
ec4e7e0
@albanD
albanD force-pushed the fix-issue-108520 branch from 9d43084 to ec4e7e0 Compare August 29, 2023 20:52
@albanD

albanD commented Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

Rebased to check fi the windows test failure was just flaky.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pitrou
pitrou reviewed Aug 30, 2023
View reviewed changes
Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pitrou
pitrou reviewed Aug 30, 2023
View reviewed changes
Comment thread Lib/test/_test_multiprocessing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pitrou
pitrou reviewed Aug 30, 2023
View reviewed changes
Comment thread Lib/test/_test_multiprocessing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@albanD
Cleanup test code
1b62ab0
albanD
albanD commented Aug 30, 2023
View reviewed changes
Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

albanD added 2 commits August 30, 2023 11:15
@albanD
Properly set is_fork_ctx in set_state
9212635
@albanD
Rename is_fork_ctx to _is_fork_ctx
c0ee443
@albanD

albanD commented Aug 30, 2023

Copy link
Copy Markdown
Contributor Author

All the updates are done.
Let me know if the a blurb is needed?
Also I guess commits are squashed before merging? let me know if you want me to cleanup the commits in the branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pitrou

pitrou commented Aug 30, 2023

Copy link
Copy Markdown
Member

Yes, a blurb is needed especially as it fixes a regression.
Yes, commits will be squashed automatically when merging :-)

albanD reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@blurb-it
📜🤖 Added by blurb_it.
19225fc
@pitrou pitrou added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Aug 30, 2023
@pitrou
Blurb nits
61d402d
pitrou
pitrou approved these changes Aug 30, 2023
View reviewed changes

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

+1, thanks for the quick fix @albanD

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
pitrou
pitrou reviewed Aug 30, 2023
View reviewed changes
Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pitrou added 2 commits August 30, 2023 18:19
@pitrou
Improve co
6eae967
@pitrou
Merge branch 'main' into fix-issue-108520
11e6b02
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Aug 30, 2023
pitrou
pitrou reviewed Aug 30, 2023
View reviewed changes
Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@pitrou
Update comment
051f326
@pitrou

pitrou commented Aug 30, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Ok, it took me a couple attempts to get the comments right from the GitHub UI :-) Will merge if CI is green.

albanD reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pitrou
pitrou enabled auto-merge (squash) August 30, 2023 16:25
@pitrou
pitrou merged commit add8d45 into python:main Aug 30, 2023
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting merge labels Aug 30, 2023
@bedevere-bot

bedevere-bot commented Aug 30, 2023

Copy link
Copy Markdown

There's a new commit after the PR has been approved.

@pitrou: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot
bedevere-bot requested a review from pitrou August 30, 2023 17:16
@miss-islington

miss-islington commented Aug 30, 2023

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 30, 2023
@albanD @blurb-it
@pitrou @miss-islington
pythongh-108520: Fix bad fork detection in nested multiprocessing use…
c62507f
… case (pythonGH-108568)

pythongh-107275 introduced a regression where a SemLock would fail being passed along nested child processes, as the `is_fork_ctx` attribute would be left missing after the first deserialization.

---------

(cherry picked from commit add8d45)

Co-authored-by: albanD <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <[email protected]>
@bedevere-bot bedevere-bot removed the awaiting core review label Aug 30, 2023
@bedevere-bot

bedevere-bot commented Aug 30, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Aug 30, 2023
carljm added a commit to carljm/cpython that referenced this pull request Aug 30, 2023
@carljm
Merge branch 'main' into icompexc
37377e6
* main:
  pythongh-108520: Fix bad fork detection in nested multiprocessing use case (python#108568)
  pythongh-108590: Revert pythongh-108657 (commit 400a1ce) (python#108686)
  pythongh-108494: Argument Clinic: Document how to generate code that uses the limited C API (python#108584)
  Document Python build requirements (python#108646)
  pythongh-101100: Fix Sphinx warnings in the Logging Cookbook (python#108678)
  Fix typo in multiprocessing docs (python#108666)
  pythongh-108669: unittest: Fix documentation for TestResult.collectedDurations (python#108670)
  pythongh-108590: Fix sqlite3.iterdump for invalid Unicode in TEXT columns (python#108657)
  Revert "pythongh-103224: Use the realpath of the Python executable in `test_venv` (pythonGH-103243)" (pythonGH-108667)
  pythongh-106320: Remove private _Py_ForgetReference() (python#108664)
  Mention Ellipsis pickling in the docs (python#103660)
  Revert "Use non alternate name for Kyiv (pythonGH-108533)" (pythonGH-108649)
  pythongh-108278: Deprecate passing the first param of sqlite3.Connection callback APIs by keyword (python#108632)
  pythongh-108455: peg_generator: install two stubs packages before running mypy (python#108637)
  pythongh-107801: Improve the accuracy of io.IOBase.seek docs (python#108268)
@bedevere-bot

bedevere-bot commented Aug 30, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Aug 30, 2023
@albanD
albanD deleted the fix-issue-108520 branch August 30, 2023 19:23
pitrou added a commit that referenced this pull request Aug 30, 2023
@miss-islington @albanD
@blurb-it @pitrou
[3.11] gh-108520: Fix bad fork detection in nested multiprocessing us…
385b195
…e case (GH-108568) (#108692)

gh-107275 introduced a regression where a SemLock would fail being passed along nested child processes, as the `is_fork_ctx` attribute would be left missing after the first deserialization.

---------

(cherry picked from commit add8d45)

Co-authored-by: albanD <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <[email protected]>
Yhg1s pushed a commit that referenced this pull request Aug 30, 2023
@miss-islington @albanD
@blurb-it @pitrou
[3.12] gh-108520: Fix bad fork detection in nested multiprocessing us…
320d398
…e case (GH-108568) (#108691)

gh-108520: Fix bad fork detection in nested multiprocessing use case (GH-108568)

gh-107275 introduced a regression where a SemLock would fail being passed along nested child processes, as the `is_fork_ctx` attribute would be left missing after the first deserialization.

---------

(cherry picked from commit add8d45)

Co-authored-by: albanD <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <[email protected]>
@vstinner vstinner mentioned this pull request Sep 22, 2023
test_multiprocessing_fork: test_nested_startmethod() fails randomly. #109706
Closed
@vstinner

vstinner commented Sep 22, 2023

Copy link
Copy Markdown
Member

The test fails randomly. Can you please have a look? I proposed a fix.

  • Issue test_multiprocessing_fork: test_nested_startmethod() fails randomly. #109706
  • PR gh-109706: Fix multiprocessing test_nested_startmethod() #109707
albanD reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou Awaiting requested review from pitrou

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@albanD @pitrou @bedevere-bot @miss-islington @vstinner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.