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.

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-129817: thread safety for tp_flags #130983

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Mar 8, 2025

Note: this PR needs a bit more polish before it becomes non-draft. I added Sam and Matt to the reviewers in case they have some initial feedback on this approach.

Use a separate structure member in PyHeapTypeObject, ht_flags, to store a copy of the type flags. That allows safe toggling of some of the flags after the type has been initialized and potentially exposed to other threads.

We would prefer to not use an atomic load whenever tp_flags is read. That causes a lot of code churn (see gh-130892) and potentially some performance hit on platforms with weak memory ordering. Instead, we would like to use a normal load and only set the flags before the type has been exposed to other threads. That's mostly the case except for the flags listed below.

The following type flags cause issues in that they may be toggled after the type is initially created and exposed:

  • Py_TPFLAGS_SEQUENCE and Py_TPFLAGS_MAPPING. Toggled by assigning special methods to the type object.
  • Py_TPFLAGS_IS_ABSTRACT. Toggled by assigning a non-empty set to __abstractmethods__
  • Py_TPFLAGS_HAVE_VECTORCALL. Toggled off (no way to turn back on) by assigning __call__ to the type.

This PR does the following (only to the free-threaded build, the default build continues to work the same):

  • Add a new member to PyHeapTypeObject, ht_flags. This member only exists in the free-threaded build.
  • Copy the tp_flags value into ht_flags when type_ready() completes.
  • If one of the above flags is toggled after type initialization, toggle it in ht_flags. Use atomic operations to read and write it.
  • Use _PyType_HasFeatureSafe() to test the above flags. That function uses an atomic load.

This approach causes a bit of extra complication since we have to check Py_TPFLAGS_HEAPTYPE first to know where to look at the flags. For non-heap types, they are in tp_flags always. This could be avoided by adding an additional member to PyTypeObject. However, I think the Py_TPFLAGS_HEAPTYPE check should be cheap enough and putting it in PyHeapTypeObject ensures that extension types don't get confused by it.

Note that since all non-heap types are immutable, it is not possible to toggle type flags for them (at least, CPython itself doesn't do so). Also note that this change can break extensions that manipulate tp_flags directly or directly tests them, rather than using functions.

Use separate structure member in PyHeapTypeObject, `ht_flags`, to store
a copy of the type flags.  That allows safe toggling of some of the
flags after the type has been exposed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant