Skip to content

Free-threading: data race on interp->threads.head in handle_thread_shutdown_exception (unlocked assert before _PyEval_StopTheWorld) #154822

Description

@zangjiucheng

Bug report

Bug description:

handle_thread_shutdown_exception() reads interp->threads.head unlocked, one line before it stops the world:

    PyInterpreterState *interp = tstate->interp;
    assert(interp->threads.head != NULL);   // 3830: unlocked read
    _PyEval_StopTheWorld(interp);           // 3831: protection starts here

The world is still running at 3830, since make_pre_finalization_calls asserts
!world_stopped before calling wait_for_thread_shutdown. Any thread entering or
leaving the interpreter is writing that same field under HEAD_LOCK:
add_threadstate / new_threadstate at pystate.c:1661 on create,
tstate_delete_common at pystate.c:1936 on exit. One side locks, the other
does not, and there is no happens-before between them.

Severity is low. The read is inside assert(), so -DNDEBUG removes it, and it
only runs on the error path of wait_for_thread_shutdown: threading._shutdown()
raised (3863), or threading was never imported while an error was pending
(3856). Worth noting that the TSan free-threading CI job does build with
--with-pydebug and Tools/tsan/suppressions_free_threading.txt has no active
suppressions, but regrtest does not take that path, so CI would not hit it by
accident.

Originally reported as TSAN-0034 in umbrella issue #153852 by @devdanzin
(gist), filed separately per the instructions there.

Reproducer

The gist's reproducer with the churn bounded, so the process always terminates:

import _thread, sys, threading, time

assert not sys._is_gil_enabled(), "need a --disable-gil build with PYTHON_GIL=0"

_go = threading.Event()

def _tiny():
    pass

def _churn():
    _go.wait()
    for _ in range(400):
        try:
            _thread.start_new_thread(_tiny, ())
        except RuntimeError:
            break

def _boom():
    raise RuntimeError("forced threading._shutdown() failure")

threading._shutdown = _boom

for _ in range(4):
    threading.Thread(target=_churn, daemon=True).start()
_go.set()
time.sleep(0.05)
# falling off the end -> Py_FinalizeEx -> wait_for_thread_shutdown -> _boom()
CC=clang ./configure --disable-gil --with-pydebug --with-thread-sanitizer
make -j"$(nproc)"
PYTHON_GIL=0 TSAN_OPTIONS="halt_on_error=0" ./python repro.py

170 of 170 runs report the race on main @ aa533dce22b, clang 20.1.8, x86_64
Linux, no suppressions and no unrelated races. The gist's own script gets 50 of
50 on the same build. Dropping the threading._shutdown monkeypatch gives 20 of
20 clean runs, which is the control for this being error-path-only.

WARNING: ThreadSanitizer: data race
  Read of size 8 by main thread:
    #0 handle_thread_shutdown_exception  Python/pylifecycle.c:3830
    #1 make_pre_finalization_calls       Python/pylifecycle.c:3863
    #2 _Py_Finalize                      Python/pylifecycle.c:2403
  Previous write of size 8 by thread T1:
    #0 new_threadstate                   Python/pystate.c:1661
    #1 _PyThreadState_New                Python/pystate.c:1741
    #2 do_start_new_thread               Modules/_threadmodule.c:458
  Location is global '_PyRuntime'
SUMMARY: ThreadSanitizer: data race Python/pylifecycle.c:3830 in handle_thread_shutdown_exception
Why the churn is needed: a one-shot thread pool never fires (0 hits in 170 runs)

Start N threads, let them die, then finalize, and TSan reports nothing at all. I
measured that across 12 thread-count and jitter configurations before working out
why. Two reasons:

make_pre_finalization_calls (pylifecycle.c:2279) is a for (;;) loop, but
handle_thread_shutdown_exception relabels every WHENCE_THREADING tstate to
..._THREADING_DAEMON, and interp_has_threads() counts only
WHENCE_THREADING. If every thread predates finalization, the loop breaks after
one pass and the unlocked read runs exactly once.

HEAD_LOCK is a PyMutex built on atomics, so TSan models it as
synchronisation. Once the main thread takes HEAD_LOCK again, or any lock an
exiting thread released after its write, that write is ordered before the read
and there is nothing left to report.

Threads created during finalization are freshly WHENCE_THREADING, so a spawner
keeps the loop re-entering the handler and its own HEAD_LOCK write lands
microseconds before the next read.

That is also why I could not get TSan to attribute a run to the exit-path write
at pystate.c:1936: the loop only continues because a thread was created, so the
newest write before each read is almost always the creation-path one. Running
with suppressions=race:new_threadstate,race:add_threadstate drops the
reproducer to 0/10. The exit-path pairing looks sound on paper, but I have no
measurement of it.

Suggested fix

Move the assert under stop-the-world, where the rest of the function already
lives. It still says something useful there: the very next statement walks that
list, and the function's own comment already leans on that invariant.

     PyInterpreterState *interp = tstate->interp;
-    assert(interp->threads.head != NULL);
     _PyEval_StopTheWorld(interp);
+    assert(interp->threads.head != NULL);

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

Metadata

Metadata

Assignees

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions