Sharing a dict iterator across threads double-DECREFs di_dict under free-threading

[devdanzin]

Crash report

What happened?

On a free-threaded build, advancing a single, shared dict iterator (iter({...})) from multiple threads double-frees the underlying dict and crashes.

All three dict iterators (dict_keyiterator / dict_valueiterator / dict_itemiterator) route next() through dictiter_iternext_threadsafe (Objects/dictobject.c). Its exhaustion path drops the iterator's owning reference to the dict:

fail:
    di->di_dict = NULL;   /* non-atomic clear */
    Py_DECREF(d);         /* drop the iterator's ONE owning ref to the dict */
    return -1;

and the caller reads that reference with no lock:

static PyObject*
dictiter_iternextkey(PyObject *self)
{
    dictiterobject *di = (dictiterobject *)self;
    PyDictObject *d = di->di_dict;      /* plain read */
    if (d == NULL)
        return NULL;
    PyObject *value;
    if (dictiter_iternext_threadsafe(d, self, &value, NULL) < 0) {
        value = NULL;
    }
    return value;
}

The iterator holds exactly one reference to the dict (di_dict). Two threads calling next() on the same near-exhausted iterator interleave as: both read d = di->di_dict (non-NULL), both reach fail:, and both run Py_DECREF(d). The second Py_DECREF has no matching reference — the dict's refcount underflows / it is freed one owner too early, and a sibling thread still walking d (and the dict freelist / keys object) then touches freed memory.

This fail: di->di_dict = NULL; Py_DECREF(d) pattern predates free-threading (it is correct under the GIL, where only one thread runs the iterator); the lock-free dictiter_iternext_threadsafe wrapper carried it over unguarded.

Reproducer

import threading

NT = 8
ITERS = 200_000

def newit():
    return iter({k: k for k in range(32)})

cell = [newit()]

def worker():
    for _ in range(ITERS):
        it = cell[0]
        try:
            next(it)                 # dictiter_iternextkey -> dictiter_iternext_threadsafe
        except StopIteration:
            cell[0] = newit()        # refill so the fail: (exhaustion) path is hit repeatedly
        except Exception:
            pass

threads = [threading.Thread(target=worker) for _ in range(NT)]
for t in threads: t.start()
for t in threads: t.join()
print("done, no crash")

PYTHON_GIL=0 ./python repro.py on a free-threaded build:

• Debug build → SIGABRT within seconds, ~8/8 runs, with _Py_NegativeRefcount on the dict (Objects/dictobject.c:6159), plus downstream corruption faces as the freed dict/keys object is reused (dictkeys_incref immortal-refcount assert :484; new_dict type assert :978; clear_freelist Objects/object.c:909; validate_refcounts in gc_free_threading.c).
• Release build (-O0, no sanitizer) → SIGSEGV (use-after-free), or occasionally Fatal Python error: PyMutex_Unlock: unlocking mutex that is not locked from the corrupted dict mutex.

So it is neither debug-only nor sanitizer-only. Debug backtrace (the negative-refcount object is the dict d):

#8  _PyObject_AssertFailed (obj=0x...dict...) at Objects/object.c:3278
#9  _Py_NegativeRefcount               at Objects/object.c:275
#12 Py_DECREF                          at ./Include/refcount.h:363
#13 dictiter_iternext_threadsafe (d=0x...dict...) at Objects/dictobject.c:6159    <-- fail: Py_DECREF(d)
#14 dictiter_iternextkey                at Objects/dictobject.c:5791
#15 builtin_next                        at Python/bltinmodule.c:1776

Suggested fix

Consume the reference atomically so exactly one thread performs the DECREF:

fail:
    PyDictObject *old = _Py_atomic_exchange_ptr(&di->di_dict, NULL);
    if (old != NULL) {
        Py_DECREF(old);
    }
    return -1;

and keep the dict alive for the duration of the lock-free walk (the caller uses d and its keys/values across the whole dictiter_iternext_threadsafe body) so a sibling that wins the exchange cannot free it mid-iteration — take a strong reference or the dict's critical section for the walk. One fix covers keys/values/items, since all three route through dictiter_iternext_threadsafe.

Why this is not the documented value-benign iterator race

This function was written to be shared across threads, and a crash is explicitly out of contract:

• The lock-free dict iterator dictiter_iternext_threadsafe was added by Make dict objects thread-safe in --disable-gil builds #112075 / PR gh-112075: Iterating a dict shouldn't require locks #115108 ("Iterating a dict shouldn't require locks"), under the umbrella Make dict objects thread-safe in --disable-gil builds #112075 ("Make dict objects thread-safe in --disable-gil builds"). PR gh-112075: Iterating a dict shouldn't require locks #115108 states it "[handles] races against the dict as well as allowing the iterator to be used from multiple threads simultaneously." It made the value read safe (_Py_TryIncrefCompare / acquire_key_value) but carried the old fail: di->di_dict = NULL; Py_DECREF(d) exhaustion path in unchanged — hence this double-free.
• Sequence iterator thread-safety #120496 ("Sequence iterator thread-safety") decided not to fix the fact that concurrent iteration can return duplicate/skipped values, and documented it instead (only the Doc/glossary.rst note, PR gh-120496: Add a note about iterator thread-safe #120685, merged; the code-fix PRs were closed). But the contract agreed on that issue is explicit — @colesbury and @eendebakpt: iterating from multiple threads "will not crash the interpreter" (that's the acceptable line; wrong values are OK, crashes are not), and @eendebakpt flagged "the risks of overflows inside the C code."
• Possible data race in dictiter_iternext with with free-threading build #148873 reported this iterator's data-race face and was closed as a duplicate of Sequence iterator thread-safety #120496 — folding a double-free into the value-benign class. That is the gap this issue closes: the same unsynchronized clear-and-DECREF is not benign — it double-frees the dict and crashes (negative refcount / UAF / SIGSEGV) on plain free-threaded builds, which Sequence iterator thread-safety #120496 / Strategy for Iterators in Free Threading #124397 put squarely on the not-acceptable side.

(Found by fusil --tsan, a ThreadSanitizer fuzzer; crash confirmed by re-running the reproducer without a sanitizer on a plain free-threaded build. Draft and reproducer by Claude Code, minimized and reviewed by hand.)

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.16.0a0 free-threading build (heads/main:a1d580430c8, Jul 18 2026, 20:23:36) [Clang 21.1.8 (6ubuntu1)]

[BHUVANSH855]

Hi! I've been investigating this issue and wanted to ask for some guidance on the intended fix.

I first tried implementing the suggested atomic exchange on the exhaustion path:

fail:
    PyDictObject *old = _Py_atomic_exchange_ptr(&di->di_dict, NULL);
    if (old != NULL) {
        Py_DECREF(old);
    }
    return -1;

This compiles successfully, but rebuilding CPython causes _bootstrap_python to abort during the freezing step with assertions in interpreter_clear() (cold->vm_data.valid / cold_dynamic->vm_data.valid), so it doesn't appear to be sufficient on its own.

While reading through dictiter_iternext_threadsafe(), I also noticed that the callers pass a borrowed PyDictObject *d = di->di_dict, and the helper dereferences d throughout the lock-free walk. The issue description mentions "keep the dict alive for the duration of the lock-free walk", but doesn't specify the intended mechanism.

Could you clarify what the preferred approach is here?

• Should the iterator acquire a temporary strong reference to the dict for the duration of dictiter_iternext_threadsafe()?
• Should the dict's critical section be used instead?
• Or is there another intended approach?

I'd appreciate any guidance before continuing with the implementation.

[devdanzin]

Thanks for taking a look at this! Are you building with the JIT? That error message seems to be the same in #154014. I reproduced this issue on a free-threaded non-JIT debug build (--disable-gil --with-assertions, no --enable-experimental-jit) and don't hit the freezing abort. If you're not using the JIT, could you share your exact configure options?

You're right that the atomic exchange alone isn't enough. The exchange fixes the double-DECREF (so the iterator's owning reference is dropped exactly once), but as you noticed, the caller reads d = di->di_dict as a borrowed pointer and the helper dereferences it throughout the lock-free walk, so a sibling thread that wins the exchange and drops the last reference can still free the dict out from under an in-flight walk (a use-after-free).

I think the cleanest fix keeps the lock-free forward-iteration design (the point of gh-112075 / #115108) by having the caller take a temporary strong reference for the duration of the walk:

• in dictiter_iternextkey / ...value / ...item, atomically load-and-incref di->di_dict (e.g. _Py_TryXGetRef) so it can't be freed mid-walk, pass that kept-alive d to dictiter_iternext_threadsafe, and Py_DECREF it after the call;
• keep the atomic-exchange in the fail: path so the iterator's owning reference is cleared and dropped exactly once.

That way the walk always operates on a dict kept alive by the temporary reference, and only one thread performs the owning-ref DECREF on exhaustion.

What do you think?

Drafted with help from Claude Code

[devdanzin]

Follow-up on the reproducer: the same dictiter_iternext_threadsafe double-DECREF is not limited to throwaway iter({...}). It hits any long-lived shared dict — and, because iter(frozendict) returns a dict_keyiterator, any shared frozendict (e.g. a module-level constant) too. On a long-lived object (refcount > 1) the double-DECREF doesn't go immediately negative — the corruption is silent until the next gc.collect() catches it:

import threading, gc

fd = frozendict({4:'FREE',1:'LOCAL',3:'GLOBAL_IMPLICIT',2:'GLOBAL_EXPLICIT',5:'CELL'})
NT = 8
cell = [iter(fd)]
def worker(role):
    for i in range(100_000):
        it = cell[0]
        try:
            next(it)
        except StopIteration:
            cell[0] = iter(fd)
        except Exception:
            pass
        if role == 2 and i % 32 == 0:
            gc.collect()
ts = [threading.Thread(target=worker, args=(i % 3,)) for i in range(NT)]
for t in ts: t.start()
for t in ts: t.join()

PYTHON_GIL=0 ./python repro.py → Python/gc_free_threading.c:1116: validate_gc_objects: Assertion "gc_get_refs(op) >= 0" failed: refcount is too small, object type frozendict, ~8/8 on a debug free-threaded build (SIGSEGV on release).

This is the more concerning form of the bug: a real program that keeps a shared module-level dict/frozendict and iterates it from several threads silently corrupts its refcount, surfacing as an unrelated GC crash. A ThreadSanitizer fuzzer (fusil --tsan) hit it across ~9 unrelated stdlib modules that each expose a module-level frozendict (symtable, functools, gettext, json, …) — the module is incidental; the shared frozendict iterator is the cause.

(Reproducer + note draft by Claude Code, reviewed by hand.)

[BHUVANSH855]

Thanks! That direction makes sense, and I agree that taking a temporary strong reference in the caller seems like the right approach.

I have one implementation question before I start coding it. _Py_TryXGetRef() returns NULL both when the pointer is actually NULL and when _Py_TryIncrefCompare() loses a race.

For the iterator callers (dictiter_iternextkey, dictiter_iternextvalue, and dictiter_iternextitem), what would be the preferred behavior if _Py_TryXGetRef((PyObject **)&di->di_dict) returns NULL due to that optimistic race?

Should the caller:

• retry,
• fall back to a locked path,
• or simply return NULL and let the next next() call retry?

I just want to make sure I follow the intended pattern rather than introducing a new one.

[devdanzin]

I think the ambiguity you spotted is exactly why the one-shot _Py_TryXGetRef is the wrong tool for the caller here.

Use _Py_XGetRef, not _Py_TryXGetRef. In Include/internal/pycore_object.h, _Py_TryXGetRef is the one-shot variant that returns NULL on either a NULL pointer or a lost _Py_TryIncrefCompare race. _Py_XGetRef is the for(;;) version that re-loads the pointer and retries the incref, returning NULL only when it actually observes di_dict == NULL. So it collapses your three options into the correct one automatically:

static PyObject*
dictiter_iternextkey(PyObject *self)
{
    dictiterobject *di = (dictiterobject *)self;
#ifdef Py_GIL_DISABLED
    // Loops past a lost optimistic incref; returns NULL only if di_dict is really NULL.
    PyDictObject *d = (PyDictObject *)_Py_XGetRef((PyObject **)&di->di_dict);
    if (d == NULL)
        return NULL;                 // genuine exhaustion -> StopIteration
    PyObject *value;
    if (dictiter_iternext_threadsafe(d, self, &value, NULL) < 0) {
        value = NULL;
    }
    Py_DECREF(d);                    // drop the temporary walk ref
    return value;
#else
    PyDictObject *d = di->di_dict;
    if (d == NULL)
        return NULL;
    return dictiter_iternextkey_lock_held(d, self);
#endif
}

A couple of reasons this is the right shape:

• Don't surface the race as a return-NULL. Returning NULL without an exception is StopIteration, so a NULL on a merely-lost optimistic race would end iteration early: a spurious StopIteration. Sequence iterator thread-safety #120496 explicitly tolerates imprecise results (duplicated/skipped values) but this would be a premature end of a non-exhausted iterator, which is a step worse than the imprecision that issue signed off on. _Py_XGetRef avoids it by only returning NULL for a real NULL.
• It matches the function's own precedent. When the value incref races today, dictiter_iternext_threadsafe doesn't spin ad-hoc or bail — acquire_key_value returns -1 and it does goto try_locked, finishing under Py_BEGIN_CRITICAL_SECTION(d) (dictobject.c ~6162). _Py_XGetRef is the equivalent "resolve the race rather than expose it" choice for the dict pointer. (For the di_dict read specifically, the internal retry is cleaner than a locked fallback, since we can't take d's critical section before we hold a live ref to it.)
• No livelock risk. di_dict is monotonic — set once when the iterator is created and only ever transitioned to NULL on exhaustion, never re-pointed — so _Py_XGetRef's loop terminates in a couple of iterations at most (either the incref wins, or the atomic exchange in the fail: path publishes NULL).

Ref accounting stays balanced: the iterator's single owning ref is consumed exactly once via the atomic exchange in fail:, and each caller's temporary _Py_XGetRef ref is dropped by the matching Py_DECREF(d) after the walk, which is also what keeps d alive for the duration.

One thing I'd want a core dev to confirm before you build on this (@colesbury, as the author of the lock-free iterator in gh-112075/#115108): _Py_XGetRef/_Py_TryXGetRef carry the precondition "the writer must set maybe-weakref on the stored object," so this isn't purely a read-site change — the site that stores di_dict (dictiter_new) and the atomic clear in fail: need to cooperate so the try-incref on di_dict is valid. I'm not certain whether the dict stored in di_dict already satisfies that or needs explicit setup, and getting it wrong reintroduces exactly the kind of race we're closing. I'd confirm that detail (and that a temporary strong-ref-per-next() is the preferred trade-off vs. a short critical section on the iterator itself) before finalizing.

Drafted with help from Claude Code