Naserume · GitHub

Bug report

Bug description:

On a free-threaded build, ThreadHandle.ident is written and read under two different locks.

ThreadHandle_start sets self->ident while holding the handle's own mutex (self->mutex):

// Mark the handle running
PyMutex_Lock(&self->mutex);
assert(self->state == THREAD_HANDLE_STARTING);
self->ident = ident;
self->has_os_handle = 1;
self->os_handle = os_handle;
self->state = THREAD_HANDLE_RUNNING;
PyMutex_Unlock(&self->mutex);

thread_shutdown reads cur->ident while holding the runtime head lock (HEAD_LOCK(&_PyRuntime)):

for (;;) {
ThreadHandle *handle = NULL;
// Find a thread that's not yet finished.
HEAD_LOCK(&_PyRuntime);
struct llist_node *node;
llist_for_each_safe(node, &state->shutdown_handles) {
ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node);
if (cur->ident != ident) {
ThreadHandle_incref(cur);
handle = cur;
break;
}
}
HEAD_UNLOCK(&_PyRuntime);

Because the write is guarded by self->mutex and the read by HEAD_LOCK, the two do not exclude each other.

if (!daemon) {
// Add the handle before starting the thread to avoid adding a handle
// to a thread that has already finished (i.e. if the thread finishes
// before the call to `ThreadHandle_start()` below returns).
add_to_shutdown_handles(state, handle);
}
if (ThreadHandle_start(handle, func, args, kwargs, daemon) < 0) {
if (!daemon) {
remove_from_shutdown_handles(handle);
}
return -1;
}

The comment explains the handle is added early to avoid missing a thread that finishes quickly, but that is exactly what exposes ident to the HEAD_LOCK-side reader in thread_shutdown before it is written under self->mutex.

Reproducer:

import _thread
import time
from threading import Thread
def chain1_thread():
    for _ in range(12000):
        try:
            _thread._shutdown()
        except Exception:
            pass
def chain2_thread():
    for _ in range(12000):
        try:
            _thread.start_new_thread(lambda: None, ())
        except Exception:
            pass
        time.sleep(0.0001)
N_C1 = 1
N_C2 = 8
threads  = [Thread(target=chain1_thread) for _ in range(N_C1)]
threads += [Thread(target=chain2_thread) for _ in range(N_C2)]
for t in threads: t.start()
for t in threads: t.join()

TSAN Report:

WARNING: ThreadSanitizer: data race (pid=657186)
  Read of size 8 at 0x7fffb40a0570 by thread T1:
    #0 thread_shutdown /cpython/./Modules/_threadmodule.c:2416:22
    #1 cfunction_vectorcall_NOARGS /cpython/Objects/methodobject.c:508:24
    #2 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:144:11
    #3 PyObject_Vectorcall /cpython/Objects/call.c:327:12
    #4 _Py_VectorCall_StackRefSteal /cpython/Python/ceval.c:726:11
    #5 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:4559:35
  Previous write of size 8 at 0x7fffb40a0570 by main thread:
    #0 ThreadHandle_start /cpython/./Modules/_threadmodule.c:486:17
    #1 do_start_new_thread /cpython/./Modules/_threadmodule.c:1919:9
    #2 thread_PyThread_start_joinable_thread /cpython/./Modules/_threadmodule.c:2042:14
    #3 cfunction_call /cpython/Objects/methodobject.c:564:18
    #4 _PyObject_MakeTpCall /cpython/Objects/call.c:242:18
    #5 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:142:16
    #6 PyObject_Vectorcall /cpython/Objects/call.c:327:12
    #7 _Py_VectorCall_StackRefSteal /cpython/Python/ceval.c:726:11
    #8 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:3686:35
SUMMARY: ThreadSanitizer: data race /cpython/./Modules/_threadmodule.c:2416:22 in thread_shutdown

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

Read the original on github.com ↗