Reapply to leak internals

XuehaiPan · XuehaiPan · commit cdefbf3547af · 2026-01-25T21:20:07.000+08:00
This reverts commit 8f25a25.
diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h
@@ -337,20 +337,7 @@ struct internals {
     internals(internals &&other) = delete;
     internals &operator=(const internals &other) = delete;
     internals &operator=(internals &&other) = delete;
-    ~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.  This also applies across sub-interpreters, we should only DECREF when the
-        // original owning interpreter is active.
-        auto *cur_istate = get_interpreter_state_unchecked();
-        if (cur_istate && cur_istate == istate) {
-            gil_scoped_acquire_simple gil;
-            Py_CLEAR(instance_base);
-            Py_CLEAR(default_metaclass);
-            Py_CLEAR(static_property_type);
-        }
-    }
+    ~internals() = default;
 };
 
 // the internals struct (above) is shared between all the modules. local_internals are only
@@ -360,29 +347,13 @@ 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.
     fast_type_map<type_info *> registered_types_cpp;
 
     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.  This also applies across sub-interpreters, we should only DECREF when the
-        // original owning interpreter is active.
-        auto *cur_istate = get_interpreter_state_unchecked();
-        if (cur_istate && cur_istate == istate) {
-            gil_scoped_acquire_simple gil;
-            Py_CLEAR(function_record_py_type);
-        }
-    }
 };
 
 enum class holder_enum_t : uint8_t {
@@ -580,7 +551,7 @@ inline void translate_local_exception(std::exception_ptr p) {
 
 // Sentinel value for the `dtor` parameter of `atomic_get_or_create_in_state_dict`.
 // Indicates no destructor was explicitly provided (distinct from nullptr, which means "leak").
-#define PYBIND11_DTOR_UNSPECIFIED (reinterpret_cast<void (*)(PyObject *)>(1))
+#define PYBIND11_DTOR_USE_DELETE (reinterpret_cast<void (*)(PyObject *)>(1))
 
 // Get or create per-storage capsule in the current interpreter's state dict.
 //   - The storage is interpreter-dependent: different interpreters will have different storage.
@@ -605,7 +576,7 @@ inline void translate_local_exception(std::exception_ptr p) {
 template <typename Payload>
 std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
                                                               void (*dtor)(PyObject *)
-                                                              = PYBIND11_DTOR_UNSPECIFIED) {
+                                                              = PYBIND11_DTOR_USE_DELETE) {
     error_scope err_scope; // preserve any existing Python error states
 
     auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
@@ -651,7 +622,7 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
         // - Otherwise, our `new_capsule` is now in the dict, and it owns the storage and the state
         //   dict will incref it.  We need to set the caller's destructor on it, which will be
         //   called when the interpreter shuts down.
-        if (created && dtor != PYBIND11_DTOR_UNSPECIFIED) {
+        if (created && dtor != PYBIND11_DTOR_USE_DELETE) {
             if (PyCapsule_SetDestructor(capsule_obj, dtor) < 0) {
                 throw error_already_set();
             }
@@ -668,7 +639,7 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
     return std::pair<Payload *, bool>(static_cast<Payload *>(raw_ptr), created);
 }
 
-#undef PYBIND11_DTOR_UNSPECIFIED
+#undef PYBIND11_DTOR_USE_DELETE
 
 template <typename InternalsType>
 class internals_pp_manager {
@@ -741,27 +712,16 @@ class internals_pp_manager {
     internals_pp_manager(char const *id, on_fetch_function *on_fetch)
         : holder_id_(id), on_fetch_(on_fetch) {}
 
-    static void internals_shutdown(PyObject *capsule) {
-        auto *pp = static_cast<std::unique_ptr<InternalsType> *>(
-            PyCapsule_GetPointer(capsule, nullptr));
-        if (pp) {
-            pp->reset();
-        }
-        // We reset the unique_ptr's contents but cannot delete the unique_ptr itself here.
-        // The pp_manager in this module (and possibly other modules sharing internals) holds
-        // a raw pointer to this unique_ptr, and that pointer would dangle if we deleted it now.
-        //
-        // For pybind11-owned interpreters (via embed.h or subinterpreter.h), destroy() is
-        // called after Py_Finalize/Py_EndInterpreter completes, which safely deletes the
-        // unique_ptr. For interpreters not owned by pybind11 (e.g., a pybind11 extension
-        // loaded into an external interpreter), destroy() is never called and the unique_ptr
-        // shell (8 bytes, not its contents) is leaked.
-        // (See PR #5958 for ideas to eliminate this leak.)
-    }
-
     std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
+        // The `unique_ptr<InternalsType>` is intentionally leaked on interpreter shutdown.
+        // Once an instance is created, it will never be deleted until the process exits (compare
+        // to interpreter shutdown in multiple-interpreter scenarios).
+        // We cannot guarantee the destruction order of capsules in the interpreter state dict on
+        // interpreter shutdown, so deleting internals too early could cause undefined behavior
+        // when other pybind11 objects access `get_internals()` during finalization (which would
+        // recreate empty internals).
         auto result = atomic_get_or_create_in_state_dict<std::unique_ptr<InternalsType>>(
-            holder_id_, &internals_shutdown);
+            holder_id_, /*dtor=*/nullptr /* leak the capsule content */);
         auto *pp = result.first;
         bool created = result.second;
         // Only call on_fetch_ when fetching existing internals, not when creating new ones.
@@ -841,8 +801,6 @@ PYBIND11_NOINLINE internals &get_internals() {
 
 /// Return the PyObject* for the internals capsule (borrowed reference).
 /// Returns nullptr if the capsule doesn't exist yet.
-/// This is used to prevent use-after-free during interpreter shutdown by allowing pybind11 types
-/// to hold a reference to the capsule (see comments in generic_type::initialize).
 inline PyObject *get_internals_capsule() {
     auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
     return dict_getitemstring(state_dict.ptr(), PYBIND11_INTERNALS_ID);
@@ -860,8 +818,6 @@ inline const std::string &get_local_internals_key() {
 
 /// Return the PyObject* for the local_internals capsule (borrowed reference).
 /// Returns nullptr if the capsule doesn't exist yet.
-/// This is used to prevent use-after-free during interpreter shutdown by allowing pybind11 types
-/// to hold a reference to the capsule (see comments in generic_type::initialize).
 inline PyObject *get_local_internals_capsule() {
     const auto &key = get_local_internals_key();
     auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -1733,31 +1733,6 @@ class generic_type : public object {
             PYBIND11_WARNING_POP
         });
 
-        // Prevent use-after-free during interpreter shutdown. GC order is not guaranteed, so the
-        // internals capsule may be destroyed (resetting internals via internals_shutdown) before
-        // all pybind11 types are destroyed. If a type's tp_traverse/tp_clear then calls py::cast,
-        // it would recreate an empty internals and fail because the type registry is gone.
-        //
-        // By holding references to the capsules, we ensure they outlive all pybind11 types. We use
-        // weakrefs on the type with a cpp_function callback. When the type is destroyed, Python
-        // will call the callback which releases the capsule reference and the weakref.
-        if (PyObject *capsule = get_internals_capsule()) {
-            Py_INCREF(capsule);
-            (void) weakref(handle(m_ptr), cpp_function([](handle wr) -> void {
-                               Py_XDECREF(get_internals_capsule());
-                               wr.dec_ref();
-                           }))
-                .release();
-        }
-        if (PyObject *capsule = get_local_internals_capsule()) {
-            Py_INCREF(capsule);
-            (void) weakref(handle(m_ptr), cpp_function([](handle wr) -> void {
-                               Py_XDECREF(get_local_internals_capsule());
-                               wr.dec_ref();
-                           }))
-                .release();
-        }
-
         if (rec.bases.size() > 1 || rec.multiple_inheritance) {
             mark_parents_nonsimple(tinfo->type);
             tinfo->simple_ancestors = false;
diff --git a/tests/test_custom_type_setup.cpp b/tests/test_custom_type_setup.cpp
@@ -36,9 +36,9 @@ void add_gc_checkers_with_weakrefs(const py::object &obj) {
     if (!global_capsule) {
         throw std::runtime_error("No global internals capsule found");
     }
-    (void) py::weakref(obj, py::cpp_function([global_capsule, obj](py::handle weakref) -> void {
-                           py::handle new_global_capsule = py::detail::get_internals_capsule();
-                           if (!new_global_capsule.is(global_capsule)) {
+    (void) py::weakref(obj, py::cpp_function([global_capsule](py::handle weakref) -> void {
+                           py::handle current_global_capsule = py::detail::get_internals_capsule();
+                           if (!current_global_capsule.is(global_capsule)) {
                                throw std::runtime_error(
                                    "Global internals capsule was destroyed prematurely");
                            }
@@ -51,9 +51,9 @@ void add_gc_checkers_with_weakrefs(const py::object &obj) {
         throw std::runtime_error("No local internals capsule found");
     }
     (void) py::weakref(
-        obj, py::cpp_function([local_capsule, obj](py::handle weakref) -> void {
-            py::handle new_local_capsule = py::detail::get_local_internals_capsule();
-            if (!new_local_capsule.is(local_capsule)) {
+        obj, py::cpp_function([local_capsule](py::handle weakref) -> void {
+            py::handle current_local_capsule = py::detail::get_local_internals_capsule();
+            if (!current_local_capsule.is(local_capsule)) {
                 throw std::runtime_error("Local internals capsule was destroyed prematurely");
             }
             weakref.dec_ref();
diff --git a/tests/test_custom_type_setup.py b/tests/test_custom_type_setup.py
@@ -58,6 +58,7 @@ def test_indirect_cycle(gc_tester):
 )
 @pytest.mark.skipif("env.PYPY or env.GRAALPY")
 def test_py_cast_useable_on_shutdown():
+    """Test that py::cast works during interpreter shutdown."""
     env.check_script_success_in_subprocess(
         f"""
         import sys
@@ -67,8 +68,14 @@ def test_py_cast_useable_on_shutdown():
 
         from pybind11_tests import custom_type_setup as m
 
+        # Create a self-referential cycle that will be collected during shutdown.
+        # The tp_traverse and tp_clear callbacks call py::cast, which requires
+        # internals to still be valid.
         obj = m.ContainerOwnsPythonObjects()
         obj.append(obj)
+
+        # Add weakref callbacks that verify the capsule is still alive when the
+        # pybind11 object is garbage collected during shutdown.
         m.add_gc_checkers_with_weakrefs(obj)
         """
     )
