gh-127271: Replace use of PyCell_GET/SET#127272
Merged
nascheme merged 6 commits intopython:mainfrom Dec 3, 2024
Merged
Conversation
nascheme
changed the title
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`.
nascheme
force-pushed
the
gh-127271-cell-get-set
branch
from
ac281e2 to
10ac6e6
Compare
nascheme
requested review from
ericsnowcurrently and
markshannon
as code owners
Member
Author
|
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.
mpage
reviewed
Dec 3, 2024
Contributor
mpage
left a comment
mpage
left a comment
There was a problem hiding this comment.
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.
mpage
added
the
🔨 test-with-refleak-buildbots
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
Contributor
|
Performance on the default build looks neutral. |
|
Member
Author
|
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. |
srinivasreddy
pushed a commit
to srinivasreddy/cpython
that referenced
this pull request
Jan 8, 2025
* Replace uses of `PyCell_GET` and `PyCell_SET`. These macros are not safe to use in the free-threaded build. Use `PyCell_GetRef()` and `PyCell_SetTakeRef()` instead. * Since `PyCell_GetRef()` returns a strong rather than borrowed ref, some code restructuring was required, e.g. `frame_get_var()` returns a strong ref now. * Add critical sections to `PyCell_GET` and `PyCell_SET`. * Move critical_section.h earlier in the Python.h file. * Add `PyCell_GET` to the free-threading howto table of APIs that return borrowed refs. * Add additional unit tests for free-threading.
ebonnal
pushed a commit
to ebonnal/cpython
that referenced
this pull request
Jan 12, 2025
* Replace uses of `PyCell_GET` and `PyCell_SET`. These macros are not safe to use in the free-threaded build. Use `PyCell_GetRef()` and `PyCell_SetTakeRef()` instead. * Since `PyCell_GetRef()` returns a strong rather than borrowed ref, some code restructuring was required, e.g. `frame_get_var()` returns a strong ref now. * Add critical sections to `PyCell_GET` and `PyCell_SET`. * Move critical_section.h earlier in the Python.h file. * Add `PyCell_GET` to the free-threading howto table of APIs that return borrowed refs. * Add additional unit tests for free-threading.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
These macros are not safe to use in the free-threaded build. Use
PyCell_GetRef()andPyCell_SetTakeRef()instead. AddPyCell_GETto the free-threading howto table of APIs that return borrowed refs. Add critical sections toPyCell_GETandPyCell_SET.Fix some thread safety issues with cell objects:
cell_richcompare,cell_repr,cell_set_contentsneeded some work. Add an addtional unit test module that tries to trigger data races for these.