GeezerVison is showing github.com — `_Py_SetImmortal` must be run on allocating thread (no-gil) · Issue #113956 · python/cpython
_Py_SetImmortal must be run on allocating thread (no-gil) #113956
DescriptionBug report
Bug description:
The code in _Py_SetImmortal must be run on the thread that originally allocated the object. If this is not the case, then a race between Py_INCREF can cause the _Py_SetImmortal to be ignored.
|
static inline void _Py_SetImmortal(PyObject *op) |
|
{ |
|
if (op) { |
|
#ifdef Py_GIL_DISABLED |
|
op->ob_tid = _Py_UNOWNED_TID; |
|
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL; |
|
op->ob_ref_shared = 0; |
|
#else |
|
op->ob_refcnt = _Py_IMMORTAL_REFCNT; |
|
#endif |
|
} |
|
} |
Example sequence:
- Allocating thread begins an
_Py_Incref.
- Allocating thread detects it should perform inc ref on
ob_ref_local, and calculates new value.
- Non-allocating thread runs all of
_Py_SetImmortal.
- Allocating thread completes incref with assignment to
ob_ref_local, and unsets the value from immortal.
In the nogil fork, there is some defensive code on other operations that need to be run on the allocating thread, e.g.
_PyObject_SetDeferredRefcount(PyObject *op)
{
assert(_Py_ThreadLocal(op) && "non thread-safe");https://github.com/colesbury/nogil-3.12/blob/cedde4f5ec3759ad723c89d44738776f362df564/Include/internal/pycore_object.h#L361C19-L361C19
I think something like this should occur inside _Py_SetImmortal.
It looks like _Py_SetImmortal is not exposed outside the runtime. However, PEP 683, says:
Then setting any object’s refcount to _Py_IMMORTAL_REFCNT makes it immortal.
This implies there is an intent to expose it, and with the changes to nogil. It seems that this should be exposed as an explicit operation, rather than by setting the ref count directly.
CC @ericsnowcurrently @colesbury
P.S. I am happy to PR any required changes.
CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
Activitycolesbury commented on Jan 11, 2024 A few thoughts:
- Yes, if the owning thread calls
Py_INCREF() concurrently with another thread trying to immortalize the object, that will result in a race condition
_Py_SetImmortal() is not public and I don't think we should make it public (at least not in 3.13)- The check that the caller is the owning thread is conservative. It's also safe to immortalize objects in certain other cases, such as during a stop-the-world event.
colesbury commented on Jan 11, 2024 I would be hesitant to add an assertion to _Py_SetImmortal() because we call it from a number of places where we already know we are in a bad state, like none_dealloc.
The only call I've seen that concerns me is in _PyUnicode_InternInPlace(). In the free-threaded builds, we should probably only immortalize the string if it is owned by the current thread. mjp41 commented on Jan 12, 2024 @colesbury thanks for the reply. This makes sense. I had thought this is more externally reachable.
I think you could make it concurrency safe by borrowing a third bit from ob_ref_shared to represent Immortal. colesbury commented on Jan 12, 2024
I think you could make it concurrency safe by borrowing a third bit from ob_ref_shared to represent Immortal.
Yeah, that would address concurrency safety, but using ob_ref_local serves two other purposes:
- It let's us make
ob_ref_local 32-bits without having to worry about overflow, because the immortal value is UINT32_MAX.
- It makes the common cases for
Py_INCREF and Py_DECREF faster because we only need to access ob_tid and ob_ref_local.
Reacted by Matthew Parkinson devdanzin commented on Apr 10, 2026 Adding a concrete reproducer for this race. The scenario @mjp41 described and @colesbury confirmed above is reachable from pure-Python code via concurrent setattr() on shared objects with fresh string keys — exactly the _PyUnicode_InternInPlace caller Sam flagged as the concerning one.
Reproducer (pure stdlib, no extensions)
import os
import signal
import sys
import threading
import time
N_THREADS = 8
N_ITERS = 500
N_KEYS_PER_ITER = 5
SCENARIO_TIMEOUT = 120
class Target:
pass
def scenario():
for i in range(N_ITERS):
target = Target()
# Fresh strings via concatenation — NOT bytecode literals, so they
# aren't already interned-immortal when the threads start.
keys = [
"tsan_intern_race_probe_" + str(i) + "_" + str(j)
for j in range(N_KEYS_PER_ITER)
]
values = list(range(N_KEYS_PER_ITER))
barrier = threading.Barrier(N_THREADS)
def worker():
barrier.wait()
for k, v in zip(keys, values):
try:
setattr(target, k, v)
except Exception:
pass
threads = [threading.Thread(target=worker) for _ in range(N_THREADS)]
for t in threads:
t.start()
for t in threads:
t.join()
# Run in a forked subprocess so a crash doesn't take down the parent.
pid = os.fork()
if pid == 0:
try:
scenario()
os._exit(0)
except Exception:
os._exit(1)
deadline = time.monotonic() + SCENARIO_TIMEOUT
status = None
while time.monotonic() < deadline:
r, s = os.waitpid(pid, os.WNOHANG)
if r != 0:
status = s
break
time.sleep(0.1)
if status is None:
os.kill(pid, signal.SIGKILL)
os.waitpid(pid, 0)
print("TIMEOUT")
elif os.WIFSIGNALED(status):
print(f"CRASH ({signal.Signals(os.WTERMSIG(status)).name})")
elif os.WIFEXITED(status) and os.WEXITSTATUS(status) != 0:
print(f"FAIL (exit {os.WEXITSTATUS(status)})")
else:
print("OK")Observed results
Reproduced on two independent free-threaded debug builds, in every run tried (4 runs total, 2 per build):
| Build |
Commit |
TSan |
Result |
| 3.14 debug FT + TSan |
heads/3.14-dirty:ea9ecc8955a, Apr 6 2026 |
yes |
11 TSan data races + SIGABRT at Objects/unicodeobject.c:778 |
| 3.15 debug FT + ASAN (no TSan) |
heads/main:4d0e8ee649c, Mar 29 2026 |
no |
SIGABRT at Objects/unicodeobject.c:691 (same check; file reorganized) |
Both builds: Clang 21.1.2, Linux x86_64, --disable-gil --with-pydebug. The 3.14 build additionally has --with-thread-sanitizer; the 3.15 build has -fsanitize=address.
The SIGABRT in both cases is the debug consistency check at _PyUnicode_CheckConsistency:
Objects/unicodeobject.c:778: _PyUnicode_CheckConsistency:
Assertion failed: !_Py_IsImmortal(((PyObject*)((op))))
object address : 0x7fffb4d1ae90
object refcount : 3221225472
object type : <class 'str'>
object repr : 'tsan_intern_race_probe_0_4'
Fatal Python error: _PyObject_AssertFailed
The object's ob_ref_local has been set to _Py_IMMORTAL_REFCNT_LOCAL (3221225472 = 0xC0000000) by one thread, while another thread is observing it in the SSTATE_INTERNED_MORTAL state — the invariant the consistency check enforces.
TSan on the 3.14 build flags the underlying race directly. Writer (Thread T8, inside intern_common after LOCK_INTERNED):
WARNING: ThreadSanitizer: data race
Write of size 4 at 0x7fffb4d1ab1c by thread T8:
#0 _Py_SetImmortalUntracked Objects/object.c:2687:22
#1 _Py_SetImmortal Objects/object.c:2704:5
#2 immortalize_interned Objects/unicodeobject.c:15957:5
#3 intern_common Objects/unicodeobject.c:16106:9
#4 _PyUnicode_InternMortal Objects/unicodeobject.c:16123:10
#5 PyObject_SetAttr Objects/object.c:1452:5
#6 builtin_setattr_impl Python/bltinmodule.c:1731:9
...
Reader (Thread T1, still in PyObject_SetAttr before entering intern_common):
Previous atomic read of size 4 at 0x7fffb4d1ab1c by thread T1:
#0 _Py_atomic_load_uint32_relaxed Include/cpython/pyatomic_gcc.h:367:10
#1 _Py_IsImmortal Include/refcount.h:128:13
#2 Py_INCREF Include/refcount.h:299:10
#3 PyObject_SetAttr Objects/object.c:1449:5
#4 builtin_setattr_impl Python/bltinmodule.c:1731:9
...
Same address (ob_ref_local of the interned string), same step sequence @mjp41 described in the original post: one thread is mid-_Py_SetImmortal, another thread's Py_INCREF is concurrently reading the field.
Notes
- Not reproducible (user-visibly) on a release FT build — the consistency check is compiled out. The underlying data race still exists and TSan still flags it.
- Not reproducible on a GIL build (serialized access to the interning path).
- The 3.15 reproduction is with ASAN, no TSan — so the SIGABRT is not a TSan instrumentation artifact. It's a natural consequence of the program flow violating the
SSTATE_INTERNED_MORTAL → !immortal invariant.
- Both test builds include the reordering from gh-146041: Avoid lock in sys.intern() for already interned strings #146072 (merged 2026-03-24), which moved
_Py_SetImmortal(s) to execute before the atomic publication of SSTATE_INTERNED_IMMORTAL. The comment added in that PR explicitly documents the ordering as the synchronization point for intern_common's lock-free fast path — which makes the race window visible in the source. I don't think gh-146041: Avoid lock in sys.intern() for already interned strings #146072 introduced the race (the Py_INCREF at PyObject_SetAttr:1449 has always been concurrent with immortalize_interned under the lock), but the new layout makes it easier to reason about.
- The comment in
_PyUnicode_CheckConsistency (Objects/unicodeobject.c:760-763 on 3.14, :675-676 on 3.15) calls the "object observed in INTERNED_MORTAL state turns out to be immortal" case "extremely unlikely (worth a potential failed assertion in a debug build)." The reproducer above suggests "extremely unlikely" is too strong — 8 threads and a fresh key per iteration is enough to hit it on first try.
No fix proposed — I'm not familiar enough with the FT internals to judge between making the writes atomic, relaxing the debug assertion, or requiring the owning-thread discipline Matthew originally suggested. Happy to provide full TSan output or test any candidate fix.
Found while investigating TSan results from a C extension (wrapt) using labeille and ft-review-toolkit; isolated to pure stdlib after ruling out the extension's own races.
Root cause analysis, reproducer creation and comment draft done with assistance from Claude Code, reviewed by a human. ngoldbaum commented on Apr 21, 2026 @colesbury it seems a test in the CFFI thread sanitizer CI is able to reliably trigger a race related to this, see python-cffi/cffi#242. I'm not able to reproduce this race on 3.15. It also seems to have started happening relatively recently. Maybe something was included in a 3.14 backport that makes triggering this easier?
Since it's fixed in 3.15 and appears to be harmless for CFFI, my plan right now for CFFI is to just add a suppression for this. colesbury commented on Apr 21, 2026 It's not fully fixed in 3.15 either Reacted by Nathan Goldbaum colesbury commented on Apr 23, 2026 I'm closing the issue now that:
sys.intern, etc. are now thread-safe in the way that they use _Py_SetImmortal- The other internal uses are safe, I believe
- The public C API
PyUnstable_SetImmortal is limited to _PyObject_IsUniquelyReferenced()
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|