Rely on stop-the-world and the GIL instead of a dedicated RW mutex.

This solution is much simpler, but will perform a little bit worse.
Instead of using a dedicated lock on the interpreter state, we can
simply use stop-the-world (and the GIL on the default build) to ensure that
no other threads can create pre-finalization callbacks concurrently.

Author: ZeroIntensity

Include/internal/pycore_interp_structs.h

@@ -971,10 +971,6 @@ struct _is {
 #  endif
 #endif
 
-    /* The "pre-finalization" lock, which protects against things like starting
-     * threads. The exclusive writer is only used when the interpreter finalizes. */
-    _PyRWMutex prefini_mutex;
-
     /* the initial PyInterpreterState.threads.head */
     _PyThreadStateImpl _initial_thread;
     // _initial_thread should be the last field of PyInterpreterState.

Modules/_threadmodule.c

@@ -176,13 +176,11 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle)
 }
 
 static void
-add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle, PyInterpreterState *interp)
+add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle)
 {
-    _PyRWMutex_RLock(&interp->prefini_mutex);
     HEAD_LOCK(&_PyRuntime);
     llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node);
     HEAD_UNLOCK(&_PyRuntime);
-    _PyRWMutex_RUnlock(&interp->prefini_mutex);
 }
 
 static void
@@ -197,16 +195,13 @@ clear_shutdown_handles(thread_module_state *state)
 }
 
 static void
-remove_from_shutdown_handles(ThreadHandle *handle, PyInterpreterState *interp)
+remove_from_shutdown_handles(ThreadHandle *handle)
 {
-    assert(interp != NULL);
-    _PyRWMutex_RLock(&interp->prefini_mutex);
     HEAD_LOCK(&_PyRuntime);
     if (handle->shutdown_node.next != NULL) {
         llist_remove(&handle->shutdown_node);
     }
     HEAD_UNLOCK(&_PyRuntime);
-    _PyRWMutex_RUnlock(&interp->prefini_mutex);
 }
 
 static ThreadHandle *
@@ -314,7 +309,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state)
         handle->mutex = (PyMutex){_Py_UNLOCKED};
         _PyEvent_Notify(&handle->thread_is_exiting);
         llist_remove(node);
-        remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
+        remove_from_shutdown_handles(handle);
     }
 }
 
@@ -347,7 +342,6 @@ thread_run(void *boot_raw)
 {
     struct bootstate *boot = (struct bootstate *) boot_raw;
     PyThreadState *tstate = boot->tstate;
-    PyInterpreterState *interp = tstate->interp;
 
     // Wait until the handle is marked as running
     PyEvent_Wait(&boot->handle_ready);
@@ -374,7 +368,7 @@ thread_run(void *boot_raw)
 
     _PyThreadState_Bind(tstate);
     PyEval_AcquireThread(tstate);
-    _Py_atomic_add_ssize(&interp->threads.count, 1);
+    _Py_atomic_add_ssize(&tstate->interp->threads.count, 1);
 
     PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs);
     if (res == NULL) {
@@ -392,13 +386,13 @@ thread_run(void *boot_raw)
 
     thread_bootstate_free(boot, 1);
 
-    _Py_atomic_add_ssize(&interp->threads.count, -1);
+    _Py_atomic_add_ssize(&tstate->interp->threads.count, -1);
     PyThreadState_Clear(tstate);
     _PyThreadState_DeleteCurrent(tstate);
 
 exit:
     // Don't need to wait for this thread anymore
-    remove_from_shutdown_handles(handle, interp);
+    remove_from_shutdown_handles(handle);
 
     _PyEvent_Notify(&handle->thread_is_exiting);
     ThreadHandle_decref(handle);
@@ -1877,12 +1871,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args,
         // 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, interp);
+        add_to_shutdown_handles(state, handle);
     }
 
     if (ThreadHandle_start(handle, func, args, kwargs) < 0) {
         if (!daemon) {
-            remove_from_shutdown_handles(handle, interp);
+            remove_from_shutdown_handles(handle);
         }
         return -1;
     }

Modules/atexitmodule.c

@@ -196,15 +196,9 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
         return NULL;
     }
 
-    PyInterpreterState *interp = _PyInterpreterState_GET();
-    assert(interp != NULL);
     struct atexit_state *state = get_atexit_state();
-    _PyRWMutex_RLock(&interp->prefini_mutex);
     // atexit callbacks go in a LIFO order
-    int res = PyList_Insert(state->callbacks, 0, callback);
-    _PyRWMutex_RUnlock(&interp->prefini_mutex);
-
-    if (res < 0)
+    if (PyList_Insert(state->callbacks, 0, callback) < 0)
     {
         Py_DECREF(callback);
         return NULL;

Python/ceval_gil.c

@@ -785,10 +785,8 @@ _PyEval_AddPendingCall(PyInterpreterState *interp,
     }
 
     PyMutex_Lock(&pending->mutex);
-    _PyRWMutex_RLock(&interp->prefini_mutex);
     _Py_add_pending_call_result result =
         _push_pending_call(pending, func, arg, flags);
-    _PyRWMutex_RUnlock(&interp->prefini_mutex);
     PyMutex_Unlock(&pending->mutex);
 
     if (main_only) {
@@ -919,7 +917,7 @@ clear_pending_handling_thread(struct _pending_calls *pending)
 }
 
 static int
-make_pending_calls_lock_held(PyThreadState *tstate)
+make_pending_calls(PyThreadState *tstate)
 {
     PyInterpreterState *interp = tstate->interp;
     struct _pending_calls *pending = &interp->ceval.pending;
@@ -977,15 +975,6 @@ make_pending_calls_lock_held(PyThreadState *tstate)
     return 0;
 }
 
-static int
-make_pending_calls(PyThreadState *tstate)
-{
-    _PyRWMutex_RLock(&tstate->interp->prefini_mutex);
-    int res = make_pending_calls_lock_held(tstate);
-    _PyRWMutex_RUnlock(&tstate->interp->prefini_mutex);
-    return res;
-}
-
 
 void
 _Py_set_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit)

Python/pylifecycle.c

@@ -2036,6 +2036,8 @@ make_pre_finalization_calls(PyThreadState *tstate)
      * could start a thread or vice versa. To ensure that we properly clean
      * call everything, we run these in a loop until none of them run anything. */
     for (;;) {
+        assert(!interp->runtime->stoptheworld.world_stopped);
+
         // Wrap up existing "threading"-module-created, non-daemon threads.
         wait_for_thread_shutdown(tstate);
 
@@ -2054,15 +2056,18 @@ make_pre_finalization_calls(PyThreadState *tstate)
 
         _PyAtExit_Call(tstate->interp);
 
-        _PyRWMutex_Lock(&tstate->interp->prefini_mutex);
+        // XXX Why does _PyThreadState_DeleteList() rely on all interpreters
+        // being stopped?
+        _PyEval_StopTheWorldAll(interp->runtime);
         int should_continue = (interp_has_threads(interp)
                               || interp_has_atexit_callbacks(interp)
                               || interp_has_pending_calls(interp));
-        _PyRWMutex_Unlock(&tstate->interp->prefini_mutex);
         if (!should_continue) {
             break;
         }
+        _PyEval_StartTheWorldAll(interp->runtime);
     }
+    assert(interp->runtime->stoptheworld.world_stopped);
 }
 
 static int
@@ -2099,7 +2104,7 @@ _Py_Finalize(_PyRuntimeState *runtime)
 #endif
 
     /* Ensure that remaining threads are detached */
-    _PyEval_StopTheWorldAll(runtime);
+    assert(tstate->interp->runtime->stoptheworld.world_stopped);
 
     /* Remaining daemon threads will be trapped in PyThread_hang_thread
        when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
@@ -2490,6 +2495,7 @@ Py_EndInterpreter(PyThreadState *tstate)
     /* Remaining daemon threads will automatically exit
        when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
     _PyInterpreterState_SetFinalizing(interp, tstate);
+    _PyEval_StartTheWorldAll(interp->runtime);
 
     // XXX Call something like _PyImport_Disable() here?