gh-126914: Use an atomic field for determining if the initial thread can be used

Include/internal/pycore_interp.h

@@ -139,6 +139,11 @@ struct _is {
            or the size specified by the THREAD_STACK_SIZE macro. */
         /* Used in Python/thread.c. */
         size_t stacksize;
+        /*
+         * Whether the _initial_thread has been used.
+         * This should only be accessed atomically.
+         */
+        int used_initial;
     } threads;
 
     /* Reference to the _PyRuntime global variable. This field exists

Lib/test/test_interpreters/test_stress.py

@@ -32,6 +32,24 @@ def task():
         with threading_helper.start_threads(threads):
             pass
 
+    @support.requires_resource('cpu')
+    def test_many_threads_to_same_interp(self):
+        # GH-126914: The initial thread of a subinterpreter could be accessed
+        # concurrently in new_threadstate() while it was finalizing.
+        interp = interpreters.create()
+
+        def run():
+            this_interp = interpreters.create()
+            this_interp.exec(f"import _interpreters; _interpreters.run_string({interp.id}, '1')")
+
+
+        threads = (threading.Thread(target=run) for _ in range(100))
+        with threading_helper.catch_threading_exception() as cm:
+            with threading_helper.start_threads(threads):
+                pass
+
+            self.assertIsNone(cm.exc_value)
+
 
 if __name__ == '__main__':
     # Test needs to be a package, so we can do relative imports.

Misc/NEWS.d/next/Core_and_Builtins/2024-11-16-17-42-36.gh-issue-126914.smTnX0.rst

@@ -0,0 +1,2 @@
+Fix crash when trying to concurrently create a thread state for a
+subinterpreter.

Python/pystate.c

@@ -1399,13 +1399,17 @@ alloc_threadstate(void)
 static void
 free_threadstate(_PyThreadStateImpl *tstate)
 {
+    PyInterpreterState *interp = tstate->base.interp;
     // The initial thread state of the interpreter is allocated
     // as part of the interpreter state so should not be freed.
-    if (tstate == &tstate->base.interp->_initial_thread) {
+    if (tstate == &interp->_initial_thread) {
         // Restore to _PyThreadState_INIT.
         memcpy(tstate,
                &initial._main_interpreter._initial_thread,
                sizeof(*tstate));
+        _Py_atomic_store_int(
+                            &interp->threads.used_initial,
+                            0);
     }
     else {
         PyMem_RawFree(tstate);
@@ -1526,18 +1530,30 @@ new_threadstate(PyInterpreterState *interp, int whence)
     interp->threads.next_unique_id += 1;
     uint64_t id = interp->threads.next_unique_id;
 
+    /*
+     * GH-126914: Using the initial thread is a bit of a headache
+     * in terms of thread safety. Originally, this function just checked
+     * if interp->threads.head was NULL, but since that gets set
+     * before the thread is fully cleaned up, it's possible
+     * for it to still be in use while interp->threads.head is NULL.
+     *
+     * So, we have a dedicated atomic field to make sure that only one
+     * thread accesses it at a time.
+     */
+    int used_initial = _Py_atomic_load_int_relaxed(&interp->threads.used_initial);
+
     // Allocate the thread state and add it to the interpreter.
     PyThreadState *old_head = interp->threads.head;
-    if (old_head == NULL) {
-        // It's the interpreter's initial thread state.
+    if (used_initial == 0) {
+        // The initial thread state is not in use.
+        _Py_atomic_store_int_relaxed(&interp->threads.used_initial, 1);
         used_newtstate = 0;
         tstate = &interp->_initial_thread;
     }
-    // XXX Re-use interp->_initial_thread if not in use?
     else {
         // Every valid interpreter must have at least one thread.
         assert(id > 1);
-        assert(old_head->prev == NULL);
+        assert(old_head == NULL || old_head->prev == NULL);
         used_newtstate = 1;
         tstate = new_tstate;
         // Set to _PyThreadState_INIT.