From ca1142fe53a94d550b22c65ba5bb23652093809b Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Mon, 10 Nov 2025 20:28:37 +0530 Subject: [PATCH 1/5] skip locking on both mutexes --- Python/critical_section.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/Python/critical_section.c b/Python/critical_section.c index e628ba2f6d19bc..218b580e95176d 100644 --- a/Python/critical_section.c +++ b/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 && - untag_critical_section(tstate->critical_section)->_cs_mutex == m) { - c->_cs_mutex = NULL; - c->_cs_prev = 0; - return; + // If the top-most critical section is a two-mutex critical section, + // then locking is skipped if either mutex is m. + if (tstate->critical_section) { + PyCriticalSection *prev = untag_critical_section(tstate->critical_section); + 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; From 62de52e12cdf7592e2dacdc0bff994859e7e7344 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 14 Nov 2025 21:30:33 +0530 Subject: [PATCH 2/5] add tests --- .../test_critical_sections.c | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/Modules/_testinternalcapi/test_critical_sections.c b/Modules/_testinternalcapi/test_critical_sections.c index e0ba37abcdd332..56119182f29c86 100644 --- a/Modules/_testinternalcapi/test_critical_sections.c +++ b/Modules/_testinternalcapi/test_critical_sections.c @@ -284,10 +284,105 @@ test_critical_sections_gc(PyObject *self, PyObject *Py_UNUSED(args)) #endif +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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil(_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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1); + // Releasing second critical section is a no-op. + PyCriticalSection_End(&cs2); + assert_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1); + // Releasing first critical section unlocks the object + PyCriticalSection_End(&cs1); + assert_nogil(!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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil((_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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil((_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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil((_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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil((_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_nogil(PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); + assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); + assert_nogil((_PyThreadState_GET()->critical_section & + ~_Py_CRITICAL_SECTION_MASK) == (uintptr_t)&cs); + + // Releasing critical section on both objects unlocks them + PyCriticalSection2_End(&cs); + assert_nogil(!PyMutex_IsLocked(&a->ob_mutex)); + assert_nogil(!PyMutex_IsLocked(&b->ob_mutex)); + + Py_DECREF(a); + Py_DECREF(b); + Py_RETURN_NONE; +} + 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}, + {"test_critical_section1_reacquisition", test_critical_section1_reacquisition, METH_NOARGS}, + {"test_critical_section2_reacquisition", test_critical_section2_reacquisition, METH_NOARGS}, #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}, From 54d4b24c84f95262024bddce4eb29c3a481c05e7 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 14 Nov 2025 21:36:48 +0530 Subject: [PATCH 3/5] skip tests on gil-enabled builds --- Modules/_testinternalcapi/test_critical_sections.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Modules/_testinternalcapi/test_critical_sections.c b/Modules/_testinternalcapi/test_critical_sections.c index 56119182f29c86..72149e8091c0c2 100644 --- a/Modules/_testinternalcapi/test_critical_sections.c +++ b/Modules/_testinternalcapi/test_critical_sections.c @@ -284,6 +284,8 @@ 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)) { @@ -377,12 +379,16 @@ test_critical_section2_reacquisition(PyObject *self, PyObject *Py_UNUSED(args)) 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}, From 948ddf91d16c80fdefcae622b7704813cbf0fdda Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 14 Nov 2025 21:47:48 +0530 Subject: [PATCH 4/5] use plain asserts --- .../test_critical_sections.c | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Modules/_testinternalcapi/test_critical_sections.c b/Modules/_testinternalcapi/test_critical_sections.c index 72149e8091c0c2..e3b2fe716d48d3 100644 --- a/Modules/_testinternalcapi/test_critical_sections.c +++ b/Modules/_testinternalcapi/test_critical_sections.c @@ -295,23 +295,23 @@ test_critical_section1_reacquisition(PyObject *self, PyObject *Py_UNUSED(args)) PyCriticalSection cs1, cs2; // First acquisition of critical section on object locks it PyCriticalSection_Begin(&cs1, a); - assert_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1); + 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_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1); + 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_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil(_PyThreadState_GET()->critical_section == (uintptr_t)&cs1); + 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_nogil(!PyMutex_IsLocked(&a->ob_mutex)); + assert(!PyMutex_IsLocked(&a->ob_mutex)); Py_DECREF(a); Py_RETURN_NONE; @@ -328,10 +328,10 @@ test_critical_section2_reacquisition(PyObject *self, PyObject *Py_UNUSED(args)) PyCriticalSection2 cs; // First acquisition of critical section on objects locks them PyCriticalSection2_Begin(&cs, a, b); - assert_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil((_PyThreadState_GET()->critical_section & + 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 @@ -340,39 +340,39 @@ test_critical_section2_reacquisition(PyObject *self, PyObject *Py_UNUSED(args)) // Check re-acquiring on first object PyCriticalSection a_cs; PyCriticalSection_Begin(&a_cs, a); - assert_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil((_PyThreadState_GET()->critical_section & + 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_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil((_PyThreadState_GET()->critical_section & + 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_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil((_PyThreadState_GET()->critical_section & + 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_nogil(PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(PyMutex_IsLocked(&b->ob_mutex)); - assert_nogil(_PyCriticalSection_IsActive(PyThreadState_GET()->critical_section)); - assert_nogil((_PyThreadState_GET()->critical_section & + 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_nogil(!PyMutex_IsLocked(&a->ob_mutex)); - assert_nogil(!PyMutex_IsLocked(&b->ob_mutex)); + assert(!PyMutex_IsLocked(&a->ob_mutex)); + assert(!PyMutex_IsLocked(&b->ob_mutex)); Py_DECREF(a); Py_DECREF(b); From 90799488852d2fd47699f7e16d988a1c3c2f4341 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 14 Nov 2025 16:25:21 +0000 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-11-14-16-25-15.gh-issue-114203.n3tlQO.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-11-14-16-25-15.gh-issue-114203.n3tlQO.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-11-14-16-25-15.gh-issue-114203.n3tlQO.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-11-14-16-25-15.gh-issue-114203.n3tlQO.rst new file mode 100644 index 00000000000000..883f9333cae880 --- /dev/null +++ b/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.