gh-111924: use atomics for interp id refcounting (#125321)

python · Oct 12, 2024 · 5d8739e · 5d8739e
1 parent 5a074aa
commit 5d8739e
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 52 deletions.
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -102,9 +102,8 @@ struct _is {
     PyInterpreterState *next;
 
     int64_t id;
-    int64_t id_refcount;
+    Py_ssize_t id_refcount;
     int requires_idref;
-    PyThread_type_lock id_mutex;
 
 #define _PyInterpreterState_WHENCE_NOTSET -1
 #define _PyInterpreterState_WHENCE_UNKNOWN 0
@@ -318,8 +317,7 @@ _PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tst
 PyAPI_FUNC(int64_t) _PyInterpreterState_ObjectToID(PyObject *);
 PyAPI_FUNC(PyInterpreterState *) _PyInterpreterState_LookUpID(int64_t);
 PyAPI_FUNC(PyInterpreterState *) _PyInterpreterState_LookUpIDObject(PyObject *);
-PyAPI_FUNC(int) _PyInterpreterState_IDInitref(PyInterpreterState *);
-PyAPI_FUNC(int) _PyInterpreterState_IDIncref(PyInterpreterState *);
+PyAPI_FUNC(void) _PyInterpreterState_IDIncref(PyInterpreterState *);
 PyAPI_FUNC(void) _PyInterpreterState_IDDecref(PyInterpreterState *);
 
 PyAPI_FUNC(int) _PyInterpreterState_IsReady(PyInterpreterState *interp);

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -523,12 +523,6 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime)
 
     _PyTypes_AfterFork();
 
-    /* bpo-42540: id_mutex is freed by _PyInterpreterState_Delete, which does
-     * not force the default allocator. */
-    if (_PyThread_at_fork_reinit(&runtime->interpreters.main->id_mutex) < 0) {
-        return _PyStatus_ERR("Failed to reinitialize runtime locks");
-    }
-
     PyStatus status = gilstate_tss_reinit(runtime);
     if (_PyStatus_EXCEPTION(status)) {
         return status;
@@ -629,6 +623,8 @@ init_interpreter(PyInterpreterState *interp,
     assert(id > 0 || (id == 0 && interp == runtime->interpreters.main));
     interp->id = id;
 
+    interp->id_refcount = 0;
+
     assert(runtime->interpreters.head == interp);
     assert(next != NULL || (interp == runtime->interpreters.main));
     interp->next = next;
@@ -989,10 +985,6 @@ PyInterpreterState_Delete(PyInterpreterState *interp)
     }
     HEAD_UNLOCK(runtime);
 
-    if (interp->id_mutex != NULL) {
-        PyThread_free_lock(interp->id_mutex);
-    }
-
     _Py_qsbr_fini(interp);
 
     _PyObject_FiniState(interp);
@@ -1031,9 +1023,6 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime)
         // the "current" tstate to be set?
         PyInterpreterState_Clear(interp);  // XXX must activate?
         zapthreads(interp);
-        if (interp->id_mutex != NULL) {
-            PyThread_free_lock(interp->id_mutex);
-        }
         PyInterpreterState *prev_interp = interp;
         interp = interp->next;
         free_interpreter(prev_interp);
@@ -1247,9 +1236,6 @@ PyInterpreterState_GetID(PyInterpreterState *interp)
 PyObject *
 _PyInterpreterState_GetIDObject(PyInterpreterState *interp)
 {
-    if (_PyInterpreterState_IDInitref(interp) != 0) {
-        return NULL;
-    };
     int64_t interpid = interp->id;
     if (interpid < 0) {
         return NULL;
@@ -1259,50 +1245,22 @@ _PyInterpreterState_GetIDObject(PyInterpreterState *interp)
 }
 
 
-int
-_PyInterpreterState_IDInitref(PyInterpreterState *interp)
-{
-    if (interp->id_mutex != NULL) {
-        return 0;
-    }
-    interp->id_mutex = PyThread_allocate_lock();
-    if (interp->id_mutex == NULL) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "failed to create init interpreter ID mutex");
-        return -1;
-    }
-    interp->id_refcount = 0;
-    return 0;
-}
-
 
-int
+void
 _PyInterpreterState_IDIncref(PyInterpreterState *interp)
 {
-    if (_PyInterpreterState_IDInitref(interp) < 0) {
-        return -1;
-    }
-
-    PyThread_acquire_lock(interp->id_mutex, WAIT_LOCK);
-    interp->id_refcount += 1;
-    PyThread_release_lock(interp->id_mutex);
-    return 0;
+    _Py_atomic_add_ssize(&interp->id_refcount, 1);
 }
 
 
 void
 _PyInterpreterState_IDDecref(PyInterpreterState *interp)
 {
-    assert(interp->id_mutex != NULL);
     _PyRuntimeState *runtime = interp->runtime;
 
-    PyThread_acquire_lock(interp->id_mutex, WAIT_LOCK);
-    assert(interp->id_refcount != 0);
-    interp->id_refcount -= 1;
-    int64_t refcount = interp->id_refcount;
-    PyThread_release_lock(interp->id_mutex);
+    Py_ssize_t refcount = _Py_atomic_add_ssize(&interp->id_refcount, -1);
 
-    if (refcount == 0 && interp->requires_idref) {
+    if (refcount == 1 && interp->requires_idref) {
         PyThreadState *tstate =
             _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI);