diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5569e3a92..22c1f489c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,7 +88,7 @@ jobs: cmake-args: -DCMAKE_CXX_STANDARD=20 -DPYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION=ON - runs-on: ubuntu-latest python-version: '3.14' - cmake-args: -DCMAKE_CXX_STANDARD=14 -DCMAKE_CXX_FLAGS="-DPYBIND11_HAS_SUBINTERPRETER_SUPPORT=0" + cmake-args: -DCMAKE_CXX_STANDARD=14 - runs-on: ubuntu-latest python-version: 'pypy-3.10' cmake-args: -DCMAKE_CXX_STANDARD=14 diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index ce9014a7e9..4406adf5b6 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -441,12 +441,11 @@ Note that this is run once for each (sub-)interpreter the module is imported int possibly concurrently. The PyModuleDef is allowed to be static, but the PyObject* resulting from PyModuleDef_Init should be treated like any other PyObject (so not shared across interpreters). */ -#define PYBIND11_MODULE_PYINIT(name, pre_init, ...) \ +#define PYBIND11_MODULE_PYINIT(name, ...) \ static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ PYBIND11_PLUGIN_IMPL(name) { \ PYBIND11_CHECK_PYTHON_VERSION \ - pre_init; \ - PYBIND11_ENSURE_INTERNALS_READY \ + pybind11::detail::ensure_internals(); \ static ::pybind11::detail::slots_array mod_def_slots = ::pybind11::detail::init_slots( \ &PYBIND11_CONCAT(pybind11_exec_, name), ##__VA_ARGS__); \ static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \ @@ -465,6 +464,7 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \ try { \ + pybind11::detail::ensure_internals(); \ auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm); \ if (!pybind11::detail::get_cached_module(m.attr("__spec__").attr("name"))) { \ PYBIND11_CONCAT(pybind11_init_, name)(m); \ @@ -518,8 +518,7 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across \endrst */ #define PYBIND11_MODULE(name, variable, ...) \ - PYBIND11_MODULE_PYINIT( \ - name, (pybind11::detail::get_num_interpreters_seen() += 1), ##__VA_ARGS__) \ + PYBIND11_MODULE_PYINIT(name, ##__VA_ARGS__) \ PYBIND11_MODULE_EXEC(name, variable) // pop gnu-zero-variadic-macro-arguments diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 95c1f2dfc1..a92f196b1f 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -418,11 +418,12 @@ inline PyThreadState *get_thread_state_unchecked() { #endif } -/// We use this counter to figure out if there are or have been multiple subinterpreters active at -/// any point. This must never decrease while any interpreter may be running in any thread! -inline std::atomic &get_num_interpreters_seen() { - static std::atomic counter(0); - return counter; +/// We use this to figure out if there are or have been multiple subinterpreters active at any +/// point. This must never go from true to false while any interpreter may be running in any +/// thread! +inline std::atomic_bool &has_seen_non_main_interpreter() { + static std::atomic_bool multi(false); + return multi; } template *get_pp() { #ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT - if (get_num_interpreters_seen() > 1) { + if (has_seen_non_main_interpreter()) { // Whenever the interpreter changes on the current thread we need to invalidate the // internals_pp so that it can be pulled from the interpreter's state dict. That is // slow, so we use the current PyThreadState to check if it is necessary. @@ -675,7 +676,7 @@ class internals_pp_manager { /// Drop all the references we're currently holding. void unref() { #ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT - if (get_num_interpreters_seen() > 1) { + if (has_seen_non_main_interpreter()) { last_istate_tls() = nullptr; internals_p_tls() = nullptr; return; @@ -686,7 +687,7 @@ class internals_pp_manager { void destroy() { #ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT - if (get_num_interpreters_seen() > 1) { + if (has_seen_non_main_interpreter()) { auto *tstate = get_thread_state_unchecked(); // this could be called without an active interpreter, just use what was cached if (!tstate || tstate->interp == last_istate_tls()) { @@ -791,6 +792,16 @@ PYBIND11_NOINLINE internals &get_internals() { return *internals_ptr; } +inline void ensure_internals() { + pybind11::detail::get_internals_pp_manager().unref(); +#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT + if (PyInterpreterState_Get() != PyInterpreterState_Main()) { + has_seen_non_main_interpreter() = true; + } +#endif + pybind11::detail::get_internals(); +} + inline internals_pp_manager &get_local_internals_pp_manager() { // Use the address of this static itself as part of the key, so that the value is uniquely tied // to where the module is loaded in memory diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index a820bfbfc3..c05887c33d 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -58,7 +58,7 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_EMBEDDED_MODULE(name, variable, ...) \ - PYBIND11_MODULE_PYINIT(name, {}, ##__VA_ARGS__) \ + PYBIND11_MODULE_PYINIT(name, ##__VA_ARGS__) \ ::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \ PYBIND11_TOSTRING(name), PYBIND11_CONCAT(PyInit_, name)); \ PYBIND11_MODULE_EXEC(name, variable) @@ -202,7 +202,7 @@ inline void initialize_interpreter(bool init_signal_handlers = true, #endif // There is exactly one interpreter alive currently. - detail::get_num_interpreters_seen() = 1; + detail::has_seen_non_main_interpreter() = false; } /** \rst @@ -242,12 +242,12 @@ inline void initialize_interpreter(bool init_signal_handlers = true, \endrst */ inline void finalize_interpreter() { // get rid of any thread-local interpreter cache that currently exists - if (detail::get_num_interpreters_seen() > 1) { + if (detail::has_seen_non_main_interpreter()) { detail::get_internals_pp_manager().unref(); detail::get_local_internals_pp_manager().unref(); - // We know there can be no other interpreter alive now, so we can lower the count - detail::get_num_interpreters_seen() = 1; + // We know there can be no other interpreter alive now + detail::has_seen_non_main_interpreter() = false; } // Re-fetch the internals pointer-to-pointer (but not the internals itself, which might not @@ -265,8 +265,8 @@ inline void finalize_interpreter() { // avoid undefined behaviors when initializing another interpreter detail::get_local_internals_pp_manager().destroy(); - // We know there is no interpreter alive now, so we can reset the count - detail::get_num_interpreters_seen() = 0; + // We know there is no interpreter alive now, so we can reset the multi-flag + detail::has_seen_non_main_interpreter() = false; } /** \rst diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h index 3c8ed84dfc..770ed49998 100644 --- a/include/pybind11/gil_safe_call_once.h +++ b/include/pybind11/gil_safe_call_once.h @@ -232,7 +232,7 @@ class gil_safe_call_once_and_store { // Indicator of fast path for single-interpreter case. bool is_last_storage_valid() const { return is_initialized_by_at_least_one_interpreter_ - && detail::get_num_interpreters_seen() == 1; + && !detail::has_seen_non_main_interpreter(); } // Get the unique key for this storage instance in the interpreter's state dict. diff --git a/include/pybind11/subinterpreter.h b/include/pybind11/subinterpreter.h index aaf5204570..c47787b6ef 100644 --- a/include/pybind11/subinterpreter.h +++ b/include/pybind11/subinterpreter.h @@ -109,7 +109,7 @@ class subinterpreter { // upon success, the new interpreter is activated in this thread result.istate_ = result.creation_tstate_->interp; - detail::get_num_interpreters_seen() += 1; // there are now many interpreters + detail::has_seen_non_main_interpreter() = true; detail::get_internals(); // initialize internals.tstate, amongst other things... // In 3.13+ this state should be deleted right away, and the memory will be reused for diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3d618662f9..15e18705be 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -592,6 +592,14 @@ if(NOT PYBIND11_CUDA_TESTS) foreach(mod IN LISTS PYBIND11_MULTIPLE_INTERPRETERS_TEST_MODULES) set_target_properties("${mod}" PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${pybind11_tests_output_directory}") + # Also set config-specific output directories for multi-configuration generators (MSVC) + if(DEFINED CMAKE_CONFIGURATION_TYPES) + foreach(config ${CMAKE_CONFIGURATION_TYPES}) + string(TOUPPER ${config} config) + set_target_properties("${mod}" PROPERTIES LIBRARY_OUTPUT_DIRECTORY_${config} + "${pybind11_tests_output_directory}") + endforeach() + endif() endforeach() if(PYBIND11_TEST_SMART_HOLDER) diff --git a/tests/mod_per_interpreter_gil_with_singleton.cpp b/tests/mod_per_interpreter_gil_with_singleton.cpp index 874b93c77f..90e2ec8389 100644 --- a/tests/mod_per_interpreter_gil_with_singleton.cpp +++ b/tests/mod_per_interpreter_gil_with_singleton.cpp @@ -9,6 +9,8 @@ namespace py = pybind11; # include #endif +namespace pybind11_tests { +namespace mod_per_interpreter_gil_with_singleton { // A singleton class that holds references to certain Python objects // This singleton is per-interpreter using gil_safe_call_once_and_store class MySingleton { @@ -95,11 +97,15 @@ enum class MyEnum : int { TWO = 2, THREE = 3, }; +} // namespace mod_per_interpreter_gil_with_singleton +} // namespace pybind11_tests PYBIND11_MODULE(mod_per_interpreter_gil_with_singleton, m, py::mod_gil_not_used(), py::multiple_interpreters::per_interpreter_gil()) { + using namespace pybind11_tests::mod_per_interpreter_gil_with_singleton; + #ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT m.attr("defined_PYBIND11_HAS_SUBINTERPRETER_SUPPORT") = true; #else diff --git a/tests/test_multiple_interpreters.py b/tests/test_multiple_interpreters.py index f706e9282b..aadf3d96bb 100644 --- a/tests/test_multiple_interpreters.py +++ b/tests/test_multiple_interpreters.py @@ -90,7 +90,7 @@ def test_independent_subinterpreters(): run_string, create = get_interpreters(modern=True) - m = pytest.importorskip("mod_per_interpreter_gil") + import mod_per_interpreter_gil as m if not m.defined_PYBIND11_HAS_SUBINTERPRETER_SUPPORT: pytest.skip("Does not have subinterpreter support compiled in") @@ -139,7 +139,7 @@ def test_independent_subinterpreters_modern(): sys.path.insert(0, os.path.dirname(pybind11_tests.__file__)) - m = pytest.importorskip("mod_per_interpreter_gil") + import mod_per_interpreter_gil as m if not m.defined_PYBIND11_HAS_SUBINTERPRETER_SUPPORT: pytest.skip("Does not have subinterpreter support compiled in") @@ -187,7 +187,7 @@ def test_dependent_subinterpreters(): run_string, create = get_interpreters(modern=False) - m = pytest.importorskip("mod_shared_interpreter_gil") + import mod_shared_interpreter_gil as m if not m.defined_PYBIND11_HAS_SUBINTERPRETER_SUPPORT: pytest.skip("Does not have subinterpreter support compiled in") @@ -222,15 +222,27 @@ def test(): objects = m.get_objects_in_singleton() expected = [ - type(None), - tuple, - list, - dict, - collections.OrderedDict, - collections.defaultdict, - collections.deque, + type(None), # static type: shared between interpreters + tuple, # static type: shared between interpreters + list, # static type: shared between interpreters + dict, # static type: shared between interpreters + collections.OrderedDict, # static type: shared between interpreters + collections.defaultdict, # heap type: dynamically created per interpreter + collections.deque, # heap type: dynamically created per interpreter ] - assert objects == expected, f"Expected {{expected!r}}, got {{objects!r}}." + # Check that we have the expected objects. Avoid IndexError by checking lengths first. + assert len(objects) == len(expected), ( + f"Expected {{expected!r}} ({{len(expected)}}), got {{objects!r}} ({{len(objects)}})." + ) + # The first ones are static types shared between interpreters. + assert objects[:-2] == expected[:-2], ( + f"Expected static objects {{expected[:-2]!r}}, got {{objects[:-2]!r}}." + ) + # The last two are heap types created per-interpreter. + # The expected objects are dynamically imported from `collections`. + assert objects[-2:] == expected[-2:], ( + f"Expected heap objects {{expected[-2:]!r}}, got {{objects[-2:]!r}}." + ) assert hasattr(m, 'MyClass'), "Module missing MyClass" assert hasattr(m, 'MyGlobalError'), "Module missing MyGlobalError" @@ -240,11 +252,6 @@ def test(): ).lstrip() -@pytest.mark.xfail( - reason="Duplicate C++ type registration under multiple-interpreters, needs investigation.", - # raises=interpreters.ExecutionFailed, # need to import the module - strict=False, -) @pytest.mark.skipif( sys.platform.startswith("emscripten"), reason="Requires loadable modules" ) @@ -278,14 +285,9 @@ def check_script_success_in_subprocess(code: str, *, rerun: int = 8) -> None: f"```\n\n" f"Output:\n" f"{ex.output}" - ) from ex + ) from None -@pytest.mark.xfail( - reason="Duplicate C++ type registration under multiple-interpreters, needs investigation.", - raises=RuntimeError, - strict=False, -) @pytest.mark.skipif( sys.platform.startswith("emscripten"), reason="Requires loadable modules" ) @@ -342,11 +344,6 @@ def test_import_in_subinterpreter_after_main(): ) -@pytest.mark.xfail( - reason="Duplicate C++ type registration under multiple-interpreters, needs investigation.", - raises=RuntimeError, - strict=False, -) @pytest.mark.skipif( sys.platform.startswith("emscripten"), reason="Requires loadable modules" ) @@ -427,11 +424,6 @@ def test_import_in_subinterpreter_before_main(): ) -@pytest.mark.xfail( - reason="Duplicate C++ type registration under multiple-interpreters, needs investigation.", - raises=RuntimeError, - strict=False, -) @pytest.mark.skipif( sys.platform.startswith("emscripten"), reason="Requires loadable modules" ) diff --git a/tests/test_with_catch/test_subinterpreter.cpp b/tests/test_with_catch/test_subinterpreter.cpp index b17b6fbc36..e322e0fe90 100644 --- a/tests/test_with_catch/test_subinterpreter.cpp +++ b/tests/test_with_catch/test_subinterpreter.cpp @@ -31,7 +31,7 @@ void unsafe_reset_internals_for_single_interpreter() { py::detail::get_local_internals_pp_manager().unref(); // we know there are no other interpreters, so we can lower this. SUPER DANGEROUS - py::detail::get_num_interpreters_seen() = 1; + py::detail::has_seen_non_main_interpreter() = false; // now we unref the static global singleton internals py::detail::get_internals_pp_manager().unref();