Skip to content

gh-154937: Fix _thread._shutdown() racing with _thread.start_joinable_thread on ThreadHandle.ident - #155085

Open
LindaSummer wants to merge 8 commits into
python:mainfrom
LindaSummer:fix/thread-ident-154937
Open

gh-154937: Fix _thread._shutdown() racing with _thread.start_joinable_thread on ThreadHandle.ident#155085
LindaSummer wants to merge 8 commits into
python:mainfrom
LindaSummer:fix/thread-ident-154937

Conversation

@LindaSummer

@LindaSummer LindaSummer commented Aug 2, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

Issue

Close #154937

Proposed Changes

The ThreadHandle.ident should be guarded by ThreadHandle.mutex as the below comment.

cpython/Modules/_threadmodule.c

Lines 132 to 136 in 083e038

// The `ident`, `os_handle`, `has_os_handle`, and `state` fields are
// protected by `mutex`.
PyThread_ident_t ident;
PyThread_handle_t os_handle;
int has_os_handle;

Root Cause

The thread.Thread will invoke _start_joinable_thread with the same flag as the invoke thread. The main thread is a non-daemon thread.

cpython/Lib/threading.py

Lines 1111 to 1147 in 083e038

def start(self):
"""Start the thread's activity.
It must be called at most once per thread object. It arranges for the
object's run() method to be invoked in a separate thread of control.
This method will raise a RuntimeError if called more than once on the
same thread object.
"""
if not self._initialized:
raise RuntimeError("thread.__init__() not called")
if self._started.is_set():
raise RuntimeError("threads can only be started once")
with _active_limbo_lock:
_limbo[self] = self
if self._context is None:
# No context provided
if _sys.flags.thread_inherit_context:
# start with a copy of the context of the caller
self._context = _contextvars.copy_context()
else:
# start with an empty context
self._context = _contextvars.Context()
try:
# Start joinable thread
_start_joinable_thread(self._bootstrap, handle=self._os_thread_handle,
daemon=self.daemon)
except Exception:
with _active_limbo_lock:
del _limbo[self]
raise
self._started.wait() # Will set ident and native_id

cpython/Lib/threading.py

Lines 1527 to 1537 in 083e038

class _MainThread(Thread):
def __init__(self):
Thread.__init__(self, name="MainThread", daemon=False)
self._started.set()
self._ident = _get_main_thread_ident()
self._os_thread_handle = _make_thread_handle(self._ident)
if _HAVE_THREAD_NATIVE_ID:
self._set_native_id()
with _active_limbo_lock:
_active[self._ident] = self

In _start_joinable_thread=>do_start_new_thread, the non-daemon thread will add to the shutdown_handles.

cpython/Modules/_threadmodule.c

Lines 1912 to 1917 in 083e038

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);
}

After being added to shutdown_handlers, do_start_new_thread will invoke ThreadHandle_start, where the racing happened.

cpython/Modules/_threadmodule.c

Lines 483 to 490 in 083e038

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

At the same time, _shutdown will read the ident for shutting down without a lock with the ThreadHandle.mutex.

cpython/Modules/_threadmodule.c

Lines 2403 to 2422 in 083e038

thread_shutdown(PyObject *self, PyObject *args)
{
PyThread_ident_t ident = PyThread_get_thread_ident_ex();
thread_module_state *state = get_thread_state(self);
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);

Comment

Referring to the above root cause analytics, I find the original test case may not be precise.

The original case in #154937 uses _thread.start_new_thread alongside threading.Thread.
The race was caused by threading.Thread not _thread.start_new_thread.

def chain2_thread():
    for _ in range(12000):
        try:
            _thread.start_new_thread(lambda: None, ())
        except Exception:
            pass
        time.sleep(0.0001)

...

threads += [Thread(target=chain2_thread) for _ in range(N_C2)]

So, I changed the original case to below with daemon=True, and the racing doesn't happen.

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, daemon=True) for _ in range(N_C1)]
threads += [Thread(target=chain2_thread, daemon=True) for _ in range(N_C2)]
for t in threads: t.start()
for t in threads: t.join()

I created another case that aligns with the PR's unit test, and the problem is reproduced again.

import _thread
from threading import Barrier

ITERATIONS = 1_000
phase = Barrier(2)


def shutdown_worker():
    phase.wait()
    for _ in range(ITERATIONS):
        try:
            _thread._shutdown()
        except RuntimeError:
            pass

def startup_worker():
    phase.wait()
    for _ in range(ITERATIONS):
        handle = _thread.start_joinable_thread(
            lambda: None,
            daemon=False,
        )
        handle.join()

startup_worker = _thread.start_joinable_thread(
    startup_worker,
    daemon=True,
)

shutdown_worker = _thread.start_joinable_thread(
    shutdown_worker,
    daemon=True,
)
shutdown_worker.join()
startup_worker.join()

Here is the TSAN report.

