Description

MojoVampire

opened on May 19, 2026

Issue body actions

Feature or enhancement

Proposal:

Right now, Py_SETREF is implemented in non-atomic terms, which makes it subject to operation reordering issues on weakly ordered architectures (which caused a problem fixing gh-149816, where even switching from separate Py_CLEAR and later assignment to set_sni_cb to a single "atomic" Py_SETREF didn't fix the issue on ARM, because SSL's callback was being invoked asynchronously).

Py_SETREF is naturally implemented in terms of an atomic exchange, and at least for the free-threaded builds, it seems like it should be implemented as such to avoid such raciness, supplementing the existing:

#ifdef _Py_TYPEOF
#define Py_SETREF(dst, src) \
    do { \
        _Py_TYPEOF(dst)* _tmp_dst_ptr = &(dst); \
        _Py_TYPEOF(dst) _tmp_old_dst = (*_tmp_dst_ptr); \
        *_tmp_dst_ptr = (src); \
        Py_DECREF(_tmp_old_dst); \
    } while (0)
#else
#define Py_SETREF(dst, src) \
    do { \
        PyObject **_tmp_dst_ptr = _Py_CAST(PyObject**, &(dst)); \
        PyObject *_tmp_old_dst = (*_tmp_dst_ptr); \
        PyObject *_tmp_src = _PyObject_CAST(src); \
        memcpy(_tmp_dst_ptr, &_tmp_src, sizeof(PyObject*)); \
        Py_DECREF(_tmp_old_dst); \
    } while (0)
#endif

// Similarly long nonsense for Py_XSETREF

with a new atomic-based approach for free-threaded builds:

#define Py_SETREF(dst, src) Py_DECREF(_Py_atomic_exchange_ptr(&(dst), (src)))
#define Py_XSETREF(dst, src) Py_XDECREF(_Py_atomic_exchange_ptr(&(dst), (src)))

Please poke holes in this approach. I have a lot of experience with writing multithreaded C code, but will never be convinced I've covered every race condition no matter how simple the code.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Activity

1. MojoVampire

   added

   type-featureA feature request or enhancementA feature request or enhancement

   on May 19, 2026
2. aisk

   added

   topic-C-API

   topic-free-threading

   on May 19, 2026
3. picnixz

   added

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   on May 19, 2026
4. ZeroIntensity commented on May 19, 2026

   

   ZeroIntensity

   on May 19, 2026

   Member

   More actions

   We might want to add an (internal) atomic setref API. I don't think it's a good idea to use Py_SETREF/Py_XSETREF because the loads also need to be atomic, which is harder to do for third-party extensions.
5. MojoVampire commented on May 19, 2026

   

   MojoVampire

   on May 19, 2026

   ContributorAuthor

   More actions

   Yeah, on further consideration, it shouldn't alter the base macros; variables accessed entirely within mutex protected/critical sections, or variables that are entirely internal to a single function call and Py_SETREF is used for convenience, should not pay the overhead of atomic operations (especially a full sequential consistency operation, which appears to be the only version of atomic_exchange we're exposing within Python; we're not allowing one with mere acquire/release semantics that would have no overhead on an architecture with a strongly ordered memory model like the x86 family).
6. MojoVampire

   changed the title [-]Py_(X)SETREF should be atomic on free-threaded builds[/-] [+]Py_(X)SETREF should have atomic versions on free-threaded builds[/+] on May 19, 2026
7. MojoVampire commented on May 19, 2026

   

   MojoVampire

   on May 19, 2026

   ContributorAuthor

   More actions

   I figure the revised approach would be something like this:

   #ifdef Py_GIL_DISABLED
   #define _Py_SETREF_ATOMIC(dst, src) Py_DECREF(_Py_atomic_exchange_ptr(&(dst), (src)))
   #else
   #define _Py_SETREF_ATOMIC Py_SETREF
   #endif

   so on free-threaded builds it uses the full atomic exchange, while non-freethreaded builds continue to use the "assumed atomic due to GIL protections" Py_SETREF.
8. colesbury commented on May 19, 2026

   

   colesbury

   on May 19, 2026

   Contributor

   More actions

   Where do you intend to use this? Do you have a specific PR?

   Reacted by Peter Bierma
9. eendebakpt

   added 4 commits that reference this issue on May 22, 2026

   gh-NNNNN: Make _STORE_ATTR_SLOT lock-free on the free-threaded build

   73d3dc3

   gh-NNNNN: Make _STORE_ATTR_SLOT lock-free on the free-threaded build

   2cf7c01

   gh-NNNNN: Make _STORE_ATTR_SLOT lock-free on the free-threaded build

   17cca5e

   gh-NNNNN: Make _STORE_ATTR_SLOT lock-free on the free-threaded build

   e72e080

Issue actions