-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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-127271: Replace use of PyCell_GET/SET #127272
Conversation
These macros are not safe to use in the free-threaded build. Use `PyCell_GetRef()` and `PyCell_SetTakeRef()` instead. Add `PyCell_GET` to the free-threading howto table of APIs that return borrowed refs. Add critical sections to `PyCell_GET` and `PyCell_SET`.
ac281e2
to
10ac6e6
Compare
While tinkering with the cellobject code, I found a couple more data races. I've included the fixes for those in this PR. For the reviewer, the reference counting changes to |
Avoid the type error and return results as before.
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.
I kicked off benchmark runs for this change for both the default and free-threaded builds. I'll post the results here once they're ready, likely tomorrow morning.
Performance on the default build looks neutral. |
|
I can't reproduce those two failing tests on my Debian dev machine. My guess is the failure is not related to this commit. I'll investigate some more though. |
These macros are not safe to use in the free-threaded build. Use
PyCell_GetRef()
andPyCell_SetTakeRef()
instead. AddPyCell_GET
to the free-threading howto table of APIs that return borrowed refs. Add critical sections toPyCell_GET
andPyCell_SET
.Fix some thread safety issues with cell objects:
cell_richcompare
,cell_repr
,cell_set_contents
needed some work. Add an addtional unit test module that tries to trigger data races for these.