PyCriticalSection in Stable ABI

[encukou]

After discussion, I propose that:
if we get a stable ABI for free-threading, we should add PyCriticalSection to the Stable ABI.

struct PyCriticalSection;   // 2 pointers; members are private
struct PyCriticalSection2;  // 3 pointers; members are private
int PyCriticalSection_BeginWithError(PyCriticalSection *sc, PyObject *obj);
void PyCriticalSection_End(PyCriticalSection *sc);
int PyCriticalSection2_BeginWithError(PyCriticalSection *sc, PyObject *o1, PyObject *o2);
void PyCriticalSection2_End(PyCriticalSection *sc);

The _BeginWithError functions need to be added to the non-limited API/ABI as well. They work like the existing PyCriticalSection_Begin, but return 0 on success and -1 with an exception set on failure.

The structs are meant to be heap-allocated, hence adding them to the ABI. Dynamic allocation would make the calls slow (7.283 vs. 3.93 ns in microbenchmarks).
If we ever need an incompatible change to the structs, all the related APIs need to be deprecated, and new API needs to be added.

In the existing API, the Begin functions are infallible and return void. I am not convinced they are special enough to break our “no infallible functions” rule. There's a strong argument for that though:

Lots of callers don’t handle error cases either because it’s inconvenient or impractical in the context of lock acquisition. We want to be able to add mutexes (or critical sections) to existing code without transforming functions or code paths that can’t fail into ones that can.

