Merge pull request #514 from python-greenlet/issue513-preserve-crit-section

Py3.13+: Preserve thread state critical_section to prevent crash on free-threaded builds

Author: jamadden

CHANGES.rst

@@ -5,7 +5,11 @@
 3.5.3 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Fix a crash on free-threaded builds when multiple greenlets were
+  holding a critical section on an object and the GIL for the thread
+  was dropped. See `issue 513
+  <https://github.com/python-greenlet/greenlet/issues/513>`_. Thanks
+  to ddorian.
 
 
 3.5.2 (2026-06-17)

src/greenlet/TGreenlet.hpp

@@ -147,7 +147,8 @@ namespace greenlet
         int recursion_depth;
 #endif
 #if GREENLET_PY313
-        PyObject *delete_later;
+        PyObject* delete_later;
+        uintptr_t critical_section;
 #else
         int trash_delete_nesting;
 #endif

src/greenlet/TPythonState.cpp

@@ -27,6 +27,7 @@ PythonState::PythonState()
 #endif
 #if GREENLET_PY313
     ,delete_later(nullptr)
+    ,critical_section(0)
 #else
     ,trash_delete_nesting(0)
 #endif
@@ -191,6 +192,7 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
     // ``greenlet.tests.test_greenlet_trash`` tries, but under 3.14,
     // at least, fails to do so.
     this->delete_later = Py_XNewRef(tstate->delete_later);
+    this->critical_section = tstate->critical_section;
   #elif GREENLET_PY312
     this->trash_delete_nesting = tstate->trash.delete_nesting;
   #else // not 312 or 3.13+
@@ -298,7 +300,7 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
         tstate->delete_later = this->delete_later;
         Py_CLEAR(this->delete_later);
     }
-
+    tstate->critical_section = this->critical_section;
 
   #elif GREENLET_PY312
     tstate->trash.delete_nesting = this->trash_delete_nesting;

src/greenlet/tests/test_greenlet.py

@@ -1027,6 +1027,46 @@ def inner(frame):
         # The next line crashes on 3.12 if we haven't exposed the frames.
         self.assertIsNone(frame.f_back)
 
+    def test_switching_holding_critical_section_no_crash(self):
+        # https://github.com/python-greenlet/greenlet/issues/513
+        # In no-GIL builds, we were failing to restore the
+        # critical_section pointer, leading to
+        # ``Fatal Python error: PyMutex_Unlock: unlocking mutex that is not locked``
+        # when two greenlets both held a lock on an object, and then
+        # the GIL was released, which, according to
+        # ``Include/cpython/critical_section.h`` causes the critical
+        # section to get released.
+        # The field is present on Python 3.13+, and only used
+        # in no-GIL builds.
+        # Initial version of this test case provided in the
+        # github issue by ddorian
+        def k1(x):
+            g2.switch()  # into G2 while holding G1's sort critical section
+            return x
+
+
+        def k2(x):
+            g1.switch()  # back into G1 while holding G2's sort critical section
+            return x
+
+
+        def g1_body():
+            sorted([0], key=k1)
+            g2.switch()
+
+
+        def g2_body():
+            sorted([0], key=k2)
+            # Do a blocking I/O operation which would cause the
+            # critical sections to get suspended
+            with open(__file__, "rt", encoding='utf-8') as f:
+                f.read()
+
+
+        g1 = greenlet.greenlet(g1_body)
+        g2 = greenlet.greenlet(g2_body)
+        g1.switch()
+
 
 class TestGreenletSetParentErrors(TestCase):
     def test_threaded_reparent(self):