Significant rewrite to avoid using thread_locals as much as possible.

Since we can avoid them by checking this atomic, the cmake config conditional shouldn't be necessary.

The slower path (with thread_locals and extra checks) only comes in when a second interpreter is actually instanciated.

Author: b-pass

CMakeLists.txt

@@ -92,7 +92,6 @@ option(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION
        "To enforce that a handle_type_name<> specialization exists" OFF)
 option(PYBIND11_SIMPLE_GIL_MANAGEMENT
        "Use simpler GIL management logic that does not support disassociation" OFF)
-option(PYBIND11_SUBINTERPRETER_SUPPORT "Enable support for sub-interpreters" OFF)
 option(PYBIND11_NUMPY_1_ONLY
        "Disable NumPy 2 support to avoid changes to previous pybind11 versions." OFF)
 set(PYBIND11_INTERNALS_VERSION
@@ -106,9 +105,6 @@ endif()
 if(PYBIND11_SIMPLE_GIL_MANAGEMENT)
   add_compile_definitions(PYBIND11_SIMPLE_GIL_MANAGEMENT)
 endif()
-if(PYBIND11_SUBINTERPRETER_SUPPORT)
-  add_compile_definitions(PYBIND11_SUBINTERPRETER_SUPPORT)
-endif()
 if(PYBIND11_NUMPY_1_ONLY)
   add_compile_definitions(PYBIND11_NUMPY_1_ONLY)
 endif()

include/pybind11/detail/common.h

@@ -291,6 +291,13 @@
 #    define PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF
 #endif
 
+// Slightly faster code paths are available when this is NOT defined, so undefine it for impls
+// that do not have subinterpreter.  Nothing breaks if this is defined but the impl does not
+// actually support subinterpreters.
+#if PY_VERSION_HEX >= 0x030C0000 && !defined(PYPY_VERSION) && !defined(GRAALVM_PYTHON)
+#    define PYBIND11_SUBINTERPRETER_SUPPORT
+#endif
+
 // #define PYBIND11_STR_LEGACY_PERMISSIVE
 // If DEFINED, pybind11::str can hold PyUnicodeObject or PyBytesObject
 //             (probably surprising and never documented, but this was the
@@ -466,6 +473,7 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
         return m.ptr();                                                                           \
     }                                                                                             \
     int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) {                                    \
+        pybind11::detail::get_interpreter_count()++;                                              \
         try {                                                                                     \
             auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm);                       \
             PYBIND11_CONCAT(pybind11_init_, name)(m);                                             \

include/pybind11/detail/internals.h

@@ -18,6 +18,7 @@
 #include <pybind11/conduit/pybind11_platform_abi_id.h>
 #include <pybind11/pytypes.h>
 
+#include <atomic>
 #include <exception>
 #include <mutex>
 #include <thread>
@@ -257,29 +258,47 @@ struct type_info {
     "__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION)                     \
         PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE PYBIND11_PLATFORM_ABI_ID "__"
 
+inline PyThreadState *get_thread_state_unchecked() {
+#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)
+    return PyThreadState_GET();
+#elif PY_VERSION_HEX < 0x030D0000
+    return _PyThreadState_UncheckedGet();
+#else
+    return PyThreadState_GetUnchecked();
+#endif
+}
+
+/// We use this count to figure out if there are multiple sub-interpreters currently present.
+/// Might be read/written from multiple interpreters in multiple threads at the same time with no
+/// syncronization. This must never decrease while any interpreter may be running in any thread!
+inline std::atomic<int64_t> &get_interpreter_count() {
+    static std::atomic<int64_t> counter(0);
+    return counter;
+}
+
 /// Each module locally stores a pointer to the `internals` data. The data
 /// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`.
 inline internals **&get_internals_pp() {
-#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON) || PY_VERSION_HEX < 0x030C0000               \
-    || !defined(PYBIND11_SUBINTERPRETER_SUPPORT)
-    static internals **internals_pp = nullptr;
-#else
-    static thread_local internals **internals_pp = nullptr;
-    // This is one per interpreter, we cache it but if the thread changed
-    // then we need to invalidate our cache
-    // the caller will find the right value and set it if its null
-    static thread_local PyThreadState *tstate_cached = nullptr;
-#    if PY_VERSION_HEX < 0x030D0000
-    PyThreadState *tstate = _PyThreadState_UncheckedGet();
-#    else
-    PyThreadState *tstate = PyThreadState_GetUnchecked();
-#    endif
-    if (tstate != tstate_cached) {
-        tstate_cached = tstate;
-        internals_pp = nullptr;
+#ifdef PYBIND11_SUBINTERPRETER_SUPPORT
+    if (get_interpreter_count() > 1) {
+        // Internals is one per interpreter. When multiple interpreters are alive in different
+        // threads we have to allow them to have different internals, so we need a thread_local.
+        static thread_local internals **t_internals_pp = nullptr;
+        // Whenever the interpreter changes we need to invalidate the internals_pp.  That is slow,
+        // so we only do it when the PyThreadState has changed, which indicates the interpreter
+        // might have changed as well.
+        static thread_local PyThreadState *tstate_cached = nullptr;
+        auto *tstate = get_thread_state_unchecked();
+        if (tstate != tstate_cached) {
+            tstate_cached = tstate;
+            // the caller will fetch the instance from the state dict or create a new one
+            t_internals_pp = nullptr;
+        }
+        return t_internals_pp;
     }
 #endif
-    return internals_pp;
+    static internals **s_internals_pp = nullptr;
+    return s_internals_pp;
 }
 
 // forward decl
@@ -410,20 +429,6 @@ inline object get_python_state_dict() {
     return state_dict;
 }
 
-inline object get_internals_obj_from_state_dict(handle state_dict) {
-    return reinterpret_steal<object>(
-        dict_getitemstringref(state_dict.ptr(), PYBIND11_INTERNALS_ID));
-}
-
-inline internals **get_internals_pp_from_capsule(handle obj) {
-    void *raw_ptr = PyCapsule_GetPointer(obj.ptr(), /*name=*/nullptr);
-    if (raw_ptr == nullptr) {
-        raise_from(PyExc_SystemError, "pybind11::detail::get_internals_pp_from_capsule() FAILED");
-        throw error_already_set();
-    }
-    return static_cast<internals **>(raw_ptr);
-}
-
 inline uint64_t round_up_to_next_pow2(uint64_t x) {
     // Round-up to the next power of two.
     // See https://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2
@@ -438,13 +443,8 @@ inline uint64_t round_up_to_next_pow2(uint64_t x) {
     return x;
 }
 
-/// Return a reference to the current `internals` data
-PYBIND11_NOINLINE internals &get_internals() {
-    auto **&internals_pp = get_internals_pp();
-    if (internals_pp && *internals_pp) {
-        return **internals_pp;
-    }
-
+template <typename InternalsType>
+inline InternalsType **find_or_create_internals_pp(char const *state_dict_key) {
     // 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 {
@@ -458,10 +458,31 @@ PYBIND11_NOINLINE internals &get_internals() {
     error_scope err_scope;
 
     dict state_dict = get_python_state_dict();
-    if (object internals_obj = get_internals_obj_from_state_dict(state_dict)) {
-        internals_pp = get_internals_pp_from_capsule(internals_obj);
+    object internals_obj
+        = reinterpret_steal<object>(dict_getitemstringref(state_dict.ptr(), state_dict_key));
+    if (internals_obj) {
+        void *raw_ptr = PyCapsule_GetPointer(internals_obj.ptr(), /*name=*/nullptr);
+        if (!raw_ptr) {
+            pybind11_fail("find_or_create_internals_pp: broken capsule!");
+        } else {
+            return reinterpret_cast<InternalsType **>(raw_ptr);
+        }
     }
+
+    auto pp = new InternalsType *(nullptr);
+    state_dict[state_dict_key] = capsule(reinterpret_cast<void *>(pp));
+    return pp;
+}
+
+/// Return a reference to the current `internals` data
+PYBIND11_NOINLINE internals &get_internals() {
+    auto **&internals_pp = get_internals_pp();
     if (internals_pp && *internals_pp) {
+        return **internals_pp;
+    }
+
+    internals_pp = find_or_create_internals_pp<internals>(PYBIND11_INTERNALS_ID);
+    if (*internals_pp) {
         // We loaded the internals through `state_dict`, which means that our `error_already_set`
         // and `builtin_exception` may be different local classes than the ones set up in the
         // initial exception translator, below, so add another for our local exception classes.
@@ -478,13 +499,10 @@ PYBIND11_NOINLINE internals &get_internals() {
         }
 #endif
     } else {
-        if (!internals_pp) {
-            internals_pp = new internals *();
-        }
         auto *&internals_ptr = *internals_pp;
         internals_ptr = new internals();
 
-        PyThreadState *tstate = PyThreadState_Get();
+        PyThreadState *tstate = get_thread_state_unchecked();
         // NOLINTNEXTLINE(bugprone-assignment-in-if-condition)
         if (!PYBIND11_TLS_KEY_CREATE(internals_ptr->tstate)) {
             pybind11_fail("get_internals: could not successfully initialize the tstate TSS key!");
@@ -498,7 +516,6 @@ PYBIND11_NOINLINE internals &get_internals() {
         }
 
         internals_ptr->istate = tstate->interp;
-        state_dict[PYBIND11_INTERNALS_ID] = capsule(reinterpret_cast<void *>(internals_pp));
         internals_ptr->registered_exception_translators.push_front(&translate_exception);
         internals_ptr->static_property_type = make_static_property_type();
         internals_ptr->default_metaclass = make_default_metaclass();
@@ -528,69 +545,48 @@ struct local_internals {
     std::forward_list<ExceptionTranslator> registered_exception_translators;
 };
 
-/// Works like `get_internals`, but for things which are locally registered.
-inline local_internals &get_local_internals() {
-    // Current static can be created in the interpreter finalization routine. If the later will be
-    // destroyed in another static variable destructor, creation of this static there will cause
-    // static deinitialization fiasco. In order to avoid it we avoid destruction of the
-    // local_internals static. One can read more about the problem and current solution here:
-    // https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
-
-#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON) || PY_VERSION_HEX < 0x030C0000               \
-    || !defined(PYBIND11_SUBINTERPRETER_SUPPORT)
-    static auto *locals = new local_internals();
-#else
-    static thread_local local_internals *locals = nullptr;
-    // This is one per interpreter, we cache it but if the interpreter changed
-    // then we need to invalidate our cache and re-fetch from the state dict
-    static thread_local PyThreadState *tstate_cached = nullptr;
-#    if PY_VERSION_HEX < 0x030D0000
-    PyThreadState *tstate = _PyThreadState_UncheckedGet();
-#    else
-    PyThreadState *tstate = PyThreadState_GetUnchecked();
-#    endif
-    if (!tstate) {
-        pybind11_fail(
-            "pybind11::detail::get_local_internals() called without a current python thread");
-    }
-    if (tstate != tstate_cached) {
-        // we create a unique value at first run which is based on a pointer to
-        // a (non-thread_local) static value in this function, then multiple
-        // loaded modules using this code will still each have a unique key.
-        static const std::string this_module_idstr
-            = PYBIND11_MODULE_LOCAL_ID
-              + std::to_string(reinterpret_cast<uintptr_t>(&this_module_idstr));
-
-        // 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 {
-            gil_scoped_acquire_local() : state(PyGILState_Ensure()) {}
-            gil_scoped_acquire_local(const gil_scoped_acquire_local &) = delete;
-            gil_scoped_acquire_local &operator=(const gil_scoped_acquire_local &) = delete;
-            ~gil_scoped_acquire_local() { PyGILState_Release(state); }
-            const PyGILState_STATE state;
-        } gil;
-
-        error_scope err_scope;
-        dict state_dict = get_python_state_dict();
-        object local_capsule = reinterpret_steal<object>(
-            dict_getitemstringref(state_dict.ptr(), this_module_idstr.c_str()));
-        if (!local_capsule) {
-            locals = new local_internals();
-            state_dict[this_module_idstr.c_str()] = capsule(reinterpret_cast<void *>(locals));
-        } else {
-            void *ptr = PyCapsule_GetPointer(local_capsule.ptr(), nullptr);
-            if (!ptr) {
-                raise_from(PyExc_SystemError, "pybind11::detail::get_local_internals() FAILED");
-                throw error_already_set();
-            }
-            locals = reinterpret_cast<local_internals *>(ptr);
+inline local_internals **&get_local_internals_pp() {
+#ifdef PYBIND11_SUBINTERPRETER_SUPPORT
+    if (get_interpreter_count() > 1) {
+        // Internals is one per interpreter. When multiple interpreters are alive in different
+        // threads we have to allow them to have different internals, so we need a thread_local.
+        static thread_local local_internals **t_internals_pp = nullptr;
+        // Whenever the interpreter changes we need to invalidate the internals_pp.  That is slow,
+        // so we only do it when the PyThreadState has changed, which indicates the interpreter
+        // might have changed as well.
+        static thread_local PyThreadState *tstate_cached = nullptr;
+        auto *tstate = get_thread_state_unchecked();
+        if (tstate != tstate_cached) {
+            tstate_cached = tstate;
+            // the caller will fetch the instance from the state dict or create a new one
+            t_internals_pp = nullptr;
         }
-        tstate_cached = tstate;
+        return t_internals_pp;
     }
 #endif
+    static local_internals **s_internals_pp = nullptr;
+    return s_internals_pp;
+}
+
+/// Works like `get_internals`, but for things which are locally registered.
+PYBIND11_NOINLINE local_internals &get_local_internals() {
+    auto **&local_internals_pp = get_local_internals_pp();
+    if (local_internals_pp && *local_internals_pp) {
+        return **local_internals_pp;
+    }
+
+    // we create a unique value at first run which is based on a pointer to a (non-thread_local)
+    // static value in this function, then multiple loaded modules using this code will still each
+    // have a unique key.
+    static const std::string this_module_idstr
+        = PYBIND11_MODULE_LOCAL_ID
+          + std::to_string(reinterpret_cast<uintptr_t>(&this_module_idstr));
 
-    return *locals;
+    local_internals_pp = find_or_create_internals_pp<local_internals>(this_module_idstr.c_str());
+    if (!*local_internals_pp) {
+        *local_internals_pp = new local_internals();
+    }
+    return **local_internals_pp;
 }
 
 #ifdef Py_GIL_DISABLED

include/pybind11/detail/type_caster_base.h

@@ -497,16 +497,6 @@ PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_i
     });
 }
 
-inline PyThreadState *get_thread_state_unchecked() {
-#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)
-    return PyThreadState_GET();
-#elif PY_VERSION_HEX < 0x030D0000
-    return _PyThreadState_UncheckedGet();
-#else
-    return PyThreadState_GetUnchecked();
-#endif
-}
-
 // Forward declarations
 void keep_alive_impl(handle nurse, handle patient);
 inline PyObject *make_new_instance(PyTypeObject *type);

include/pybind11/embed.h

@@ -193,6 +193,8 @@ inline void initialize_interpreter(bool init_signal_handlers = true,
     config.install_signal_handlers = init_signal_handlers ? 1 : 0;
     initialize_interpreter(&config, argc, argv, add_program_dir_to_path);
 #endif
+
+    detail::get_interpreter_count() = 1;
 }
 
 /** \rst
@@ -234,23 +236,24 @@ inline void finalize_interpreter() {
     // Get the internals pointer (without creating it if it doesn't exist).  It's possible for the
     // internals to be created during Py_Finalize() (e.g. if a py::capsule calls `get_internals()`
     // during destruction), so we get the pointer-pointer here and check it after Py_Finalize().
-    detail::internals **internals_ptr_ptr = detail::get_internals_pp();
-    // It could also be stashed in state_dict, so look there too:
-    if (object internals_obj
-        = get_internals_obj_from_state_dict(detail::get_python_state_dict())) {
-        internals_ptr_ptr = detail::get_internals_pp_from_capsule(internals_obj);
-    }
-    // Local internals contains data managed by the current interpreter, so we must clear them to
-    // avoid undefined behaviors when initializing another interpreter
-    detail::get_local_internals().registered_types_cpp.clear();
-    detail::get_local_internals().registered_exception_translators.clear();
+    auto **&internals_pp = detail::get_internals_pp();
+    auto **&local_internals_pp = detail::get_local_internals_pp();
 
     Py_Finalize();
 
-    if (internals_ptr_ptr) {
-        delete *internals_ptr_ptr;
-        *internals_ptr_ptr = nullptr;
+    if (internals_pp) {
+        delete *internals_pp;
+        *internals_pp = nullptr;
     }
+
+    // Local internals contains data managed by the current interpreter, so we must clear them to
+    // avoid undefined behaviors when initializing another interpreter
+    if (local_internals_pp) {
+        delete *local_internals_pp;
+        local_internals_pp = nullptr;
+    }
+
+    detail::get_interpreter_count() = 0;
 }
 
 /** \rst

include/pybind11/pybind11.h

@@ -1291,7 +1291,6 @@ inline bool gil_not_used_option(F &&, O &&...o) {
 
 #ifdef Py_mod_multiple_interpreters
 inline void *multi_interp_option() { return Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED; }
-#    ifdef PYBIND11_SUBINTERPRETER_SUPPORT
 template <typename F, typename... O>
 void *multi_interp_option(F &&, O &&...o);
 template <typename... O>
@@ -1311,7 +1310,6 @@ inline void *multi_interp_option(mod_multi_interpreter_one_gil f, O &&...o) {
     }
     return Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED;
 }
-#    endif
 template <typename F, typename... O>
 inline void *multi_interp_option(F &&, O &&...o) {
     return multi_interp_option(o...);