Use an RW lock instead of a counter.

Author: ZeroIntensity

Include/internal/pycore_interp_structs.h

@@ -971,6 +971,10 @@ 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.

Include/internal/pycore_pylifecycle.h

@@ -85,7 +85,7 @@ extern void _PyErr_DisplayException(PyObject *file, PyObject *exc);
 
 extern void _PyThreadState_DeleteCurrent(PyThreadState *tstate);
 
-extern int _PyAtExit_Call(PyInterpreterState *interp);
+extern void _PyAtExit_Call(PyInterpreterState *interp);
 
 extern int _Py_IsCoreInitialized(void);
 

Lib/threading.py

@@ -1573,7 +1573,7 @@ def _shutdown():
         _main_thread._os_thread_handle._set_done()
 
     # Wait for all non-daemon threads to exit.
-    return _thread_shutdown()
+    _thread_shutdown()
 
 
 def main_thread():

Modules/_threadmodule.c

@@ -176,11 +176,13 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle)
 }
 
 static void
-add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle)
+add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle, PyInterpreterState *interp)
 {
+    _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
@@ -195,13 +197,16 @@ clear_shutdown_handles(thread_module_state *state)
 }
 
 static void
-remove_from_shutdown_handles(ThreadHandle *handle)
+remove_from_shutdown_handles(ThreadHandle *handle, PyInterpreterState *interp)
 {
+    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 *
@@ -309,7 +314,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);
+        remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
     }
 }
 
@@ -392,7 +397,7 @@ thread_run(void *boot_raw)
 
 exit:
     // Don't need to wait for this thread anymore
-    remove_from_shutdown_handles(handle);
+    remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
 
     _PyEvent_Notify(&handle->thread_is_exiting);
     ThreadHandle_decref(handle);
@@ -1863,12 +1868,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);
+        add_to_shutdown_handles(state, handle, interp);
     }
 
     if (ThreadHandle_start(handle, func, args, kwargs) < 0) {
         if (!daemon) {
-            remove_from_shutdown_handles(handle);
+            remove_from_shutdown_handles(handle, _PyInterpreterState_GET());
         }
         return -1;
     }
@@ -2345,7 +2350,6 @@ thread_shutdown(PyObject *self, PyObject *args)
 {
     PyThread_ident_t ident = PyThread_get_thread_ident_ex();
     thread_module_state *state = get_thread_state(self);
-    int found_thread = 0;
 
     for (;;) {
         ThreadHandle *handle = NULL;
@@ -2354,7 +2358,6 @@ thread_shutdown(PyObject *self, PyObject *args)
         HEAD_LOCK(&_PyRuntime);
         struct llist_node *node;
         llist_for_each_safe(node, &state->shutdown_handles) {
-            found_thread = 1;
             ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node);
             if (cur->ident != ident) {
                 ThreadHandle_incref(cur);
@@ -2375,13 +2378,13 @@ thread_shutdown(PyObject *self, PyObject *args)
             PyErr_FormatUnraisable("Exception ignored while joining a thread "
                                    "in _thread._shutdown()");
             ThreadHandle_decref(handle);
-            return PyBool_FromLong(found_thread);
+            Py_RETURN_NONE;
         }
 
         ThreadHandle_decref(handle);
     }
 
-    return PyBool_FromLong(found_thread);
+    Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(shutdown_doc,

Modules/atexitmodule.c

@@ -99,7 +99,7 @@ _PyAtExit_Fini(PyInterpreterState *interp)
     }
 }
 
-static int
+static void
 atexit_callfuncs(struct atexit_state *state)
 {
     assert(!PyErr_Occurred());
@@ -112,13 +112,10 @@ atexit_callfuncs(struct atexit_state *state)
     {
         PyErr_FormatUnraisable("Exception ignored while "
                                "copying atexit callbacks");
-        return 0;
+        return;
     }
 
-    int called = 0;
-
     for (Py_ssize_t i = 0; i < PyList_GET_SIZE(copy); ++i) {
-        called = 1;
         // We don't have to worry about evil borrowed references, because
         // no other threads can access this list.
         PyObject *tuple = PyList_GET_ITEM(copy, i);
@@ -144,15 +141,14 @@ atexit_callfuncs(struct atexit_state *state)
     atexit_cleanup(state);
 
     assert(!PyErr_Occurred());
-    return called;
 }
 
 
-int
+void
 _PyAtExit_Call(PyInterpreterState *interp)
 {
     struct atexit_state *state = &interp->atexit;
-    return atexit_callfuncs(state);
+    atexit_callfuncs(state);
 }
 
 
@@ -200,9 +196,15 @@ 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
-    if (PyList_Insert(state->callbacks, 0, callback) < 0)
+    int res = PyList_Insert(state->callbacks, 0, callback);
+    _PyRWMutex_RUnlock(&interp->prefini_mutex);
+
+    if (res < 0)
     {
         Py_DECREF(callback);
         return NULL;

Python/ceval_gil.c

@@ -785,8 +785,10 @@ _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) {
@@ -837,11 +839,10 @@ handle_signals(PyThreadState *tstate)
 }
 
 static int
-_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_called)
+_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending)
 {
     int res = 0;
     int32_t npending = -1;
-    int called = 0;
 
     assert(sizeof(pending->max) <= sizeof(size_t)
             && ((size_t)pending->max) <= Py_ARRAY_LENGTH(pending->calls));
@@ -869,8 +870,6 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_
             break;
         }
 
