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.

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

gh-108520: Fix bad fork detection in nested multiprocessing use case #108568

Merged
merged 9 commits into from Aug 30, 2023

Conversation

albanD
Copy link
Contributor

@albanD albanD commented Aug 28, 2023

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.

@albanD
Copy link
Contributor Author

albanD commented Aug 28, 2023

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

@albanD
Copy link
Contributor Author

albanD commented Aug 29, 2023

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

@albanD
Copy link
Contributor Author

albanD commented Aug 30, 2023

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.

@pitrou
Copy link
Member

pitrou commented Aug 30, 2023

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

@pitrou pitrou added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Aug 30, 2023
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.

+1, thanks for the quick fix @albanD

@pitrou
Copy link
Member

pitrou commented Aug 30, 2023

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

@pitrou pitrou enabled auto-merge (squash) August 30, 2023 16:25
@pitrou pitrou merged commit add8d45 into python:main Aug 30, 2023
21 of 22 checks passed
@bedevere-bot
Copy link

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

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

@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 30, 2023
… 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 <desmaison.alban@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@bedevere-bot
Copy link

GH-108691 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 Aug 30, 2023
carljm added a commit to carljm/cpython that referenced this pull request Aug 30, 2023
* 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
Copy link

GH-108692 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 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
…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 <desmaison.alban@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Yhg1s pushed a commit that referenced this pull request Aug 30, 2023
…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 <desmaison.alban@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@vstinner
Copy link
Member

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

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.

None yet

5 participants