GH-129817: thread safety for tp_flags

[nascheme]

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. Set by the _abc_register function. It seems there is no way to toggle these flags off once set.
• 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.

• Issue: Data race on tp_flags with subinterpreters and static types #129817

[colesbury]

This seems related to the following issue in that some of the operations that change tp_flags also change type slots:

Type slots are not thread-safe in free-threaded builds (#127266)

Overall, I'm not really sure about this:

• I think these sorts of changes are more likely to introduce problems than fix any problems users will actually encounter
• The abc related flags seem like things that are going to be set during or immediately after type creation. I'm not convinced we should spend effort making that thread-safe

[colesbury]

I'm a bit worried about this assumption.

[nascheme]

Do you worry that it's not currently true or that it might be not true in a future version of Python? The fact that non-heap types are immutable seems a pretty important assumption currently. In PyType_Ready():

    /* Historically, all static types were immutable. See bpo-43908 */
    if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
        type_add_flags(type, Py_TPFLAGS_IMMUTABLETYPE);
        /* Static types must be immortal */
        _Py_SetImmortalUntracked((PyObject *)type);
    }

[colesbury]

I'm worried that it's not currently true:

• I don't really trust that code comment. Static types and C API extensions predate Py_TPFLAGS_IMMUTABLETYPE. I think you can support attribute modification on types if you construct your static type in the right way.
• Immutable in that context is talking about attribute modification, not changes to tp_flags. I don't think it covers things like collections.abc.Iterator.register(my_type)
• C extensions can modify type slots and tp_flags even if you can't change attributes from Python code.

[nascheme]

This seems related to the following issue in that some of the operations that change tp_flags also change type slots:

Type slots are not thread-safe in free-threaded builds (#127266)

Yeah, I think it's related. Doing a stop-the-world pause seems like the only reasonable fix for that. Maybe the same thing could be done for these couple of tp_flags that can be toggled after initial type creation?

Overall, I'm not really sure about this:

• I think these sorts of changes are more likely to introduce problems than fix any problems users will actually encounter

I agree that it's pretty unlikely for users to actually encounter problems with this. As you say, the ABC register() is generally called soon after the type is created. It doesn't have to be but maybe we could add a documentation note somewhere that to be free-threading safe then register() needs to be called before the class is used.

Aside from the flags to support ABC, the only other one is Py_TPFLAGS_HAVE_VECTORCALL. I think having that flag read wrongly would only cause your newly set __call__ method to be ignored (not cause crashes or other nasty behavior). Instead, Python would keep using vectorcall.

We could fix this by rather than clearing the Py_TPFLAGS_HAVE_VECTORCALL flag we could set tp_vectorcall_offset to something invalid, like -1. Then, in places like _PyVectorcall_FunctionInline, check that tp_vectorcall_offset > 0. We can read and write tp_vectorcall_offset atomically.

• The abc related flags seem like things that are going to be set during or immediately after type creation. I'm not convinced we should spend effort making that thread-safe

Yeah, I think that's the case. In restrospect, we should have did the register() operation differently. E.g. when defining the class, you could have a special class attribute like __abc_types__ with a list of ABCs that should be registered. Then it can be done at type_ready() time.

Another idea is to make _abc_register() do a stop-the-world pause. Registering new ABC types is not a very common operation.

[colesbury]

I think these are reasonable ideas. I'm still not sure they are worth pursuing right now. I'd be okay with deferring work on the potential issues with tp_flags and type slots until 3.15.

[nascheme]

Closing this for now, we can re-visit for 3.15.