Free-threading race betwen two Py_SETREF

Bug report

This can be triggered with existing Python objects that use Py_SETREF or Py_XSETREF such as Exceptions (although slightly contrived):

import threading

e = Exception()

def worker():
    for _ in range(1_000_000):
        e.__init__(1)

threads = [threading.Thread(target=worker) for _ in range(2)]
for t in threads:
    t.start()
for t in threads:
    t.join()

Gives on a 3.16 free-threaded debug build:

../cpython/Include/refcount.h:617: _Py_NegativeRefcount: Assertion failed: object has negative ref count
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x2000204c3b0
object refcount : 0
object type     : 0x5623737ad5c8
object type name: tuple
object repr     : <refcnt 0 at 0x2000204c3b0>

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Stack (most recent call first):
  File "/<home>/projects/python/setref_bug.py", line 8 in worker
  File "/<home>/projects/python/cpython/Lib/threading.py", line 1161 in run
  File "/<home>/projects/python/cpython/Lib/threading.py", line 1219 in _bootstrap_inner
  File "/<home>/projects/python/cpython/Lib/threading.py", line 1181 in _bootstrap
[1]    397225 abort (core dumped)  ./build/python setref_bug.py

This arrives because Py_SETREF copies the ref and stores a new new one as two separate non-atomic steps. So two threads can do:

T1: old = *slot
T2: old = *slot      // same object as T1
T1: *slot = a
T2: *slot = b        // a's reference is lost, so a leaks
T1: Py_XDECREF(old)
T2: Py_XDECREF(old)  // the slot's one reference to old is dropped twice

I ran into this trying to build a cache which swapped an old value for a new value with Py_SETREF and finding it sometimes resulted in a double free when two threads swapped the value simultaneously (And ended up thinking they had the last reference).

CPython versions tested on:

3.16, CPython main branch

Operating systems tested on:

Linux

Concurrent Py_SETREF(dst, src) / Py_XSETREF calls on the same dst are not intended to be thread-safe.

Separately, we also generally don't try to make concurrent __init__() on the same object thread-safe unless there's some code in the wild that actually does this. We may revisit that in the future, but for now, it seems like it would add code complexity and performance regressions for little benefit.

I'm okay with that as a documented limitation.

Having an atomic version that is opt-in would make lazy thread-safe caches significantly easier to write: init to Py_None or NULL then once make a cache dictionary just Py_SETREF_THREADSAFE or Py_XSETREF_THREADSAFE. Open to a helper like that?

Across CPython currently there's quite a few descriptors which individual elements use these (ex. property___set_name___impl) where locking the whole object for the single setter likely doesn't make sense but having two threads setting .name concurrently not crash the interpreter would be nice.

Having an atomic version that is opt-in would make lazy thread-safe caches significantly easier to write: init to Py_None or NULL then once make a cache dictionary just Py_SETREF_THREADSAFE or Py_XSETREF_THREADSAFE. Open to a helper like that?

Py_SETREF() / Py_XSETREF() macros caused me a lot of troubles over the last months: C++ compatibility issue (issue 1, issue 2) and strict aliasing issue recently. If possible, I would prefer to not add more macros like these ones.

In https://github.com/python/cpython/pull/161793/changes, Py_XSETREF()/Py_SETREF() was replaced with _Py_atomic_exchange_ptr() + _PyObject_XDecRefDelayed(). Do we always need this exact implementation? Or is it fine to call Py_XDECREF() instead in some cases?

_PyObject_XDecRefDelayed() is only accessible in the internal C API. So if you want a helper calling it, it can only be part of the internal C API.

The pattern I like is in _warnings now (

cpython/Python/_warnings.c

Lines 343 to 363 in 60eca2d

    
           static void 
        
           update_state_ref(PyObject **ptr, PyObject *value) 
        
           { 
        
               if (FT_ATOMIC_LOAD_PTR_ACQUIRE(*ptr) == value) { 
        
                   return; 
        
               } 
        
               PyUnstable_EnableTryIncRef(value); 
        
               PyObject *old = _Py_atomic_exchange_ptr(ptr, Py_NewRef(value)); 
        
               Py_XDECREF(old); 
        
           } 
        
           /* Return a new reference to the warnings state field '*ptr'. */ 
        
           static PyObject * 
        
           get_state_ref(PyObject **ptr) 
        
           { 
        
           #ifdef Py_GIL_DISABLED 
        
               return _Py_XGetRef(ptr); 
        
           #else 
        
               return Py_XNewRef(*ptr); 
        
           #endif 
        
           }

).

I am definitely happy to keep all these parts internal only. I'm worried copy/paste of the pattern is worse than getting a normalized well documented form...

fixup! pythongh-161714: Add internal Py_REF_ATOMIC_SWAP() and Py_REF_…

fixup! pythongh-161714: Use Py_REF_ATOMIC_SWAP() and Py_REF_ATOMIC_INC()

I built a prototype (cmaloney@56beb44) of two primitives that solve the cases:

Py_REF_ATOMIC_SWAP: Replace an existing reference with the new reference, work properly in contended swapping cases.
Py_REF_ATOMIC_INC: Get a strong reference when something else may be swapping.

They rely on the existing _PyObject_SetMaybeWeakref + Py_XDECREF / Py_XSETREF, and _Py_XGetRef adding in just a little more atomic to fix the edges in free threaded. That simplifies _warnings (#160401), my os.strerror cache (#161455). and iterobject (#161793). If code always uses Py_REF_ATOMIC_SWAP to set and clear then it can always use Py_REF_ATOMIC_INC and not need any other forms like PyUnstable_EnableTryIncRef which hopefully makes a little easier to write code.

Thoughts?

Navigation Menu

Repository navigation

Free-threading race betwen two Py_SETREF #161714

Bug report

CPython versions tested on:

Operating systems tested on:

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	static void
	update_state_ref(PyObject *ptr, PyObject value)
	{
	if (FT_ATOMIC_LOAD_PTR_ACQUIRE(*ptr) == value) {
	return;
	}
	PyUnstable_EnableTryIncRef(value);
	PyObject *old = _Py_atomic_exchange_ptr(ptr, Py_NewRef(value));
	Py_XDECREF(old);
	}

	/* Return a new reference to the warnings state field 'ptr'. /
	static PyObject *
	get_state_ref(PyObject **ptr)
	{
	#ifdef Py_GIL_DISABLED
	return _Py_XGetRef(ptr);
	#else
	return Py_XNewRef(*ptr);
	#endif
	}

Free-threading race betwen two Py_SETREF #161714

Description

Bug report

CPython versions tested on:

Operating systems tested on:

Activity

colesbury commented on Nov 9, 2026

cmaloney commented on Nov 10, 2026

vstinner commented on Nov 13, 2026

cmaloney commented on Nov 15, 2026

cmaloney commented on Nov 15, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions