From a94a0ece0dee77cdf4bd0416a7a220cda6e470f1 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Oct 2026 10:42:12 -0400 Subject: [PATCH 1/2] fix: only cache the internals pp after a successful lookup In get_pp(), last_istate_tls was set before get_or_create_pp_in_state_dict(). If that call threw, the thread cache matched the interpreter but held a null pp, so later calls returned nullptr ("get_internals: get_pp() returned nullptr"). Seen intermittently on MinGW in test_import_in_subinterpreter_concurrently. Assisted-by: ClaudeCode:claude-opus-5-5 --- include/pybind11/detail/internals.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 164da39b5c..d755b28517 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -650,8 +650,11 @@ class internals_pp_manager { if (!tstate) { tstate = get_thread_state_unchecked(); } + // Update the cache only on success; a stale interp with a null pp would make + // later calls return nullptr. + auto *pp = get_or_create_pp_in_state_dict(); last_istate_tls() = tstate->interp; - internals_p_tls() = get_or_create_pp_in_state_dict(); + internals_p_tls() = pp; } return internals_p_tls(); } From df617e5f86e6bb5bdab01fe51bcc5d25a550f8ae Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Oct 2026 13:58:10 -0700 Subject: [PATCH 2/2] test: cover internals cache retries after failed lookup --- tests/test_with_catch/test_subinterpreter.cpp | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_with_catch/test_subinterpreter.cpp b/tests/test_with_catch/test_subinterpreter.cpp index 570519e641..52b18aa6b7 100644 --- a/tests/test_with_catch/test_subinterpreter.cpp +++ b/tests/test_with_catch/test_subinterpreter.cpp @@ -13,6 +13,8 @@ PYBIND11_WARNING_DISABLE_MSVC(4996) # include # include # include +# include +# include # include # include @@ -42,6 +44,53 @@ void unsafe_reset_internals_for_single_interpreter() { py::detail::get_local_internals(); } +TEST_CASE("Internals cache retries after a failed lookup") { + struct test_internals { + bool fail_next_fetch = true; + }; + using manager_type = py::detail::internals_pp_manager; + constexpr const char *key = "_pybind11_test_internals_cache_lookup_retry"; + auto &manager = manager_type::get_instance(key, [](test_internals *internals) { + if (internals && internals->fail_next_fetch) { + internals->fail_next_fetch = false; + throw std::bad_alloc(); + } + }); + struct reset_guard { + manager_type &manager; + ~reset_guard() { + manager.unref(); + unsafe_reset_internals_for_single_interpreter(); + } + } reset{manager}; + + // Creating a subinterpreter enables the per-thread interpreter cache. + auto sub = py::subinterpreter::create(); + manager.unref(); + + auto check_failed_lookup = [&]() { + py::subinterpreter_scoped_activate activate(sub); + // Prepopulate the capsule so that the lookup invokes on_fetch instead of creating it. + auto *expected_pp + = py::detail::atomic_get_or_create_in_state_dict>(key) + .first; + expected_pp->reset(new test_internals()); + REQUIRE_THROWS_AS(manager.get_pp(), std::bad_alloc); + + // A failed lookup must neither cache nullptr nor retain another interpreter's pointer. + REQUIRE(manager.get_pp() == expected_pp); + }; + + SECTION("Initially empty cache") { check_failed_lookup(); } + SECTION("Cached pointer from another interpreter") { + auto *main_pp + = py::detail::atomic_get_or_create_in_state_dict>(key) + .first; + REQUIRE(manager.get_pp() == main_pp); + check_failed_lookup(); + } +} + py::object &get_dict_type_object() { PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store storage; return storage