Skip to content

gh-115942: Add locked to several multiprocessing locks#115944

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
Merged
sobolevn merged 10 commits into from
Apr 8, 2025
Merged

gh-115942: Add locked to several multiprocessing locks #115944

sobolevn merged 10 commits into from
Apr 8, 2025

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commented Feb 26, 2024•
edited
Loading

TODO:

  • Get field-expert's approval
  • Docs
  • News

@bedevere-appbedevere-appbot mentioned this pull request Feb 26, 2024
Closed
@sobolevnsobolevn marked this pull request as ready for review March 13, 2024 06:44
@sobolevnsobolevn requested a review from gpshead as a code ownerMarch 13, 2024 06:44
@sobolevn
Copy link
MemberAuthor

sobolevn commented Jun 3, 2024

@gpshead do you have an opinion about this? :)

@sobolevn
Copy link
MemberAuthor

sobolevn commented Jun 21, 2024

friendly ping @gpshead

mpage
mpage reviewed Jun 21, 2024
View reviewed changes
vstinner
vstinner reviewed Mar 13, 2025
View reviewed changes
@gpshead
Copy link
Member

gpshead commented Mar 13, 2025

FWIW this PR makes sense to me.

@sobolevn
Copy link
MemberAuthor

sobolevn commented Mar 13, 2025

Ok, then I will add all the missing details. Thanks a lot! 👍

picnixz
picnixz reviewed Mar 13, 2025
View reviewed changes
vstinner
vstinner reviewed Apr 1, 2025
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

Please add tests on threading.RLock.locked().

picnixz
picnixz reviewed Apr 1, 2025
View reviewed changes
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

Just some rewording. Otherwise, once tests requested by Victor are added, LGTM.

@sobolevn
Copy link
MemberAuthor

sobolevn commented Apr 1, 2025

@vstinner@gpshead this is now ready to be reviewed :)

@sobolevnsobolevn requested review from mpage and vstinnerApril 1, 2025 09:43
picnixz
picnixz approved these changes Apr 1, 2025
View reviewed changes
vstinner
vstinner reviewed Apr 1, 2025
View reviewed changes
vstinner
vstinner approved these changes Apr 1, 2025
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM

hugovk
hugovk reviewed Apr 7, 2025
View reviewed changes
@sobolevn@hugovk
Apply suggestions from code review
1a50a20
Co-authored-by: Hugo van Kemenade <[email protected]>
gpshead
gpshead approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@gpsheadgpshead left a comment

Choose a reason for hiding this comment

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

I wish I could object to this being a locked() method instead of a .locked property... but this is matching an existing ancient API on purpose so... LGTM.

@sobolevnsobolevn merged commit f7305a0 into python:mainApr 8, 2025
42 checks passed
@sobolevn
Copy link
MemberAuthor

sobolevn commented Apr 8, 2025

Thanks everyone! 🎉

This was referenced Apr 15, 2025
Closed
Closed
@YvesDupYvesDup mentioned this pull request May 20, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovkhugovkhugovk left review comments

@gpsheadgpsheadgpshead approved these changes

@vstinnervstinnervstinner approved these changes

@picnixzpicnixzpicnixz approved these changes

@mpagempageAwaiting requested review from mpage

@brettcannonbrettcannonAwaiting requested review from brettcannonbrettcannon is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner

@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sobolevn@gpshead@vstinner@mpage@hugovk@picnixz