Crash at finalization after fail to start new thread

[chgnrdv]

Crash report

What happened?

Bisected to e11fc03, but I guess this issue exists longer, and assertion that is added to PyThreadState_Clear by this commit just made it visible.

import resource
import threading

# this isn't essential, but helps PyThread_start_new_thread() fail
resource.setrlimit(resource.RLIMIT_NPROC, (150, 150))

while True:
    t = threading.Thread()
    t.start()
    t.join()

Error message with backtrace:

Traceback (most recent call last):
  File "/home/radislav/projects/cpython/thread_repro.py", line 10, in <module>
    t.start()
  File "/home/radislav/projects/cpython/Lib/threading.py", line 978, in start
    _start_new_thread(self._bootstrap, ())
RuntimeError: can't start new thread
python: Python/pystate.c:1484: PyThreadState_Clear: Assertion `tstate->_status.initialized && !tstate->_status.cleared' failed.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7c87537 in __GI_abort () at abort.c:79
#2  0x00007ffff7c8740f in __assert_fail_base (fmt=0x7ffff7dfe688 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=0x5555559c7be0 "tstate->_status.initialized && !tstate->_status.cleared", file=0x5555559c6e2f "Python/pystate.c", line=1484, function=<optimized out>)
    at assert.c:92
#3  0x00007ffff7c96662 in __GI___assert_fail (assertion=assertion@entry=0x5555559c7be0 "tstate->_status.initialized && !tstate->_status.cleared", 
    file=file@entry=0x5555559c6e2f "Python/pystate.c", line=line@entry=1484, function=function@entry=0x5555559c83e0 <__PRETTY_FUNCTION__.47> "PyThreadState_Clear")
    at assert.c:101
#4  0x000055555587075f in PyThreadState_Clear (tstate=tstate@entry=0x555555c4be00) at Python/pystate.c:1484
#5  0x0000555555871483 in _PyThreadState_DeleteExcept (tstate=tstate@entry=0x555555c03338 <_PyRuntime+508728>) at Python/pystate.c:1680
#6  0x000055555586afa4 in Py_FinalizeEx () at Python/pylifecycle.c:1831
#7  0x000055555589f6fc in Py_RunMain () at Modules/main.c:691
#8  0x000055555589f74b in pymain_main (args=args@entry=0x7fffffffe160) at Modules/main.c:719
#9  0x000055555589f7c0 in Py_BytesMain (argc=<optimized out>, argv=<optimized out>) at Modules/main.c:743
#10 0x00005555555cf74e in main (argc=<optimized out>, argv=<optimized out>) at ./Programs/python.c:15

When trying to start a new thread, Python creates new thread state by _PyThreadState_New call, adding this new state to list of thread states for current interpreter:

cpython/Modules/_threadmodule.c

Line 1191 in c32abf1

boot->tstate = _PyThreadState_New(interp);

If consequent call to PyThread_start_new_thread fails, this new state gets cleared, but remains in list:

cpython/Modules/_threadmodule.c

Lines 1203 to 1209 in c32abf1

	unsigned long ident = PyThread_start_new_thread(thread_run, (void*) boot);
	if (ident == PYTHREAD_INVALID_THREAD_ID) {
	PyErr_SetString(ThreadError, "can't start new thread");
	PyThreadState_Clear(boot->tstate);
	thread_bootstate_free(boot, 1);
	return NULL;
	}

Then, at Python finalization, call to _PyThreadState_DeleteExcept attempts to clear this thread state again, which causes assertion failure:

cpython/Python/pystate.c

Lines 1674 to 1682 in 3e8fcb7

	/* Clear and deallocate all stale thread states. Even if this
	executes Python code, we should be safe since it executes
	in the current thread, not one of the stale threads. */
	PyThreadState *p, *next;
	for (p = list; p; p = next) {
	next = p->next;
	PyThreadState_Clear(p);
	free_threadstate(p);
	}

cc @ericsnowcurrently

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.13.0a0 (heads/main:d4cea794a7, Sep 22 2023, 18:42:05) [GCC 10.2.1 20210110]

Linked PRs

• gh-109746: Make _thread.start_new_thread delete state of new thread on its startup failure #109761
• [3.13] gh-109746: Make _thread.start_new_thread delete state of new thread on its startup failure (GH-109761) #127171
• [3.12] gh-109746: Make _thread.start_new_thread delete state of new thread on its startup failure (GH-109761) #127173
• gh-109746: Fix race condition in test_start_new_thread_failed #127299
• [3.13] gh-109746: Fix race condition in test_start_new_thread_failed (GH-127299) #127323
• [3.12] gh-109746: Fix race condition in test_start_new_thread_failed (GH-127299) #127324

[chgnrdv]

Possible fix (the tests are green):

diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index 7692bacccc..071dc90a24 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -1204,6 +1204,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
     if (ident == PYTHREAD_INVALID_THREAD_ID) {
         PyErr_SetString(ThreadError, "can't start new thread");
         PyThreadState_Clear(boot->tstate);
+        PyThreadState_Delete(boot->tstate);
         thread_bootstate_free(boot, 1);
         return NULL;
     }
diff --git a/Python/pystate.c b/Python/pystate.c
index dcc6c11221..983937202b 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -1589,7 +1589,9 @@ tstate_delete_common(PyThreadState *tstate)
     if (tstate->_status.bound_gilstate) {
         unbind_gilstate_tstate(tstate);
     }
-    unbind_tstate(tstate);
+    if (tstate->_status.bound) {
+        unbind_tstate(tstate);
+    }
 
     // XXX Move to PyThreadState_Clear()?
     clear_datastack(tstate);

@ericsnowcurrently, is my analysis correct?