Data race on `ThreadHandle.ident` between `thread_shutdown` and `ThreadHandle_start` under free-threading

[Naserume]

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):

cpython/Modules/_threadmodule.c

Lines 483 to 490 in 22a6c51

	// 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)):

cpython/Modules/_threadmodule.c

Lines 2408 to 2422 in 22a6c51

	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.

cpython/Modules/_threadmodule.c

Lines 1912 to 1924 in 22a6c51

	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

[LindaSummer]

Hi Team,
I want to have a try on this issue.

[ayishaatwork]

I'd like to work on this issue if it's still available.

My plan is to reproduce the reported data race on a free-threaded build, understand the synchronization around ThreadHandle.ident, discuss the proposed fix if needed, and then submit a PR with tests where appropriate.

If no one is already working on it, could I be assigned to this issue?