From d5abc4abb6b0ab9c2b910db6911b63caa54c830f Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 1 Sep 2026 23:25:08 -0400 Subject: [PATCH 1/2] fix: reset gil_safe_call_once_and_store cache on interpreter finalize Fixes item 3 of #6159. --- include/pybind11/gil_safe_call_once.h | 24 ++++++++++ tests/test_with_catch/test_interpreter.cpp | 52 ++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h index af26272d48..d733683c85 100644 --- a/include/pybind11/gil_safe_call_once.h +++ b/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 +class gil_safe_call_once_and_store; + PYBIND11_NAMESPACE_BEGIN(detail) template @@ -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 *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(storage)); + } if (is_initialized) { if (finalize != nullptr) { finalize(*reinterpret_cast(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(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(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; + 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 { diff --git a/tests/test_with_catch/test_interpreter.cpp b/tests/test_with_catch/test_interpreter.cpp index e39f51c274..59b9c7d715 100644 --- a/tests/test_with_catch/test_interpreter.cpp +++ b/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(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"); } @@ -370,6 +379,49 @@ TEST_CASE("Enum module survives restart") { // Added in PR #6015 REQUIRE(enum_mod.attr("SomeEnum").attr("value2").attr("name").cast() == "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 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(); + + 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). + 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(); + + 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` // should still function by using reasonable globals: `__main__.__dict__`. From 67b0007dd155e088179dd42147092f96b6c135db Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 2 Sep 2026 13:46:38 -0400 Subject: [PATCH 2/2] test: do not hold module references across an interpreter restart The old reference was released into the new interpreter's allocator, which crashed on Python 3.12. This was also the cause of the enum test skip. --- tests/test_with_catch/test_interpreter.cpp | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_with_catch/test_interpreter.cpp b/tests/test_with_catch/test_interpreter.cpp index 59b9c7d715..a72f8c1c94 100644 --- a/tests/test_with_catch/test_interpreter.cpp +++ b/tests/test_with_catch/test_interpreter.cpp @@ -366,16 +366,17 @@ 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"); - - auto enum_mod = py::module_::import("enum_module"); - REQUIRE(enum_mod.attr("SomeEnum").attr("value1").attr("name").cast() == "value1"); + { + // 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() + == "value1"); + } py::finalize_interpreter(); py::initialize_interpreter(); - enum_mod = py::module_::import("enum_module"); + auto enum_mod = py::module_::import("enum_module"); REQUIRE(enum_mod.attr("SomeEnum").attr("value2").attr("name").cast() == "value2"); } @@ -409,14 +410,17 @@ 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). - 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"); + { + // 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(); - exc_mod = py::module_::import("exception_module"); + 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"); }