WARNING: ThreadSanitizer: data race (pid=2708706)
  Read of size 8 at 0x7fca36010070 by thread T2:
    #0 thread_shutdown /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:2416:22 (python3.16+0x653ba9)
    #1 cfunction_vectorcall_NOARGS /home/someuser/projects/cpython/cpython-upstream-main/Objects/methodobject.c:508:24 (python3.16+0x2ddf41)
    #2 _PyObject_VectorcallTstate /home/someuser/projects/cpython/cpython-upstream-main/./Include/internal/pycore_call.h:144:11 (python3.16+0x2240fc)
    #3 PyObject_Vectorcall /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:327:12 (python3.16+0x2240fc)
    #4 _Py_VectorCall_StackRefSteal /home/someuser/projects/cpython/cpython-upstream-main/Python/ceval.c:726:11 (python3.16+0x457e76)
    #5 _PyEval_EvalFrameDefault /home/someuser/projects/cpython/cpython-upstream-main/Python/generated_cases.c.h:4559:35 (python3.16+0x465a0c)
    #6 _PyEval_EvalFrame /home/someuser/projects/cpython/cpython-upstream-main/./Include/internal/pycore_ceval.h:122:16 (python3.16+0x457987)
    #7 _PyEval_Vector /home/someuser/projects/cpython/cpython-upstream-main/Python/ceval.c:2172:12 (python3.16+0x457987)
    #8 _PyFunction_Vectorcall /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c (python3.16+0x2247c7)
    #9 _PyVectorcall_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:273:16 (python3.16+0x224416)
    #10 _PyObject_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:348:16 (python3.16+0x224416)
    #11 PyObject_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:373:12 (python3.16+0x22448b)
    #12 thread_run /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:388:21 (python3.16+0x654aec)
    #13 pythread_wrapper /home/someuser/projects/cpython/cpython-upstream-main/Python/thread_pthread.h:234:5 (python3.16+0x574def)

  Previous write of size 8 at 0x7fca36010070 by thread T1:
    #0 ThreadHandle_start /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:486:17 (python3.16+0x654966)
    #1 do_start_new_thread /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:1919:9 (python3.16+0x6542ef)
    #2 thread_PyThread_start_joinable_thread /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:2042:14 (python3.16+0x653268)
    #3 cfunction_call /home/someuser/projects/cpython/cpython-upstream-main/Objects/methodobject.c:564:18 (python3.16+0x2de9c5)
    #4 _PyObject_MakeTpCall /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:242:18 (python3.16+0x2232b3)
    #5 _PyObject_VectorcallTstate /home/someuser/projects/cpython/cpython-upstream-main/./Include/internal/pycore_call.h:142:16 (python3.16+0x2241c4)
    #6 PyObject_Vectorcall /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:327:12 (python3.16+0x2241c4)
    #7 _Py_VectorCallInstrumentation_StackRefSteal /home/someuser/projects/cpython/cpython-upstream-main/Python/ceval.c:768:11 (python3.16+0x4586f4)
    #8 _PyEval_EvalFrameDefault /home/someuser/projects/cpython/cpython-upstream-main/Python/generated_cases.c.h:3474:35 (python3.16+0x463029)
    #9 _PyEval_EvalFrame /home/someuser/projects/cpython/cpython-upstream-main/./Include/internal/pycore_ceval.h:122:16 (python3.16+0x457987)
    #10 _PyEval_Vector /home/someuser/projects/cpython/cpython-upstream-main/Python/ceval.c:2172:12 (python3.16+0x457987)
    #11 _PyFunction_Vectorcall /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c (python3.16+0x2247c7)
    #12 _PyVectorcall_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:273:16 (python3.16+0x224416)
    #13 _PyObject_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:348:16 (python3.16+0x224416)
    #14 PyObject_Call /home/someuser/projects/cpython/cpython-upstream-main/Objects/call.c:373:12 (python3.16+0x22448b)
    #15 thread_run /home/someuser/projects/cpython/cpython-upstream-main/./Modules/_threadmodule.c:388:21 (python3.16+0x654aec)
    #16 pythread_wrapper /home/someuser/projects/cpython/cpython-upstream-main/Python/thread_pthread.h:234:5 (python3.16+0x574def)

@bedevere-app bedevere-app Bot mentioned this pull request Aug 2, 2026
Open
@LindaSummer

LindaSummer commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi @aisk ,

Could you help take a look at this patch at your convenience?

Thank you, and have a great day!

aisk
aisk reviewed Aug 2, 2026
View reviewed changes
Comment thread Lib/test/test_free_threading/test_threading.py Outdated
aisk
aisk reviewed Aug 2, 2026
View reviewed changes
Comment thread Lib/test/test_free_threading/test_threading.py Outdated
@LindaSummer

LindaSummer commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi @aisk ,

Thanks very much for your review and suggestions!

I've updated the patch based on the feedback.
When you have a chance, could you please take another look?

Have a wonderful day!

@kumaraditya303

kumaraditya303 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The data race is trivial, I suggest to not add such heavy test for it, you can remove the test.

@LindaSummer

LindaSummer commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

The data race is trivial, I suggest to not add such heavy test for it, you can remove the test.

Hi @kumaraditya303 ,

Thanks very much for your review!

I have removed the heavy 4*1000 threads unit test.

Please help take a look at your convenience.

Have a great day!

@kumaraditya303
kumaraditya303 enabled auto-merge (squash) August 3, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aisk aisk aisk approved these changes
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

3 participants

@LindaSummer @kumaraditya303 @aisk