Directly check if/which interpreter is active before doing CLEAR in destructor (#5965)

* Directly check if/which interpreter is active before doing CLEAR in destructors.

Py_IsFinalizing only applies to the main interpreter.

* Backward compatibility fixes

* Make clang-tidy happy

* Add nullptr checks to istate as Cursor suggested

include/pybind11/detail/internals.h

@@ -143,6 +143,38 @@ inline PyTypeObject *make_default_metaclass();
 inline PyObject *make_object_base_type(PyTypeObject *metaclass);
 inline void translate_exception(std::exception_ptr p);
 
+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
+}
+
+inline PyInterpreterState *get_interpreter_state_unchecked() {
+    auto *tstate = get_thread_state_unchecked();
+    return tstate ? tstate->interp : nullptr;
+}
+
+inline object get_python_state_dict() {
+    object state_dict;
+#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)
+    state_dict = reinterpret_borrow<object>(PyEval_GetBuiltins());
+#else
+    auto *istate = get_interpreter_state_unchecked();
+    if (istate) {
+        state_dict = reinterpret_borrow<object>(PyInterpreterState_GetDict(istate));
+    }
+#endif
+    if (!state_dict) {
+        raise_from(PyExc_SystemError, "pybind11::detail::get_python_state_dict() FAILED");
+        throw error_already_set();
+    }
+    return state_dict;
+}
+
 // Python loads modules by default with dlopen with the RTLD_LOCAL flag; under libc++ and possibly
 // other STLs, this means `typeid(A)` from one module won't equal `typeid(A)` from another module
 // even when `A` is the same, non-hidden-visibility type (e.g. from a common include).  Under
@@ -195,14 +227,6 @@ struct override_hash {
 
 using instance_map = std::unordered_multimap<const void *, instance *>;
 
-inline bool is_interpreter_alive() {
-#if PY_VERSION_HEX < 0x030D0000
-    return Py_IsInitialized() != 0 || _Py_IsFinalizing() != 0;
-#else
-    return Py_IsInitialized() != 0 || Py_IsFinalizing() != 0;
-#endif
-}
-
 #ifdef Py_GIL_DISABLED
 // Wrapper around PyMutex to provide BasicLockable semantics
 class pymutex {
@@ -293,11 +317,8 @@ struct internals {
 
     internals()
         : static_property_type(make_static_property_type()),
-          default_metaclass(make_default_metaclass()) {
+          default_metaclass(make_default_metaclass()), istate(get_interpreter_state_unchecked()) {
         tstate.set(nullptr); // See PR #5870
-        PyThreadState *cur_tstate = PyThreadState_Get();
-
-        istate = cur_tstate->interp;
         registered_exception_translators.push_front(&translate_exception);
 #ifdef Py_GIL_DISABLED
         // Scale proportional to the number of cores. 2x is a heuristic to reduce contention.
@@ -320,8 +341,10 @@ struct internals {
         // Normally this destructor runs during interpreter finalization and it may DECREF things.
         // In odd finalization scenarios it might end up running after the interpreter has
         // completely shut down, In that case, we should not decref these objects because pymalloc
-        // is gone.
+        // is gone.  This also applies across sub-interpreters, we should only DECREF when the
-        if (is_interpreter_alive()) {
+        // original owning interpreter is active.
+        auto *cur_istate = get_interpreter_state_unchecked();
+        if (cur_istate && cur_istate == istate) {
             Py_CLEAR(instance_base);
             Py_CLEAR(default_metaclass);
             Py_CLEAR(static_property_type);
@@ -336,6 +359,8 @@ struct internals {
 // impact any other modules, because the only things accessing the local internals is the
 // module that contains them.
 struct local_internals {
+    local_internals() : istate(get_interpreter_state_unchecked()) {}
+
     // It should be safe to use fast_type_map here because this entire
     // data structure is scoped to our single module, and thus a single
     // DSO and single instance of type_info for any particular type.
@@ -343,13 +368,16 @@ struct local_internals {
 
     std::forward_list<ExceptionTranslator> registered_exception_translators;
     PyTypeObject *function_record_py_type = nullptr;
+    PyInterpreterState *istate = nullptr;
 
     ~local_internals() {
         // Normally this destructor runs during interpreter finalization and it may DECREF things.
         // In odd finalization scenarios it might end up running after the interpreter has
         // completely shut down, In that case, we should not decref these objects because pymalloc
-        // is gone.
+        // is gone.  This also applies across sub-interpreters, we should only DECREF when the
-        if (is_interpreter_alive()) {
+        // original owning interpreter is active.
+        auto *cur_istate = get_interpreter_state_unchecked();
+        if (cur_istate && cur_istate == istate) {
             Py_CLEAR(function_record_py_type);
         }
     }
@@ -436,16 +464,6 @@ struct native_enum_record {
     "__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 to figure out if there are or have been multiple subinterpreters active at any
 /// point. This must never go from true to false while any interpreter may be running in any
 /// thread!
@@ -558,27 +576,6 @@ inline void translate_local_exception(std::exception_ptr p) {
 }
 #endif
 
-inline object get_python_state_dict() {
-    object state_dict;
-#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)
-    state_dict = reinterpret_borrow<object>(PyEval_GetBuiltins());
-#else
-#    if PY_VERSION_HEX < 0x03090000
-    PyInterpreterState *istate = _PyInterpreterState_Get();
-#    else
-    PyInterpreterState *istate = PyInterpreterState_Get();
-#    endif
-    if (istate) {
-        state_dict = reinterpret_borrow<object>(PyInterpreterState_GetDict(istate));
-    }
-#endif
-    if (!state_dict) {
-        raise_from(PyExc_SystemError, "pybind11::detail::get_python_state_dict() FAILED");
-        throw error_already_set();
-    }
-    return state_dict;
-}
-
 // Get or create per-storage capsule in the current interpreter's state dict.
 //   - The storage is interpreter-dependent: different interpreters will have different storage.
 //     This is important when using multiple-interpreters, to avoid sharing unshareable objects

include/pybind11/subinterpreter.h

@@ -20,15 +20,6 @@
 #endif
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
-PYBIND11_NAMESPACE_BEGIN(detail)
-inline PyInterpreterState *get_interpreter_state_unchecked() {
-    auto *cur_tstate = get_thread_state_unchecked();
-    if (cur_tstate) {
-        return cur_tstate->interp;
-    }
-    return nullptr;
-}
-PYBIND11_NAMESPACE_END(detail)
 
 class subinterpreter;