From c03a7e6932c3b437bdf873acfc66fbaae31f1875 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Fri, 26 Jun 2026 11:29:58 -0500 Subject: [PATCH] Py3.13+: Preserve thread state critical_section to prevent crash on free-threaded builds. Fixes #513 --- CHANGES.rst | 6 ++++- src/greenlet/TGreenlet.hpp | 3 ++- src/greenlet/TPythonState.cpp | 4 ++- src/greenlet/tests/test_greenlet.py | 40 +++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 854610ad..62688b73 100644 --- a/CHANGES.rst +++ b/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 + `_. Thanks + to ddorian. 3.5.2 (2026-06-17) diff --git a/src/greenlet/TGreenlet.hpp b/src/greenlet/TGreenlet.hpp index 074aa973..764d46f7 100644 --- a/src/greenlet/TGreenlet.hpp +++ b/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 diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp index 9c98d14b..bdc0eef2 100644 --- a/src/greenlet/TPythonState.cpp +++ b/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; diff --git a/src/greenlet/tests/test_greenlet.py b/src/greenlet/tests/test_greenlet.py index 36d5d1e2..7350fc3e 100644 --- a/src/greenlet/tests/test_greenlet.py +++ b/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):