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.

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

PyCell_GET/SET are not thread-safe in free-threaded builds #127271

Closed
nascheme opened this issue Nov 25, 2024 · 0 comments
Closed

PyCell_GET/SET are not thread-safe in free-threaded builds #127271

nascheme opened this issue Nov 25, 2024 · 0 comments
Labels
topic-free-threading type-bug An unexpected behavior, bug, or error

Comments

@nascheme
Copy link
Member

nascheme commented Nov 25, 2024 •
edited by bedevere-app bot
Loading

Bug report

Bug description:

The PyCell_GET and PyCell_SET macros are not safe for free-threading builds. First, they don't the correct locking. Second, PyCell_GET returns a borrowed reference.

For CPython internals, the usages of these macros can be replaced (either with PyCell_GetRef(), PyCell_SetTakeRef(), or similar). For external API users, these macros will need to be marked as deprecated, at least for free-threaded builds.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs
@nascheme nascheme added type-bug An unexpected behavior, bug, or error topic-free-threading labels Nov 25, 2024
@bedevere-app bedevere-app bot mentioned this issue Nov 25, 2024
Merged
nascheme added a commit to nascheme/cpython that referenced this issue Nov 26, 2024
@nascheme
pythongh-127271: Replace use of PyCell_GET/SET
10ac6e6 
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`.
@github-actions github-actions bot mentioned this issue Dec 1, 2024
Closed
nascheme added a commit that referenced this issue Dec 3, 2024
@nascheme
gh-127271: Replace use of PyCell_GET/SET (gh-127272)
fc5a0dc 
* 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.
@nascheme nascheme closed this as completed Dec 3, 2024
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this issue Jan 8, 2025
@nascheme @srinivasreddy
pythongh-127271: Replace use of PyCell_GET/SET (pythongh-127272)
43d26ea 
* 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 issue Jan 12, 2025
@nascheme @ebonnal
pythongh-127271: Replace use of PyCell_GET/SET (pythongh-127272)
a0cb339 
* 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.
@bedevere-app bedevere-app bot mentioned this issue Feb 20, 2025
Merged
gaogaotiantian added a commit that referenced this issue Feb 27, 2025
@gaogaotiantian
gh-127271: Remove the PyCell_Get usage for framelocalsproxy (#130383)
6140b08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-free-threading type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@nascheme