fix: reset gil_safe_call_once_and_store cache on interpreter finalize

include/pybind11/gil_safe_call_once.h

@@ -135,6 +135,9 @@ class gil_safe_call_once_and_store {
 // subinterpreter has its own separate state. The cached result may not shareable across
 // interpreters (e.g., imported modules and their members).
 
+template <typename T>
+class gil_safe_call_once_and_store;
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 template <typename T>
@@ -143,9 +146,16 @@ struct call_once_storage {
     std::once_flag once_flag;
     void (*finalize)(T &) = nullptr;
     std::atomic_bool is_initialized{false};
+    // The `gil_safe_call_once_and_store` which caches a pointer into `storage`, if any.
+    // It is a global static, therefore it outlives this storage, which the interpreter owns.
+    gil_safe_call_once_and_store<T> *owner = nullptr;
 
     call_once_storage() = default;
     ~call_once_storage() {
+        // The interpreter destroys this storage, therefore the owner must forget its cache.
+        if (owner != nullptr) {
+            owner->invalidate_cache(reinterpret_cast<T *>(storage));
+        }
         if (is_initialized) {
             if (finalize != nullptr) {
                 finalize(*reinterpret_cast<T *>(storage));
@@ -193,6 +203,8 @@ class gil_safe_call_once_and_store {
                 ::new (value->storage) T(fn());
                 value->finalize = finalize_fn;
                 value->is_initialized = true;
+                // Let the storage reset the cache below when the interpreter destroys it.
+                value->owner = this;
                 // Publish the cached pointer before setting the validity flag, so that any reader
                 // which observes the flag as true is guaranteed to also observe this pointer.
                 last_storage_ptr_ = reinterpret_cast<T *>(value->storage);
@@ -216,6 +228,7 @@ class gil_safe_call_once_and_store {
             gil_scoped_acquire gil_acq;
             auto *value = get_or_create_storage_in_state_dict();
             result = reinterpret_cast<T *>(value->storage);
+            value->owner = this;
             last_storage_ptr_ = result;
         }
         assert(result != nullptr);
@@ -236,6 +249,17 @@ class gil_safe_call_once_and_store {
 
 private:
     using storage_type = detail::call_once_storage<T>;
+    friend storage_type;
+
+    // Called from the destructor of the storage, i.e. when the interpreter which owns the storage
+    // is finalized. The flag is cleared first, so that a reader which observes the flag as true
+    // never loads a null pointer.
+    void invalidate_cache(T *storage_ptr) {
+        if (last_storage_ptr_.load() == storage_ptr) {
+            is_initialized_by_at_least_one_interpreter_ = false;
+            last_storage_ptr_ = nullptr;
+        }
+    }
 
     // Indicator of fast path for single-interpreter case.
     bool is_last_storage_valid() const {

tests/test_with_catch/test_interpreter.cpp

@@ -95,6 +95,15 @@ PYBIND11_EMBEDDED_MODULE(enum_module, m, py::multiple_interpreters::per_interpre
         .value("value2", SomeEnum::value2);
 }
 
+class SomeCppException : public std::runtime_error { // Added for gh-6159
+    using std::runtime_error::runtime_error;
+};
+
+PYBIND11_EMBEDDED_MODULE(exception_module, m, py::multiple_interpreters::per_interpreter_gil()) {
+    py::register_exception<SomeCppException>(m, "SomeCppException");
+    m.def("raise_it", []() { throw SomeCppException("C++ Error"); });
+}
+
 PYBIND11_EMBEDDED_MODULE(throw_exception, , py::multiple_interpreters::not_supported()) {
     throw std::runtime_error("C++ Error");
 }
@@ -357,18 +366,65 @@ TEST_CASE("Enum module survives restart") { // Added in PR #6015
     // calls process_attributes::init after initialize_generic's strdup loop,
     // leaving arg names as string literals. Without the fix, destruct() would
     // call free() on those literals during interpreter finalization.
-    PYBIND11_CATCH2_SKIP_IF(PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION == 12,
-                            "Pre-existing crash in enum cleanup during finalize on Python 3.12");
+    {
+        // Scoped: the reference must not outlive the interpreter which owns it.
+        auto enum_mod = py::module_::import("enum_module");
+        REQUIRE(enum_mod.attr("SomeEnum").attr("value1").attr("name").cast<std::string>()
+                == "value1");
+    }
+
+    py::finalize_interpreter();
+    py::initialize_interpreter();
 
     auto enum_mod = py::module_::import("enum_module");
-    REQUIRE(enum_mod.attr("SomeEnum").attr("value1").attr("name").cast<std::string>() == "value1");
+    REQUIRE(enum_mod.attr("SomeEnum").attr("value2").attr("name").cast<std::string>() == "value2");
+}
+
+#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
+// Added for gh-6159: the per-interpreter storage is destroyed with the interpreter, therefore the
+// fast-path cache in `gil_safe_call_once_and_store` must be reset, too.
+PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> restart_test_storage;
+static int restart_test_call_count = 0;
+
+TEST_CASE("gil_safe_call_once_and_store survives restart") { // Added for gh-6159
+    auto import_sys_modules = []() {
+        restart_test_call_count++;
+        return py::module_::import("sys").attr("modules");
+    };
+
+    auto &first = restart_test_storage.call_once_and_store_result(import_sys_modules).get_stored();
+    REQUIRE(restart_test_call_count == 1);
+    REQUIRE(first.ptr() == py::module_::import("sys").attr("modules").ptr());
 
     py::finalize_interpreter();
     py::initialize_interpreter();
 
-    enum_mod = py::module_::import("enum_module");
-    REQUIRE(enum_mod.attr("SomeEnum").attr("value2").attr("name").cast<std::string>() == "value2");
+    auto &second
+        = restart_test_storage.call_once_and_store_result(import_sys_modules).get_stored();
+    // The stored value belongs to the finalized interpreter, therefore it must be stored again.
+    REQUIRE(restart_test_call_count == 2);
+    REQUIRE(second.ptr() == py::module_::import("sys").attr("modules").ptr());
+}
+
+TEST_CASE("Exception module survives restart") { // Added for gh-6159
+    // Regression test for item 3 of gh-6159: `py::register_exception` stores the Python exception
+    // type in a `gil_safe_call_once_and_store`. Without the fix, the stale cache made the second
+    // import a no-op, and the module had no `SomeCppException` attribute (use after free).
+    {
+        // Scoped: the reference must not outlive the interpreter which owns it.
+        auto exc_mod = py::module_::import("exception_module");
+        REQUIRE(py::hasattr(exc_mod, "SomeCppException"));
+        REQUIRE_THROWS_WITH(exc_mod.attr("raise_it")(), "SomeCppException: C++ Error");
+    }
+
+    py::finalize_interpreter();
+    py::initialize_interpreter();
+
+    auto exc_mod = py::module_::import("exception_module");
+    REQUIRE(py::hasattr(exc_mod, "SomeCppException"));
+    REQUIRE_THROWS_WITH(exc_mod.attr("raise_it")(), "SomeCppException: C++ Error");
 }
+#endif
 
 TEST_CASE("Execution frame") {
     // When the interpreter is embedded, there is no execution frame, but `py::exec`