All bells & whistles.

Author: rwgk

.github/workflows/ci.yml

@@ -102,10 +102,12 @@ jobs:
       run: python -m pip install pytest-github-actions-annotate-failures
 
     # First build - C++11 mode and inplace
+    # More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON here.
     - name: Configure C++11 ${{ matrix.args }}
       run: >
         cmake -S . -B .
         -DPYBIND11_WERROR=ON
+        -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON
         -DDOWNLOAD_CATCH=ON
         -DDOWNLOAD_EIGEN=ON
         -DCMAKE_CXX_STANDARD=11
@@ -129,10 +131,12 @@ jobs:
       run: git clean -fdx
 
     # Second build - C++17 mode and in a build directory
+    # More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF here.
     - name: Configure C++17
       run: >
         cmake -S . -B build2
         -DPYBIND11_WERROR=ON
+        -DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF
         -DDOWNLOAD_CATCH=ON
         -DDOWNLOAD_EIGEN=ON
         -DCMAKE_CXX_STANDARD=17

CMakeLists.txt

@@ -91,13 +91,14 @@ endif()
 option(PYBIND11_INSTALL "Install pybind11 header files?" ${PYBIND11_MASTER_PROJECT})
 option(PYBIND11_TEST "Build pybind11 test suite?" ${PYBIND11_MASTER_PROJECT})
 option(PYBIND11_NOPYTHON "Disable search for Python" OFF)
-option(PYBIND11_SIMPLE_GIL "Use simpler GIL access logic that does not support disassociation" OFF)
+option(PYBIND11_SIMPLE_GIL_MANAGEMENT
+       "Use simpler GIL management logic that does not support disassociation" OFF)
 set(PYBIND11_INTERNALS_VERSION
     ""
     CACHE STRING "Override the ABI version, may be used to enable the unstable ABI.")
 
-if(PYBIND11_SIMPLE_GIL)
-  add_compile_definitions(PYBIND11_SIMPLE_GIL)
+if(PYBIND11_SIMPLE_GIL_MANAGEMENT)
+  add_compile_definitions(PYBIND11_SIMPLE_GIL_MANAGEMENT)
 endif()
 
 cmake_dependent_option(

docs/upgrade.rst

@@ -13,16 +13,14 @@ modernization and other useful information.
 v2.10
 =====
 
-The current scoped GIL implementation doesn't support nested access. In pybind11
-In 2.10.1, a configuration option ``PYBIND11_SIMPLE_GIL`` was added, defaulting
-to OFF; the simpler GIL implementation supports nested access, but does not
-support dissociation (the ``true`` parameter of ``gil_scope_release``).  In
-pybind11 2.11, we plan to change the default to ON. If you need the old
-behavior, please set ``PYBIND11_SIMPLE_GIL`` to OFF. We plan to have an example
-for manually supporting dissociation.
-
-There may be an unconfirmed ABI breakage between 2.9 and 2.10. We plan to bump
-the internals number in 2.11.
+``py::gil_scoped_acquire`` & ``py::gil_scoped_release`` in pybind11 versions
+< v2.10.1 do not support nested access. In v2.10.1, a configuration option
+``PYBIND11_SIMPLE_GIL_MANAGEMENT`` was added, defaulting to ``OFF``; the
+simpler implementations support nested access, but do not support dissociation
+(``py::gil_scoped_release(true)``). In pybind11 2.11, we plan to change the
+default to ``ON``, to avoid pitfalls of the implementations with dissociation
+(see #4216 for more information). Note that the dissociation feature is very
+rarely used and not exercised in any pybind11 unit tests.
 
 .. _upgrade-guide-2.9:
 

include/pybind11/detail/common.h

@@ -229,6 +229,10 @@
 #    undef copysign
 #endif
 
+#if defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
+#    define PYBIND11_SIMPLE_GIL_MANAGEMENT
+#endif
+
 #if defined(_MSC_VER)
 #    if defined(PYBIND11_DEBUG_MARKER)
 #        define _DEBUG

include/pybind11/detail/internals.h

@@ -9,6 +9,10 @@
 
 #pragma once
 
+#if defined(WITH_THREAD) && defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
+#    include "../gil.h"
+#endif
+
 #include "../pytypes.h"
 
 #include <exception>
@@ -169,10 +173,12 @@ struct internals {
     PyTypeObject *default_metaclass;
     PyObject *instance_base;
 #if defined(WITH_THREAD)
+    // Unused if PYBIND11_SIMPLE_GIL_MANAGEMENT is defined:
     PYBIND11_TLS_KEY_INIT(tstate)
 #    if PYBIND11_INTERNALS_VERSION > 4
     PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
 #    endif // PYBIND11_INTERNALS_VERSION > 4
+    // Unused if PYBIND11_SIMPLE_GIL_MANAGEMENT is defined:
     PyInterpreterState *istate = nullptr;
     ~internals() {
 #    if PYBIND11_INTERNALS_VERSION > 4
@@ -408,6 +414,10 @@ PYBIND11_NOINLINE internals &get_internals() {
         return **internals_pp;
     }
 
+#if defined(WITH_THREAD)
+#    if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
+    gil_scoped_acquire gil;
+#    else
     // Ensure that the GIL is held since we will need to make Python calls.
     // Cannot use py::gil_scoped_acquire here since that constructor calls get_internals.
     struct gil_scoped_acquire_local {
@@ -417,6 +427,8 @@ PYBIND11_NOINLINE internals &get_internals() {
         ~gil_scoped_acquire_local() { PyGILState_Release(state); }
         const PyGILState_STATE state;
     } gil;
+#    endif
+#endif
     error_scope err_scope;
 
     PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID);

include/pybind11/gil.h

@@ -10,7 +10,10 @@
 #pragma once
 
 #include "detail/common.h"
-#include "detail/internals.h"
+
+#if defined(WITH_THREAD) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
+#    include "detail/internals.h"
+#endif
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
@@ -21,7 +24,9 @@ PyThreadState *get_thread_state_unchecked();
 
 PYBIND11_NAMESPACE_END(detail)
 
-#if defined(WITH_THREAD) && !defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL)
+#if defined(WITH_THREAD)
+
+#    if !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
 
 /* The functions below essentially reproduce the PyGILState_* API using a RAII
  * pattern, but there are a few important differences:
@@ -62,11 +67,11 @@ class gil_scoped_acquire {
 
         if (!tstate) {
             tstate = PyThreadState_New(internals.istate);
-#    if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
+#        if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
             if (!tstate) {
                 pybind11_fail("scoped_acquire: could not create thread state!");
             }
-#    endif
+#        endif
             tstate->gilstate_counter = 0;
             PYBIND11_TLS_REPLACE_VALUE(internals.tstate, tstate);
         } else {
@@ -87,20 +92,20 @@ class gil_scoped_acquire {
 
     PYBIND11_NOINLINE void dec_ref() {
         --tstate->gilstate_counter;
-#    if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
+#        if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
         if (detail::get_thread_state_unchecked() != tstate) {
             pybind11_fail("scoped_acquire::dec_ref(): thread state must be current!");
         }
         if (tstate->gilstate_counter < 0) {
             pybind11_fail("scoped_acquire::dec_ref(): reference count underflow!");
         }
-#    endif
+#        endif
         if (tstate->gilstate_counter == 0) {
-#    if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
+#        if defined(PYBIND11_DETAILED_ERROR_MESSAGES)
             if (!release) {
                 pybind11_fail("scoped_acquire::dec_ref(): internal error!");
             }
-#    endif
+#        endif
             PyThreadState_Clear(tstate);
             if (active) {
                 PyThreadState_DeleteCurrent();
@@ -178,12 +183,14 @@ class gil_scoped_release {
     bool disassoc;
     bool active = true;
 };
-#elif defined(PYPY_VERSION) || defined(PYBIND11_SIMPLE_GIL)
+
+#    else // PYBIND11_SIMPLE_GIL_MANAGEMENT
+
 class gil_scoped_acquire {
     PyGILState_STATE state;
 
 public:
-    gil_scoped_acquire() { state = PyGILState_Ensure(); }
+    gil_scoped_acquire() : state{PyGILState_Ensure()} {}
     gil_scoped_acquire(const gil_scoped_acquire &) = delete;
     gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete;
     ~gil_scoped_acquire() { PyGILState_Release(state); }
@@ -194,19 +201,39 @@ class gil_scoped_release {
     PyThreadState *state;
 
 public:
-    gil_scoped_release() { state = PyEval_SaveThread(); }
+    gil_scoped_release() : state{PyEval_SaveThread()} {}
     gil_scoped_release(const gil_scoped_release &) = delete;
     gil_scoped_release &operator=(const gil_scoped_acquire &) = delete;
     ~gil_scoped_release() { PyEval_RestoreThread(state); }
     void disarm() {}
 };
-#else
+
+#    endif // PYBIND11_SIMPLE_GIL_MANAGEMENT
+
+#else // WITH_THREAD
+
 class gil_scoped_acquire {
+public:
+    gil_scoped_acquire() {
+        // Trick to suppress `unused variable` error messages (at call sites).
+        (void) (this != (this + 1));
+    }
+    gil_scoped_acquire(const gil_scoped_acquire &) = delete;
+    gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete;
     void disarm() {}
 };
+
 class gil_scoped_release {
+public:
+    gil_scoped_release() {
+        // Trick to suppress `unused variable` error messages (at call sites).
+        (void) (this != (this + 1));
+    }
+    gil_scoped_release(const gil_scoped_release &) = delete;
+    gil_scoped_release &operator=(const gil_scoped_acquire &) = delete;
     void disarm() {}
 };
-#endif
+
+#endif // WITH_THREAD
 
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

tests/test_embed/test_interpreter.cpp

@@ -293,7 +293,6 @@ TEST_CASE("Threads") {
 
     {
         py::gil_scoped_release gil_release{};
-        REQUIRE(has_pybind11_internals_static());
 
         auto threads = std::vector<std::thread>();
         for (auto i = 0; i < num_threads; ++i) {