Move the unref to below the multi-interpreter check.

[b-pass]

Description

This is a fix for #6174.

AI's assessment seems plausible

Thread A observes has_seen_non_main_interpreter() == false in get_pp() and prepares to return internals_singleton_pp_.
A non-main-interpreter thread B enters ensure_internals(), still observes the flag as false in unref(), and clears internals_singleton_pp_.
Thread A returns the now-null shared member.

Publishing has_seen_non_main_interpreter() = true before calling unref() in a non-main interpreter would cause that unref() to clear only the thread-local cache instead of the shared singleton.

Moving the unref makes sense. We cannot outright remove the unref, there is a case (covered by the "Restart the interpreter" unit test) which crashes without it.

Suggested changelog entry:

• Fixed occasional test failure under heavy threading load (especially on windows)

---

📚 Documentation preview 📚: https://pybind11--6195.org.readthedocs.build/

[henryiii]

I ran a fable 5.1 xhigh review on this. This also might be related to #6190.

🤖 AI text below 🤖

Top findings

1. The reorder does not explain [BUG]: Intermittent null internals_pp_manager::get_pp() during concurrent subinterpreter imports on Windows #6174. In the subprocess test, only sub-interpreter threads touch the DSO, and each one sets the non-main flag before its first get_pp() call. No thread takes the singleton branch, so the nulled singleton was never readable in that test on master. The Windows nullptr likely comes from another path. Suggestion: keep [BUG]: Intermittent null internals_pp_manager::get_pp() during concurrent subinterpreter imports on Windows #6174 open and add a repro or stress test rather than closing it with this PR.

2. Residual race remains in get_pp() at include/pybind11/detail/internals.h:663. It re-reads the non-atomic internals_singleton_pp_ after the null check, and every main-interpreter ensure_internals() still nulls it via unref(). A concrete GIL-less reader exists: gil_scoped_acquire calls get_internals() at gil.h:72 before holding the GIL. Fix: load the member into a local once and return that, or make it atomic, or have ensure_internals() fetch-then-swap.

3. TLS branch ordering bug at internals.h:653. last_istate_tls() is assigned before the fallible get_or_create_pp_in_state_dict(). If that throws, the interpreter stays cached with a null pointer and every later call returns nullptr. Assign internals_p_tls() first. This is the second path [BUG]: Intermittent null internals_pp_manager::get_pp() during concurrent subinterpreter imports on Windows #6174 names and the PR does not touch it.

Other findings

• PYBIND11_PLUGIN at common.h:447 expands PYBIND11_ENSURE_INTERNALS_READY outside the try/catch block, so a throw from ensure_internals() escapes an extern "C" function and terminates. Move it inside the try like PYBIND11_MODULE_PYINIT.
• The new PYBIND11_ENSURE_INTERNALS_READY body calls PyInterpreterState_Get() first, which fatals without a current thread state. The old body worked without the GIL. Guard on a null tstate or acquire the GIL first.
• finalize_interpreter() at embed.h:192 resets the flag and then reads the singleton without a second unref(). With the new order the singleton may hold a stale sub-interpreter pp, so finalize destroys the wrong internals and leaks main's. The Catch helper does the second unref(); finalize_interpreter does not.
• No regression test. Both CLAUDE.md files require one. Suggested: a Catch case in tests/test_with_catch/test_subinterpreter.cpp where the main thread loops get_internals() while a thread imports external_module in a fresh sub-interpreter.
• unsafe_reset_local_internals() in tests/test_with_catch/external_module.cpp is now dead code since ensure_internals() already does that unref.
• The new comment in internals-inl.h:214 says "null it out of our global copy", which is wrong in multi-interpreter mode, and it omits the invariant that the flag must be published before unref().

The review treats the reorder itself as correct hardening. The main risks are the unexplained root cause of #6174, the residual non-atomic read, and the finalize_interpreter stale-singleton path. Tell me which of these you want fixed and I will start with the regression test.

Is this related to #6190?

Yes. PR #6190 (merged) is exactly the review's third finding: it moves last_istate_tls() assignment after get_or_create_pp_in_state_dict() succeeds, so a throw no longer leaves a null pp cached. The reviewer flagged it as "untouched by this PR" because this branch does not yet include #6190.

The two PRs are complementary, not duplicates:

• fix: only cache the internals pp after a successful lookup #6190 fixes the TLS-branch path (sub-interpreter mode) and was proposed as a probable cause of the MinGW nullptr in feat: move remaining non-template definitions to -inl.h files #6189.
• Move the unref to below the multi-interpreter check. #6195 (this branch) fixes the singleton-branch path, where a sub-interpreter import nulled the shared singleton under main-interpreter readers.

Rebasing this branch on master picks up #6190 and clears finding 3. Finding 1 (the review's claim that the reorder cannot reproduce the #6174 test failure) still stands, and #6190's own description also says its explanation of the MinGW failure is unconfirmed. So neither PR has a confirmed repro of #6174 yet. The remaining open items are the non-atomic re-read in get_pp(), the finalize_interpreter stale-singleton path, and a regression test for this branch's reorder.