Hence the new names. I also want to mark the functions with ``[[nodiscard]]/attribute((warn_unused_result))` on compilers that support it, so that their users get a warning if the result is ignored.

I am proposing to not expose the macros (Py_BEGIN_CRITICAL_SECTION & co.) right now. With fallible functions, thy don't improve ergonomics much.

I'm also not proposing to PyMutex variants, as we don't have PyMutex in Stable ABI.

[vote cancelled; see another one below]

The Discourse discussion is still open in case you want to chime in.

[vstinner]

The Discourse discussion is still open in case you want to chime in.

I replied directy in the discussion: https://discuss.python.org/t/stable-abi-for-pycriticalsection/106212/19

[encukou]

After discussion on the thread, my proposal is:

Expose current functions in Stable ABI (i.e. both abi3 and abi3t):

void PyCriticalSection_Begin(PyCriticalSection *sc, PyObject *obj);
void PyCriticalSection_End(PyCriticalSection *sc);
void PyCriticalSection2_Begin(PyCriticalSection2 *sc, PyObject *o1, PyObject *o2);
void PyCriticalSection2_End(PyCriticalSection2 *sc);

Add an additional pointer to PyCriticalSection, and expose the structs too:

typedef struct PyCriticalSection {...} PyCriticalSection;   // 3 pointers; members are private
typedef struct PyCriticalSection2 {...} PyCriticalSection2;  // 4 pointers; members are private

For convenience of C programmers, add the macros to Limited API:

Py_BEGIN_CRITICAL_SECTION(op)
Py_END_CRITICAL_SECTION()
Py_BEGIN_CRITICAL_SECTION2(a, b)
Py_END_CRITICAL_SECTION2()

And change Py_mod_gil handling, and PyUnstable_Module_SetGIL, to only consider the last bit of the value.

Note that Sam Gross is against the proposal: it “makes the API worse for unlikely scenarios that we could deal with in other ways. Like Victor said, the way to evolve the API, if necessary, is to add new structs and new functions.” (See my response.)

[edit] Vote moved further down.

[vstinner]

typedef struct PyCriticalSection {...} PyCriticalSection;   // 3 pointers; members are private
typedef struct PyCriticalSection2 {...} PyCriticalSection2;  // 4 pointers; members are private

In the current Python main branch, PyCriticalSection has 2 members and PyCriticalSection2 has 3 members. I don't think that adding a private reserved member is worth it. It would waste stack memory. If later we need to add a new member, we can change the ABI for that.

Py_BEGIN_CRITICAL_SECTION(op)
Py_END_CRITICAL_SECTION(cs)
Py_BEGIN_CRITICAL_SECTION2(a, b)
Py_END_CRITICAL_SECTION2(cs)

In the current Python main branch, the END macros have no parameters. Do you want to change the API to add cs parameter, or is it a mistake?

And change Py_mod_gil handling, and PyUnstable_Module_SetGIL, to only consider the last bit of the value.

In https://discuss.python.org/t/stable-abi-for-pycriticalsection/106212/24, you wrote that PyCriticalSection can be turned to no-ops "and thus the extension needs the GIL". I don't understand well this use case. I don't see why PyCriticalSection would be turned into no-ops.

I would prefer to defer this change until we actually need to change PyCriticalSection.

[zooba]

void PyCriticalSection2_Begin(PyCriticalSection *sc, PyObject *o1, PyObject *o2);
void PyCriticalSection2_End(PyCriticalSection *sc);

Are these support to take PyCriticalSection2 arguments? Any reason we can't just expose a single PyCriticalSection that works in both cases? It's not like anyone is doing type checks.

"Like Victor said, the way to evolve the API, if necessary, is to add new structs and new functions."

To me, this says it's a poor candidate for the limited API and the stable ABI, since it explicitly says that we can't evolve the implementation without forcing end-users to update their source code and rebuild their apps.

If it can't evolve "secretly", then users of it need to track with our regular compatibility guarantee.

[ngoldbaum]

@encukou is there a CPython branch that implements your latest proposal? I'd like to make sure I can get a working build of PyO3 using that branch.

[ngoldbaum]

I took a look today at whether it's possible for PyO3 to work around the lack of a critical section API and still support abi3t in Python 3.15, assuming this effort stalls.

There's one big issue: there's no safe API to iterate over dicts on the free-threaded build unless there is a critical section API available.

The docs for PyDict_Next say:

The function is not thread-safe in the free-threaded build without external synchronization for a mutable dict. You can use Py_BEGIN_CRITICAL_SECTION to lock the dictionary while iterating over it.

So that means there either needs to be an alternative API a critical section API in the stable ABI or some alternative safe C API function to implement this.

[encukou]

is there a CPython branch that implements your latest proposal?

[edit] Here: https://github.com/encukou/cpython/pull/39/changes

So that means there either needs to be an alternative API

That's PyObject_GetIter/PyIter_NextItem, and PyDict_GetItemRef to get the value.

[encukou]

In the current Python main branch, the END macros have no parameters. Do you want to change the API to add cs parameter, or is it a mistake?

A mistake; I corrected it.

Are these support to take PyCriticalSection2 arguments?

Yes; I also fixed this. Sorry!

Any reason we can't just expose a single PyCriticalSection that works in both cases? It's not like anyone is doing type checks.

The macros do type checks implicitly.

[ngoldbaum]

That's PyObject_GetIter/PyIter_NextItem, and PyDict_GetItemRef to get the value.

Thanks for the pointer. Good news! It turns out that I can get a PyO3 branch working with the abi3t ABI. See PyO3/pyo3#5786 (comment) for more details.

I'd still really like to see some version of the critical section API available to enable some patterns for thread-safe code in extensions, but I'm now much less worried about supporting the PyO3 ecosystem without any critical section API bindings.

[encukou]

ping @erlend-aasland @serhiy-storchaka @vstinner @zooba

[vstinner]

As I wrote previously, I would prefer to not add an unused reserved member to the two structures, since it would waste stack memory. I prefer to wait until there is a concrete need for adding new members to the structures to actually change the ABI (change structures and add new functions).

Also I don't understand the rationale for changing PyUnstable_Module_SetGIL() and Py_mod_gil handling. Can you elaborate why it's needed and how it will be used?

[encukou]

Background: currently, Py_mod_gil can be set to:

• 0 (Py_MOD_GIL_USED), enable the GIL on import
• 1 (Py_MOD_GIL_NOT_USED), keeps GIL disabled

The first part of the proposal is that we keep the Py_MOD_GIL #defines as they are, but only check the last bit (value & 0x1) at runtime. So e.g. 0b00 and 0b10 would re-enable the GIL; 0b01 and 0b11 would keep it disabled.

---

I prefer to wait until there is a concrete need for adding new members to the structures to actually change the ABI (change structures and add new functions).

Yes. At that point we add new structs and functions. The question is what to do with the existing (now old) functions.
The common plan for such a future would be:

• We add new functions/structs, and tell people to update their extensions (ideally just recompile, if the macros can be updated transparently).
• We remove the old functions from new Limited API.
• We change the Py_MOD_GIL_NOT_USED define to 0b10 (or 0b11, which would work the same way -- old interpreters will be incompatible with new functions, so we don't care how they'd interpret the last bit).

The rest depends on the situation:

1. If the old functions can no longer work:

   • We implement the old functions (which are now ABI-only) as no-ops
   • At import time we check the last two bits of Py_mod_gil:
     • 0b00 (Py_MOD_GIL_USED): Enable GIL on import.
     • 0b01 (old Py_MOD_GIL_NOT_USED): the extension (potentially) uses old critical section API, which is now no-ops. Re-enable GIL on import.
     • 0b10 (new Py_MOD_GIL_NOT_USED): the extension uses new critical section API. Keep GIL disabled.
     • 0b11: invalid value

2. If the old functions can continue working using the current structs, but perhaps with degraded performance (e.g. 3× slower) or behaviour (e.g. needing to malloc, with a fatal error if that fails):

   • We redefine Py_MOD_GIL_NOT_USED to 0b11; this signifies that the extension uses new critical section API,
   • At import time we check the last two bits of Py_mod_gil:
     • 0b00 (Py_MOD_GIL_USED): extension needs GIL. Enable GIL on import.
     • 0b01 (old Py_MOD_GIL_NOT_USED): the extension (potentially) uses old, degraded functions. On import, potentially emit a deprecation warning, but keep GIL disabled.
     • 0b10 (new Py_MOD_GIL_NOT_USED): the extension uses new critical section API only. Keep GIL disabled.
     • 0b11: invalid value

This part of the plan keeps options open but doesn't really cost anything.

---

The second part of the proposal is that PyCriticalSection is 3 pointers, 2 of which are used. This allows an additional case:

3. PyCriticalSection_Begin and the others can continue working with no API/ABI changes, but need the extra reserved pointer in the structs. The plan is instead:

   • We grow PyCriticalSection to 4 pointers, 3 of which are used.
   • We change the Py_MOD_GIL_USED define to 0b10.
   • We change the Py_MOD_GIL_NOT_USED define to 0b11.
   • We don't change how Py_mod_gil is handled, yet:
     • 0b00 (old Py_MOD_GIL_USED): Enable GIL on import.
     • 0b10 (new Py_MOD_GIL_USED): Enable GIL on import.
     • 0b01 (old Py_MOD_GIL_NOT_USED): Keep GIL disabled.
     • 0b11 (new Py_MOD_GIL_NOT_USED): Keep GIL disabled.
   • Note that old interpreters can use this extension; the last bit works as before.

   At a yet later point in time, we might need to use the new reserved pointer:

   • PyCriticalSection grows to 5 pointers, 4 of which are used.
   • We change Py_MOD_GIL_USED to 0b110, and Py_MOD_GIL_NOT_USED to 0b111
   • At import time we check the last 3 bits:
     • 0b001 (original Py_MOD_GIL_NOT_USED): Fail on import -- the struct is too small; the extension needs recompiling
     • 0b000 (original Py_MOD_GIL_USED): one of:
       • Fail on import, or
       • Enable the GIL, if this can ensure that the new functions only touch the original 3 pointers of PyCriticalSection.
     • 0b010 (now-old Py_MOD_GIL_USED): Enable GIL on import.
     • 0b110 (newest Py_MOD_GIL_USED): Enable GIL on import.
     • 0b011 (now-old Py_MOD_GIL_NOT_USED): Keep GIL disabled.
     • 0b111 (newest Py_MOD_GIL_NOT_USED): Keep GIL disabled.

If we don't do this, our main contingency plan would be "malloc, with a fatal error if that fails" (option 2 above).

[vstinner]

I don't understand how PyCriticalSection_Begin() can retrieve the Py_mod_gil slot of a module: the function doesn't get a module parameter. And I'm not aware of a function to retrieve the current module object. So I don't understand the purpose of changing Py_MOD_GIL_USED/Py_MOD_GIL_NOT_USED value and only checking the least significant bit of Py_mod_gil.

If the old functions can no longer work

I don't think that we can afford this option. Breaking the critical section ABI would affect too many extensions and it's a sensitive API, we will have to support it forever :-)

We already have multiple ABI-only functions for the stable ABI (ex: PyObject_CheckReadBuffer()).

If we change the structure, we should keep the old structures and functions, but add new ABI functions for the new structures. At the API level, names don't need to change, we can use macros to rename functions.

Example:

// old ABI
typedef struct PyCriticalSection PyCriticalSection;
PyAPI_FUNC(void) PyCriticalSection_Begin(PyCriticalSection *c, PyObject *op);

// new ABI
typedef struct PyNewCriticalSection PyNewCriticalSection;
PyAPI_FUNC(void) PyCriticalSection_BeginNew(PyNewCriticalSection *c, PyObject *op);

#define PyCriticalSection PyNewCriticalSection
#define PyCriticalSection_Begin PyCriticalSection_BeginNew

If the old functions can continue working using the current structs, but perhaps with degraded performance (e.g. 3× slower) or behaviour (e.g. needing to malloc, with a fatal error if that fails):

Sure, if the structure changes, we can modify PyCriticalSection_Begin() implementation to allocate memory on the heap for the new structure and uses the old structure just to store a pointer. In this case, IMO a fatal error on a memory allocation failure is an acceptable trade-off.

[encukou]

I don't understand how PyCriticalSection_Begin() can retrieve the Py_mod_gil slot of a module

It can't.
But Py_mod_gil can be checked at import time to enable the GIL. With the GIL enabled, the critical section API can be implemented as no-ops.

[vstinner]

Currently on a free threaded build, when importing an extension module enables the GIL, the critical section functions remain unchanged: they continue to lock the object mutex. I don't understand well the consequence of changing the critical section functions to become no-op in this case, but it may introduce issues.

I don't think that modifying Py_mod_gil is worth it. It sounds complicated and risky. As I wrote previously, if the structures are modified to add new members, we should make sure that the existing ABI just continues to work somehow. If there is no way to support new critical sections with the old structures, we can always modify functions to allocate memory on the heap.