fix: only cache the internals pp after a successful lookup

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();
         }

tests/test_with_catch/test_subinterpreter.cpp

@@ -13,6 +13,8 @@ PYBIND11_WARNING_DISABLE_MSVC(4996)
 #    include <cstdlib>
 #    include <fstream>
 #    include <functional>
+#    include <memory>
+#    include <new>
 #    include <thread>
 #    include <utility>
 
@@ -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<test_internals>;
+    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<std::unique_ptr<test_internals>>(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<std::unique_ptr<test_internals>>(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<py::object> storage;
     return storage