From 007b3352eb943a915d059d301239757b9ba69620 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sat, 24 May 2025 00:23:29 -0400 Subject: [PATCH 1/2] docs: more warnings about locking and the GIL Signed-off-by: Henry Schreiner --- docs/advanced/misc.rst | 14 +++++++++++--- tests/test_embed/test_interpreter.cpp | 5 +++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 42cfc69f8c..493e996d06 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -299,9 +299,17 @@ However, the module is no longer free-threading safe, for the same reason as before, because the calculation is not synchronized. We can synchronize it using a Python critical section. This will do nothing if not in free-threaded Python. You can have it lock one or two Python objects. You cannot nest it. -(Note: In Python 3.13t, Python re-locks if you enter a critical section again, -which happens in various places. This was optimized away in 3.14+. Use a -``std::mutex`` instead if this is a problem). + +.. warning:: + + When using a ``py::scoped_critical_section``, make sure it is not nested and + that no other synchronization primitives (such as a ``std::mutex``) are + held, which could lead to deadlocks. In 3.13, taking the same lock causes it + to release then require, which means you can't use it to, for example, read + and write to a dictionary, because the dictionary uses a critical section + internally in CPython. Use a ``std::mutex`` instead if you need this on + Python 3.13. In 3.14, taking a lock on a locked object no longer releases + and relocks as an optimization, which also fixes this case. .. code-block:: cpp :emphasize-lines: 1,4,8 diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 3654708d2e..9515310891 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -365,8 +365,13 @@ TEST_CASE("Threads") { py::gil_scoped_acquire gil{}; #ifdef Py_GIL_DISABLED # if PY_VERSION_HEX < 0x030E0000 + // This will not run with the GIL, so it won't deadlock. That's + // because of how we run our tests. Be more careful of + // deadlocks if the "free-threaded" GIL could be enabled (at + // runtime). std::lock_guard lock(mutex); # else + // CPython's thread-safe API in no-GIL mode. py::scoped_critical_section lock(locals); # endif #endif From b898a35c8471c65ef4796ed5a3c430ec3950f842 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 24 May 2025 11:29:37 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix=20require=20=E2=86=92=20reacquire=20typ?= =?UTF-8?q?o?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/advanced/misc.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 493e996d06..681bf76314 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -305,7 +305,7 @@ Python. You can have it lock one or two Python objects. You cannot nest it. When using a ``py::scoped_critical_section``, make sure it is not nested and that no other synchronization primitives (such as a ``std::mutex``) are held, which could lead to deadlocks. In 3.13, taking the same lock causes it - to release then require, which means you can't use it to, for example, read + to release then reacquire, which means you can't use it to, for example, read and write to a dictionary, because the dictionary uses a critical section internally in CPython. Use a ``std::mutex`` instead if you need this on Python 3.13. In 3.14, taking a lock on a locked object no longer releases