[b-pass]

So it's an AI battle, then .... Opus 5.5:

That review is wrong, and the PR is the right fix, though its description doesn't explain why. Since CPython 3.13, import_run_extension() in Python/import.c calls switch_to_main_interpreter() before running any extension's PyInit_*. When a subinterpreter imports a module, PyInit_ runs on the importing thread with the main interpreter's thread state and the main GIL. CPython then switches back and runs the multi-phase exec slot in the subinterpreter, under that subinterpreter's own GIL. (Verified: 3.13 and 3.14 have it, 3.12 does not.)

The flag still gets set in that subinterpreter, because the exec slot also calls ensure_internals(). PYBIND11_MODULE_EXEC in common.h expands PYBIND11_ENSURE_INTERNALS_READY. Python runs the exec slot later, in the subinterpreter, through imp.exec_dynamic → PyModule_ExecDef. py::subinterpreter (subinterpreter.h:123) also sets the flag, and so does PyInit on 3.12. So no new check is needed.

On master, before anyone has set the flag:

Worker B1, inside PyInit_ (main interpreter, holding the main GIL): ensure_internals() sees PyInterpreterState_Get() == Main, so it does not set the flag. get_pp() takes the singleton branch, assigns internals_singleton_pp_, then reads it again in return.
Worker B2, inside its exec slot (subinterpreter, holding a different GIL): ensure_internals() calls unref() while the flag is still false, so it writes internals_singleton_pp_ = nullptr.
If that write lands between B1's assignment (or null check) and its return read, B1 returns nullptr and the import fails with ImportError. The window exists only until the first exec slot sets the flag, and it is widest in unoptimized (Debug) builds that re-read memory. That fits a rare Windows Debug flake.
After the PR, a subinterpreter publishes the flag before unref(), so it only clears its own TLS. Only main-interpreter code under the main GIL ever writes the singleton. That removes the cross-GIL writer.

So the ENSURE_INTERNALS_READY call in PYBIND11_PLUGIN_IMPL is always happening in the context of the main interpreter. We/I didn't intend that ... so I've removed that one, it wasn't even needed there. There was already one in the exec slot, that one is correct.

[rwgk]

I just merged master; there were no conflicts.

[rwgk]

I'll add the suggested tests.

The build and test log links below are broken, but I'm leaving the codex output as-is for simplicity.

---

codex GPT-6.1-Sol ultra (session ID 01a11ceb-7bdc-78f0-9756-7fc38f0a1ef8):

The change looks sound, and I found three deterministic tests that show what it adds after #6190. I reviewed pushed head 71911491 against master 688e7430, which already contains #6190.

The initialization reasoning checks out. CPython 3.13+ temporarily switches to the main interpreter for PyInit_*, then switches back before module creation. Consequently, even the subinterpreter-only workload in #6174 can reach the singleton cache through the old early initialization. Removing that initialization is justified; publishing the multi-interpreter flag before unref() then ensures the subinterpreter clears its thread-local cache. CPython import sequence, switch back, updated initialization.

I would add these tests:

1. Module definition and creation leave internals uninitialized.
   In the Catch2 embedding tests, start with a fresh interpreter. Call a dedicated module’s PyInit_*, then create its module object while deferring execution. Assert that no internals capsule exists and the binding initializer has not run. Execute the module; assert that internals now exist, initialization ran exactly once, and a bound callable works.

   My probe fails on master because PyInit_* eagerly creates internals. It passes on the PR. This directly covers the removed early call.

2. The first subinterpreter transition preserves the main singleton cache.
   Prime the main interpreter’s cached holder, then create a subinterpreter through raw CPython APIs. Call ensure_internals() there as the first pybind operation. Check that it enables multi-interpreter mode and obtains a distinct holder.

   To detect whether the main singleton was cleared, my probe destroys the subinterpreter, clears TLS, resets the flag, and temporarily removes the main capsule: the preserved cache must still return the original holder. Restore the capsule and caches with cleanup guards. This should run in isolation, with no worker threads.

   Master loses the cached holder; the PR preserves it. Using py::subinterpreter::create() would mask this behavior because that helper sets the flag itself.

3. A separate extension refreshes its local cache after interpreter restart.
   Remove the manual reset in external_module.cpp, or use a small fixture without it. Before and after restarting, assert that its cached local holder matches its capsule in the current interpreter’s state dictionary, then exercise its class and callable bindings.

   This is the clearest existing coverage gap: the fixture currently supplies the reset that Move the unref to below the multi-interpreter check. #6195 adds automatically. My probe detects a stale local cache on master after restart and passes on the PR.

All three probes failed on master and passed on the PR, using regular CPython 3.14 and free-threaded 3.14 with PYTHON_GIL=0:

Environment	Build log	Probe results
Regular	Build	Results
Free-threaded	Build	Results

I would retain the existing concurrent-import stress test and run it on the recorded Windows Debug configuration. These deterministic tests establish the changed behavior; they do not identify which race caused the historical #6174 occurrence. General singleton-cache concurrency also remains a broader concern because its pointer accesses are still unsynchronized.

The repository is unchanged. The diagnostic source is available for turning these outlines into repository tests.

[rwgk]

EDIT: The below was a flake. A rerun worked.

FAILED:

CI / 🐍 (windows-latest, 3.9, -DPYBIND11_TEST_SMART_HOLDER=ON) / 🧪

  [ STARTING ] 2026-10-08 20:42:12.537Z
  [ PYTHON   ] 3.9.13 (tags/v3.9.13:6de2ca5, May 17 2022, 16:36:42) [MSC v.1929 64 bit (AMD64)]
  [ RUN      ] check sample args_convert_vector contents
  [       OK ] check sample args_convert_vector contents
  [ RUN      ] args_convert_vector push_back
  [       OK ] args_convert_vector push_back
  [ RUN      ] args_convert_vector reserve
  [       OK ] args_convert_vector reserve
  [ RUN      ] args_convert_vector reserve then push_back
  [       OK ] args_convert_vector reserve then push_back
  [ RUN      ] check sample argument_vector contents
  [       OK ] check sample argument_vector contents
  [ RUN      ] argument_vector push_back
  [       OK ] argument_vector push_back
  [ RUN      ] argument_vector reserve
  [       OK ] argument_vector reserve
  [ RUN      ] argument_vector reserve then push_back
  [       OK ] argument_vector reserve then push_back
  [ RUN      ] PYTHONPATH is used to update sys.path
  [       OK ] PYTHONPATH is used to update sys.path
  [ RUN      ] Pass classes and data between modules defined in C++ and Python
  [       OK ] Pass classes and data between modules defined in C++ and Python
  [ RUN      ] Override cache
  [       OK ] Override cache
  [ RUN      ] Import error handling
  [       OK ] Import error handling
  [ RUN      ] There can be only one interpreter
  [       OK ] There can be only one interpreter
  [ RUN      ] Custom PyConfig
  [       OK ] Custom PyConfig
  [ RUN      ] scoped_interpreter with PyConfig_InitIsolatedConfig and argv
  [       OK ] scoped_interpreter with PyConfig_InitIsolatedConfig and argv
  [ RUN      ] scoped_interpreter with PyConfig_InitPythonConfig and argv
  [       OK ] scoped_interpreter with PyConfig_InitPythonConfig and argv
  [ RUN      ] Add program dir to path without PyConfig
  [       OK ] Add program dir to path without PyConfig
  [ RUN      ] Add program dir to path using PyConfig
  [       OK ] Add program dir to path using PyConfig
  [ RUN      ] Internals are initialized when a module executes
  [       OK ] Internals are initialized when a module executes
  [ RUN      ] Restart the interpreter
  [       OK ] Restart the interpreter
  [ RUN      ] Enum module survives restart
  [       OK ] Enum module survives restart
  [ RUN      ] Execution frame
  [       OK ] Execution frame
  [ RUN      ] Threads
  [  FAILED  ] Threads
  [  FAILED  ] 1 of 23 test cases, 11295 assertions.
  Windows fatal exception: access violation
  
  Current thread 0x00002184 (most recent call first):
  <no Python frame>
C:\Program Files\Microsoft Visual Studio\18\Enterprise\MSBuild\Microsoft\VC\v180\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\pybind11\pybind11\build\CMakeFiles\f764e9d74f815559787dd23c8fb34bd9\cpptest.rule;D:\a\pybind11\pybind11\tests\test_with_catch\CMakeLists.txt' exited with code -1073741819. [D:\a\pybind11\pybind11\build\tests\test_with_catch\cpptest.vcxproj]
Done Building Project "D:\a\pybind11\pybind11\build\tests\test_with_catch\cpptest.vcxproj" (default targets) -- FAILED.

Build FAILED.

"D:\a\pybind11\pybind11\build\tests\test_with_catch\cpptest.vcxproj" (default target) (1) ->
(CustomBuild target) -> 
  C:\Program Files\Microsoft Visual Studio\18\Enterprise\MSBuild\Microsoft\VC\v180\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\pybind11\pybind11\build\CMakeFiles\f764e9d74f815559787dd23c8fb34bd9\cpptest.rule;D:\a\pybind11\pybind11\tests\test_with_catch\CMakeLists.txt' exited with code -1073741819. [D:\a\pybind11\pybind11\build\tests\test_with_catch\cpptest.vcxproj]

    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:01.36

[rwgk]

Looks good to me.

@b-pass, @henryiii I'm intentionally not merging, to give you the opportunity to review the added tests.

[b-pass]

Looks good to me