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
Bug report
Bug description:
handle_thread_shutdown_exception()readsinterp->threads.headunlocked, one line before it stops the world:The world is still running at 3830, since
make_pre_finalization_callsasserts!world_stoppedbefore callingwait_for_thread_shutdown. Any thread entering orleaving the interpreter is writing that same field under
HEAD_LOCK:add_threadstate/new_threadstateatpystate.c:1661on create,tstate_delete_commonatpystate.c:1936on exit. One side locks, the otherdoes not, and there is no happens-before between them.
Severity is low. The read is inside
assert(), so-DNDEBUGremoves it, and itonly runs on the error path of
wait_for_thread_shutdown:threading._shutdown()raised (3863), or
threadingwas never imported while an error was pending(3856). Worth noting that the TSan free-threading CI job does build with
--with-pydebugandTools/tsan/suppressions_free_threading.txthas no activesuppressions, 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:
170 of 170 runs report the race on
main@aa533dce22b, clang 20.1.8, x86_64Linux, no suppressions and no unrelated races. The gist's own script gets 50 of
50 on the same build. Dropping the
threading._shutdownmonkeypatch gives 20 of20 clean runs, which is the control for this being error-path-only.
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 afor (;;)loop, buthandle_thread_shutdown_exceptionrelabels everyWHENCE_THREADINGtstate to..._THREADING_DAEMON, andinterp_has_threads()counts onlyWHENCE_THREADING. If every thread predates finalization, the loop breaks afterone pass and the unlocked read runs exactly once.
HEAD_LOCKis aPyMutexbuilt on atomics, so TSan models it assynchronisation. Once the main thread takes
HEAD_LOCKagain, or any lock anexiting 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 spawnerkeeps the loop re-entering the handler and its own
HEAD_LOCKwrite landsmicroseconds 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 thenewest write before each read is almost always the creation-path one. Running
with
suppressions=race:new_threadstate,race:add_threadstatedrops thereproducer 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.
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs