gh-114203: skip locking if object is already locked by two-mutex critical section (#141476)

Misc/NEWS.d/next/Core_and_Builtins/2025-11-14-16-25-15.gh-issue-114203.n3tlQO.rst

@@ -0,0 +1 @@
+Skip locking if object is already locked by two-mutex critical section.

Modules/_testinternalcapi/test_critical_sections.c

@@ -284,10 +284,111 @@ test_critical_sections_gc(PyObject *self, PyObject *Py_UNUSED(args))
 
 #endif
 
+#ifdef Py_GIL_DISABLED
+
+static PyObject *
+test_critical_section1_reacquisition(PyObject *self, PyObject *Py_UNUSED(args))
+{
+    PyObject *a = PyDict_New();
+    assert(a != NULL);
+
+    PyCriticalSection cs1, cs2;
+    // First acquisition of critical section on object locks it
+    PyCriticalSection_Begin(&cs1, a);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1);
+    // Attempting to re-acquire critical section on same object which
+    // is already locked by top-most critical section is a no-op.
+    PyCriticalSection_Begin(&cs2, a);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1);
+    // Releasing second critical section is a no-op.
+    PyCriticalSection_End(&cs2);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1);
+    // Releasing first critical section unlocks the object
+    PyCriticalSection_End(&cs1);
+    assert(!PyMutex_IsLocked(&a->ob_mutex));
+
+    Py_DECREF(a);
+    Py_RETURN_NONE;
+}
+
+static PyObject *
+test_critical_section2_reacquisition(PyObject *self, PyObject *Py_UNUSED(args))
+{
+    PyObject *a = PyDict_New();
+    assert(a != NULL);
+    PyObject *b = PyDict_New();
+    assert(b != NULL);
+
+    PyCriticalSection2 cs;
+    // First acquisition of critical section on objects locks them
+    PyCriticalSection2_Begin(&cs, a, b);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(PyMutex_IsLocked(&b->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert((_PyThreadState_GET()->critical_section &
+                  ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs);
+
+    // Attempting to re-acquire critical section on either of two
+    // objects already locked by top-most critical section is a no-op.
+
+    // Check re-acquiring on first object
+    PyCriticalSection a_cs;
+    PyCriticalSection_Begin(&a_cs, a);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(PyMutex_IsLocked(&b->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert((_PyThreadState_GET()->critical_section &
+                  ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs);
+    // Releasing critical section on either object is a no-op.
+    PyCriticalSection_End(&a_cs);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(PyMutex_IsLocked(&b->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert((_PyThreadState_GET()->critical_section &
+                  ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs);
+
+    // Check re-acquiring on second object
+    PyCriticalSection b_cs;
+    PyCriticalSection_Begin(&b_cs, b);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(PyMutex_IsLocked(&b->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert((_PyThreadState_GET()->critical_section &
+                  ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs);
+    // Releasing critical section on either object is a no-op.
+    PyCriticalSection_End(&b_cs);
+    assert(PyMutex_IsLocked(&a->ob_mutex));
+    assert(PyMutex_IsLocked(&b->ob_mutex));
+    assert(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section));
+    assert((_PyThreadState_GET()->critical_section &
+                  ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs);
+
+    // Releasing critical section on both objects unlocks them
+    PyCriticalSection2_End(&cs);
+    assert(!PyMutex_IsLocked(&a->ob_mutex));
+    assert(!PyMutex_IsLocked(&b->ob_mutex));
+
+    Py_DECREF(a);
+    Py_DECREF(b);
+    Py_RETURN_NONE;
+}
+
+#endif // Py_GIL_DISABLED
+
 static PyMethodDef test_methods[] = {
     {"test_critical_sections", test_critical_sections, METH_NOARGS},
     {"test_critical_sections_nest", test_critical_sections_nest, METH_NOARGS},
     {"test_critical_sections_suspend", test_critical_sections_suspend, METH_NOARGS},
+#ifdef Py_GIL_DISABLED
+    {"test_critical_section1_reacquisition", test_critical_section1_reacquisition, METH_NOARGS},
+    {"test_critical_section2_reacquisition", test_critical_section2_reacquisition, METH_NOARGS},
+#endif
 #ifdef Py_CAN_START_THREADS
     {"test_critical_sections_threads", test_critical_sections_threads, METH_NOARGS},
     {"test_critical_sections_gc", test_critical_sections_gc, METH_NOARGS},

Python/critical_section.c

@@ -24,11 +24,24 @@ _PyCriticalSection_BeginSlow(PyCriticalSection *c, PyMutex *m)
     // As an optimisation for locking the same object recursively, skip
     // locking if the mutex is currently locked by the top-most critical
     // section.
-    if (tstate->critical_section &&
+    // If the top-most critical section is a two-mutex critical section,
-        untag_critical_section(tstate->critical_section)->_cs_mutex == m) {
+    // then locking is skipped if either mutex is m.
-        c->_cs_mutex = NULL;
+    if (tstate->critical_section) {
-        c->_cs_prev = 0;
+        PyCriticalSection *prev = untag_critical_section(tstate->critical_section);
-        return;
+        if (prev->_cs_mutex == m) {
+            c->_cs_mutex = NULL;
+            c->_cs_prev = 0;
+            return;
+        }
+        if (tstate->critical_section & _Py_CRITICAL_SECTION_TWO_MUTEXES) {
+            PyCriticalSection2 *prev2 = (PyCriticalSection2 *)
+                untag_critical_section(tstate->critical_section);
+            if (prev2->_cs_mutex2 == m) {
+                c->_cs_mutex = NULL;
+                c->_cs_prev = 0;
+                return;
+            }
+        }
     }
     c->_cs_mutex = NULL;
     c->_cs_prev = (uintptr_t)tstate->critical_section;