-        called = 1;
-
         /* having released the lock, perform the callback */
         res = func(arg);
         if ((flags & _Py_PENDING_RAWFREE) && arg != NULL) {
@@ -884,7 +883,6 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_
 
 finally:
     *p_npending = npending;
-    *p_called = called;
     return res;
 }
 
@@ -921,7 +919,7 @@ clear_pending_handling_thread(struct _pending_calls *pending)
 }
 
 static int
-make_pending_calls_with_count(PyThreadState *tstate)
+make_pending_calls_lock_held(PyThreadState *tstate)
 {
     PyInterpreterState *interp = tstate->interp;
     struct _pending_calls *pending = &interp->ceval.pending;
@@ -951,8 +949,7 @@ make_pending_calls_with_count(PyThreadState *tstate)
     unsignal_pending_calls(tstate, interp);
 
     int32_t npending;
-    int called;
-    if (_make_pending_calls(pending, &npending, &called) != 0) {
+    if (_make_pending_calls(pending, &npending) != 0) {
         clear_pending_handling_thread(pending);
         /* There might not be more calls to make, but we play it safe. */
         signal_pending_calls(tstate, interp);
@@ -963,9 +960,8 @@ make_pending_calls_with_count(PyThreadState *tstate)
         signal_pending_calls(tstate, interp);
     }
 
-    int main_called = 0;
     if (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)) {
-        if (_make_pending_calls(pending_main, &npending, &main_called) != 0) {
+        if (_make_pending_calls(pending_main, &npending) != 0) {
             clear_pending_handling_thread(pending);
             /* There might not be more calls to make, but we play it safe. */
             signal_pending_calls(tstate, interp);
@@ -978,17 +974,16 @@ make_pending_calls_with_count(PyThreadState *tstate)
     }
 
     clear_pending_handling_thread(pending);
-    return Py_MAX(called, main_called);
+    return 0;
 }
 
 static int
 make_pending_calls(PyThreadState *tstate)
 {
-    if (make_pending_calls_with_count(tstate) < 0) {
-        return -1;
-    }
-
-    return 0;
+    _PyRWMutex_RLock(&tstate->interp->prefini_mutex);
+    int res = make_pending_calls_lock_held(tstate);
+    _PyRWMutex_RUnlock(&tstate->interp->prefini_mutex);
+    return res;
 }
 
 

Python/pylifecycle.c

@@ -96,7 +96,7 @@ static PyStatus init_android_streams(PyThreadState *tstate);
 #if defined(__APPLE__) && HAS_APPLE_SYSTEM_LOG
 static PyStatus init_apple_streams(PyThreadState *tstate);
 #endif
-static int wait_for_thread_shutdown(PyThreadState *tstate);
+static void wait_for_thread_shutdown(PyThreadState *tstate);
 static void finalize_subinterpreters(void);
 static void call_ll_exitfuncs(_PyRuntimeState *runtime);
 
@@ -2010,15 +2010,11 @@ 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 (;;) {
-        int called = 0;
-
         // Wrap up existing "threading"-module-created, non-daemon threads.
-        int threads_joined = wait_for_thread_shutdown(tstate);
-        called = Py_MAX(called, threads_joined);
+        wait_for_thread_shutdown(tstate);
 
         // Make any remaining pending calls.
-        int made_pending_calls = _Py_FinishPendingCalls(tstate);
-        called = Py_MAX(called, made_pending_calls);
+        _Py_FinishPendingCalls(tstate);
 
         /* The interpreter is still entirely intact at this point, and the
         * exit funcs may be relying on that.  In particular, if some thread
@@ -2030,9 +2026,9 @@ make_pre_finalization_calls(PyThreadState *tstate)
         * the threads created via Threading.
         */
 
-        int called_atexit = _PyAtExit_Call(tstate->interp);
-        called = Py_MAX(called, called_atexit);
+        _PyAtExit_Call(tstate->interp);
 
+        _PyRWMutex_Unlock(&tstate->interp->prefini_mutex);
         if (called == 0) {
             break;
         }
@@ -3456,7 +3452,7 @@ Py_ExitStatusException(PyStatus status)
    the threading module was imported in the first place.
    The shutdown routine will wait until all non-daemon
    "threading" threads have completed. */
-static int
+static void
 wait_for_thread_shutdown(PyThreadState *tstate)
 {
     PyObject *result;
@@ -3466,19 +3462,16 @@ wait_for_thread_shutdown(PyThreadState *tstate)
             PyErr_FormatUnraisable("Exception ignored on threading shutdown");
         }
         /* else: threading not imported */
-        return 0;
+        return;
     }
     int called = 0;
     result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown));
     if (result == NULL) {
         PyErr_FormatUnraisable("Exception ignored on threading shutdown");
     }
-    else {
-        assert(PyBool_Check(result) && _Py_IsImmortal(result));
-        called = result == Py_True;
-    }
+    Py_XDECREF(result);
     Py_DECREF(threading);
-    return called;
+    return;
 }
 
 int Py_AtExit(void (*func)(void))