Switch to a single atomic field instead of using locks and events.

Author: ZeroIntensity

Include/internal/pycore_interp_structs.h

@@ -827,6 +827,8 @@ struct _Py_unique_id_pool {
 
 typedef _Py_CODEUNIT *(*_PyJitEntryFuncPtr)(struct _PyExecutorObject *exec, _PyInterpreterFrame *frame, _PyStackRef *stack_pointer, PyThreadState *tstate);
 
+#define _PyInterpreterGuard_GUARDS_NOT_ALLOWED UINTPTR_MAX
+
 /* PyInterpreterState holds the global state for one of the runtime's
    interpreters.  Typically the initial (main) interpreter is the only one.
 
@@ -1053,11 +1055,10 @@ struct _is {
 #endif
 #endif
 
-    struct {
-        _PyRWMutex lock;
-        Py_ssize_t countdown;
-        PyEvent done;
-    } finalization_guards;
+    // The number of remaining finalization guards.
+    // If this is _PyInterpreterGuard_GUARDS_NOT_ALLOWED, then finalization
+    // guards can no longer be created.
+    uintptr_t finalization_guards;
 
     /* the initial PyInterpreterState.threads.head */
     _PyThreadStateImpl _initial_thread;

Include/internal/pycore_lock.h

@@ -88,9 +88,6 @@ PyAPI_FUNC(int) _PyEvent_IsSet(PyEvent *evt);
 PyAPI_FUNC(void) _PyEvent_Notify(PyEvent *evt);
 
 
-// Reset an event. There must be no active waiters.
-void _PyEvent_Reset(PyEvent *evt);
-
 // Wait for the event to be set. If the event is already set, then this returns
 // immediately.
 PyAPI_FUNC(void) PyEvent_Wait(PyEvent *evt);

Python/lock.c

@@ -313,15 +313,6 @@ _PyEvent_Notify(PyEvent *evt)
     }
 }
 
-void
-_PyEvent_Reset(PyEvent *evt)
-{
-    assert(evt != NULL);
-    // It's not safe to reset an event with active waiters.
-    assert(_Py_atomic_load_uint8(&evt->v) != _Py_HAS_PARKED);
-    _Py_atomic_store_uint8(&evt->v, _Py_UNLOCKED);
-}
-
 void
 PyEvent_Wait(PyEvent *evt)
 {

Python/pylifecycle.c

@@ -2314,37 +2314,6 @@ make_pre_finalization_calls(PyThreadState *tstate, int subinterpreters)
             finalize_subinterpreters();
         }
 
-        /* Wait on finalization guards.
-         *
-         * To avoid eating CPU cycles, we use an event to signal when we reach
-         * zero remaining guards. But, this isn't atomic! This event can be reset
-         * later if another thread creates a new finalization guard. The actual
-         * atomic check is made below, when we hold the finalization guard lock.
-         * Again, this is purely an optimization to avoid overloading the CPU.
-         */
-        for (;;) {
-            if (_Py_atomic_load_ssize_relaxed(&interp->finalization_guards.countdown) == 0) {
-                break;
-            }
-
-            PyTime_t wait_ns = 1000 * 1000;  // 1ms
-            if (PyEvent_WaitTimed(&interp->finalization_guards.done, wait_ns, /*detach=*/1)) {
-                break;
-            }
-
-            // For debugging purposes, we emit a fatal error if someone
-            // CTRL^C'ed the process.
-            if (PyErr_CheckSignals()) {
-                int fatal = PyErr_ExceptionMatches(PyExc_KeyboardInterrupt);
-                PyErr_FormatUnraisable("Exception ignored while waiting on finalization guards");
-
-                if (fatal) {
-                    fputs("Interrupted while waiting on finalization guard", stderr);
-                    exit(1);
-                }
-            }
-        }
-
         /* Stop the world to prevent other threads from creating threads or
          * atexit callbacks. On the default build, this is simply locked by
          * the GIL. For pending calls, we acquire the dedicated mutex, because
@@ -2355,27 +2324,28 @@ make_pre_finalization_calls(PyThreadState *tstate, int subinterpreters)
         // XXX Why does _PyThreadState_DeleteList() rely on all interpreters
         // being stopped?
         _PyEval_StopTheWorldAll(interp->runtime);
-        _PyRWMutex_Lock(&interp->finalization_guards.lock);
         int has_subinterpreters = subinterpreters
                                     ? runtime_has_subinterpreters(interp->runtime)
                                     : 0;
+        uintptr_t finalization_guards_expected = 0;
         int should_continue = (interp_has_threads(interp)
                               || interp_has_atexit_callbacks(interp)
                               || interp_has_pending_calls(interp)
                               || has_subinterpreters
-                              || _Py_atomic_load_ssize_acquire(&interp->finalization_guards.countdown) > 0);
+                              || _Py_atomic_compare_exchange_uintptr(&interp->finalization_guards,
+                                                                      &finalization_guards_expected,
+                                                                      _PyInterpreterGuard_GUARDS_NOT_ALLOWED) == 0);
         if (!should_continue) {
             break;
         }
         // Temporarily let other threads execute
         _PyThreadState_Detach(tstate);
-        _PyRWMutex_Unlock(&interp->finalization_guards.lock);
         _PyEval_StartTheWorldAll(interp->runtime);
         PyMutex_Unlock(&interp->ceval.pending.mutex);
         _PyThreadState_Attach(tstate);
     }
     assert(PyMutex_IsLocked(&interp->ceval.pending.mutex));
-    assert(_Py_atomic_load_ssize(&interp->finalization_guards.countdown) == 0);
+    assert(_Py_atomic_load_uintptr(&interp->finalization_guards) == _PyInterpreterGuard_GUARDS_NOT_ALLOWED);
     ASSERT_WORLD_STOPPED(interp);
 }
 
@@ -2434,7 +2404,6 @@ _Py_Finalize(_PyRuntimeState *runtime)
     for (PyThreadState *p = list; p != NULL; p = p->next) {
         _PyThreadState_SetShuttingDown(p);
     }
-    _PyRWMutex_Unlock(&tstate->interp->finalization_guards.lock);
     _PyEval_StartTheWorldAll(runtime);
     PyMutex_Unlock(&tstate->interp->ceval.pending.mutex);
 
@@ -2817,7 +2786,6 @@ Py_EndInterpreter(PyThreadState *tstate)
         _PyThreadState_SetShuttingDown(p);
     }
 
-    _PyRWMutex_Unlock(&interp->finalization_guards.lock);
     _PyEval_StartTheWorldAll(interp->runtime);
     PyMutex_Unlock(&interp->ceval.pending.mutex);
     _PyThreadState_DeleteList(list, /*is_after_fork=*/0);

Python/pystate.c

@@ -3322,7 +3322,7 @@ Py_ssize_t
 _PyInterpreterState_GuardCountdown(PyInterpreterState *interp)
 {
     assert(interp != NULL);
-    Py_ssize_t count = _Py_atomic_load_ssize_relaxed(&interp->finalization_guards.countdown);
+    Py_ssize_t count = _Py_atomic_load_uintptr_relaxed(&interp->finalization_guards);
     assert(count >= 0);
     return count;
 }
@@ -3339,32 +3339,14 @@ static int
 try_acquire_interp_guard(PyInterpreterState *interp, PyInterpreterGuard *guard)
 {
     assert(interp != NULL);
-    _PyRWMutex_RLock(&interp->finalization_guards.lock);
 
-    if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
-        _PyRWMutex_RUnlock(&interp->finalization_guards.lock);
-        assert(_Py_atomic_load_ssize_relaxed(&interp->finalization_guards.countdown) == 0);
-        return -1;
-    }
-
-    Py_ssize_t old_value = _Py_atomic_add_ssize(&interp->finalization_guards.countdown, 1);
-    if (old_value == 0) {
-        // We first have to notify the finalization thread if it's waiting on us, but
-        // it will get trapped waiting on the RW lock, so we don't have to worry about
-        // another active waiter while we reset the event.
-        _PyEvent_Notify(&interp->finalization_guards.done);
-        // Now, when the finalization thread goes to check the countdown
-        // after we release the lock, it will see that the countdown is
-        // non-zero and begin waiting again (hence why we need to reset the
-        // event).
-        // It is worth noting that this event is intentionally a non-atomic check
-        // for remaining finalization guards (that is, even if the event is set,
-        // it is not necessarily guaranteed that countdown is zero). We use it
-        // to simply prevent the finalization from constantly spinning and
-        // atomically reading the countdown.
-        _PyEvent_Reset(&interp->finalization_guards.done);
-    }
-    _PyRWMutex_RUnlock(&interp->finalization_guards.lock);
+    uintptr_t expected;
+    do {
+        expected = _Py_atomic_load_uintptr(&interp->finalization_guards);
+        if (expected == _PyInterpreterGuard_GUARDS_NOT_ALLOWED) {
+            return -1;
+        }
+    } while (_Py_atomic_compare_exchange_uintptr(&interp->finalization_guards, &expected, expected + 1) == 0);
 
     guard->interp = interp;
     return 0;
@@ -3398,14 +3380,9 @@ PyInterpreterGuard_Close(PyInterpreterGuard *guard)
     PyInterpreterState *interp = guard->interp;
     assert(interp != NULL);
 
-    _PyRWMutex_RLock(&interp->finalization_guards.lock);
-    Py_ssize_t old = _Py_atomic_add_ssize(&interp->finalization_guards.countdown, -1);
-    if (old == 1) {
-        _PyEvent_Notify(&interp->finalization_guards.done);
-    }
-    _PyRWMutex_RUnlock(&interp->finalization_guards.lock);
+    assert(_Py_atomic_load_uintptr(&interp->finalization_guards) != _PyInterpreterGuard_GUARDS_NOT_ALLOWED);
+    _Py_atomic_add_uintptr(&interp->finalization_guards, -1);
 
-    assert(old > 0);
     PyMem_RawFree(guard);
 }