Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-116738: Use PyMutex for bz2 module#140555

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
colesbury merged 5 commits intopython:mainfromyoney:ft_bz2
Oct 27, 2025
Merged

Conversation

@yoney
Copy link
Contributor

@yoneyyoney commentedOct 24, 2025•
edited by bedevere-appbot
Loading

I was initially looking into thebz2 module for free-threading. The methods are already wrapped with a lock, which I believe is primarily used to release the GIL. This locking makes the methods thread-safe in free-threaded build. I replacedPyThread_acquire_lock withPyMutex, which releases the GIL when the thread is parked. This change removes some macros and allocation handling code.

cc:@mpage@colesbury

Copy link
Member

@picnixzpicnixz left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Thanks and sorry for the back-and-forth. A small tip: when I (and I meanme explicitly) suggest something (and don't explicitly request changes), you don't need to directly follow my advice if my suggestion was phrased as a question (that question could be addressed to a wider audience).

@yoney
Copy link
ContributorAuthor

@picnixz Thanks for the review! I’m glad we discoveredzstd, thanks to@emmatyping.

Since the changes were quite small, I just created the extra commit, knowing we might need to revert if needed. This is just part of a good review process!

emmatyping reacted with heart emoji

Copy link
Member

@emmatypingemmatyping left a comment

Choose a reason for hiding this comment

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

Thanks! I think this looks good

@emmatyping
Copy link
Member

Oh, perhaps we should add a test on free-threading to ensure we aren't getting data races and the locking is working? Feel free to take some inspiration fromhttps://github.com/python/cpython/blob/main/Lib/test/test_zstd.py#L2668

yoney reacted with thumbs up emoji

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

@yoney
Copy link
ContributorAuthor

Oh, perhaps we should add a test on free-threading to ensure we aren't getting data races and the locking is working? Feel free to take some inspiration fromhttps://github.com/python/cpython/blob/main/Lib/test/test_zstd.py#L2668

@emmatyping Thanks for pointing to thezstd tests! I had a local test for compressor, but after seeing the decompressor example inzstd, I also created a decompression test and added it to the PR. Thank you!

@colesburycolesbury merged commit9479a62 intopython:mainOct 27, 2025
43 checks passed
@yoneyyoney deleted the ft_bz2 branchOctober 28, 2025 15:51
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull requestDec 6, 2025
The methods are already wrapped with a lock, which makes them thread-safe infree-threaded build. This replaces `PyThread_acquire_lock` with `PyMutex` andremoves some macros and allocation handling code.Also add a test for free-threading to ensure we aren't getting data races andthat the locking is working.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes

@colesburycolesburycolesbury approved these changes

@emmatypingemmatypingemmatyping approved these changes

@picnixzpicnixzpicnixz approved these changes

@kumaraditya303kumaraditya303kumaraditya303 approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yoney@emmatyping@vstinner@colesbury@picnixz@kumaraditya303
[8]ページ先頭
©2009-2025 Movatter.jp