From 79c3decdae1e1313042c01e8a1254f0d91b8ad0d Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 16 May 2025 20:04:43 -0400 Subject: [PATCH 01/10] Move embedded modules to multiphase init So that they too can support multi-interpreter and nogil tags --- include/pybind11/embed.h | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index a456e80a6e..d9b3d2dca5 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -38,22 +38,40 @@ }); } \endrst */ -#define PYBIND11_EMBEDDED_MODULE(name, variable) \ +PYBIND11_WARNING_PUSH +PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") +#define PYBIND11_EMBEDDED_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \ + static ::pybind11::module_::slots_array PYBIND11_CONCAT(pybind11_module_slots_, name); \ + static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \ - auto m = ::pybind11::module_::create_extension_module( \ - PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \ + static auto result = []() { \ + auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ + slots[0] = {Py_mod_exec, \ + reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ + slots[1] = {0, nullptr}; \ + return ::pybind11::module_::initialize_multiphase_module_def( \ + PYBIND11_TOSTRING(name), \ + nullptr, \ + &PYBIND11_CONCAT(pybind11_module_def_, name), \ + slots, \ + ##__VA_ARGS__); \ + }(); \ + return result.ptr(); \ + } \ + PYBIND11_EMBEDDED_MODULE_IMPL(name) \ + ::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \ + PYBIND11_TOSTRING(name), PYBIND11_CONCAT(pybind11_init_impl_, name)); \ + int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \ try { \ + auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm); \ PYBIND11_CONCAT(pybind11_init_, name)(m); \ - return m.ptr(); \ + return 0; \ } \ PYBIND11_CATCH_INIT_EXCEPTIONS \ - return nullptr; \ + return -1; \ } \ - PYBIND11_EMBEDDED_MODULE_IMPL(name) \ - ::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \ - PYBIND11_TOSTRING(name), PYBIND11_CONCAT(pybind11_init_impl_, name)); \ void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ \ & variable) // NOLINT(bugprone-macro-parentheses) From 733d248db7118df1cffaedfe6bd1bb4bdc6c1263 Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 16 May 2025 20:05:24 -0400 Subject: [PATCH 02/10] Update the multiple interpreter test for embedded module changes --- tests/test_embed/test_interpreter.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index cf9f86b521..40dbbd3065 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -55,7 +55,7 @@ class test_override_cache_helper_trampoline : public test_override_cache_helper int func() override { PYBIND11_OVERRIDE(int, test_override_cache_helper, func); } }; -PYBIND11_EMBEDDED_MODULE(widget_module, m) { +PYBIND11_EMBEDDED_MODULE(widget_module, m, py::multiple_interpreters::per_interpreter_gil()) { py::class_(m, "Widget") .def(py::init()) .def_property_readonly("the_message", &Widget::the_message); @@ -512,10 +512,11 @@ TEST_CASE("Per-Subinterpreter GIL") { // we have switched to the new interpreter and released the main gil - // widget_module did not provide the mod_per_interpreter_gil tag, so it cannot be imported + // trampoline_module did not provide the per_interpreter_gil tag, so it cannot be + // imported bool caught = false; try { - py::module_::import("widget_module"); + py::module_::import("trampoline_module"); } catch (pybind11::error_already_set &pe) { T_REQUIRE(pe.matches(PyExc_ImportError)); std::string msg(pe.what()); @@ -525,6 +526,15 @@ TEST_CASE("Per-Subinterpreter GIL") { } T_REQUIRE(caught); + // widget_module did provide the per_interpreter_gil tag, so it this does not throw + try { + py::module_::import("widget_module"); + caught = false; + } catch (pybind11::error_already_set &pe) { + caught = true; + } + T_REQUIRE(!caught); + T_REQUIRE(!py::hasattr(py::module_::import("external_module"), "multi_interp")); py::module_::import("external_module").attr("multi_interp") = std::to_string(num); From 8e5e26d9b685d08cab01262c4af0900e2f1dba87 Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 16 May 2025 20:07:10 -0400 Subject: [PATCH 03/10] Add a note to embedded module docs about the new tags --- docs/advanced/embedding.rst | 5 +++++ docs/advanced/misc.rst | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/docs/advanced/embedding.rst b/docs/advanced/embedding.rst index dc09161ec8..dec767aac9 100644 --- a/docs/advanced/embedding.rst +++ b/docs/advanced/embedding.rst @@ -212,6 +212,11 @@ naturally: assert(locals["message"].cast() == "1 + 2 = 3"); } +``PYBIND11_EMBEDDED_MODULE`` also accepts +:func:`py::mod_gil_not_used()`, +:func:`py::multiple_interpreters::per_interpreter_gil()`, and +:func:`py::multiple_interpreters::shared_gil()` tags just like ``PYBIND11_MODULE``. +See :ref:`misc_subinterp` and :ref:`misc_free_threading` for more information. Interpreter lifetime ==================== diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index a4b941f9aa..7d2a279585 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -155,6 +155,8 @@ following checklist. within pybind11 that will throw exceptions on certain GIL handling errors (reference counting operations). +.. _misc_free_threading: + Free-threading support ================================================================== @@ -178,6 +180,8 @@ your code is thread safe. Modules must still be built against the Python free-t enable free-threading, even if they specify this tag. Adding this tag does not break compatibility with non-free-threaded Python. +.. _misc_subinterp: + Sub-interpreter support ================================================================== From d7946ab07106892b85e764513177cd461abd880e Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 16 May 2025 20:17:25 -0400 Subject: [PATCH 04/10] Oops, missed a warning pop --- include/pybind11/embed.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index d9b3d2dca5..99a765ba16 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -74,6 +74,7 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") } \ void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ \ & variable) // NOLINT(bugprone-macro-parentheses) +PYBIND11_WARNING_POP PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) From 64f7a0a8f70dd1e4031799b243fe8489bda4575c Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 16 May 2025 20:35:37 -0400 Subject: [PATCH 05/10] Remove unused variable --- tests/test_embed/test_interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 40dbbd3065..8d2a5a505a 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -530,7 +530,7 @@ TEST_CASE("Per-Subinterpreter GIL") { try { py::module_::import("widget_module"); caught = false; - } catch (pybind11::error_already_set &pe) { + } catch (pybind11::error_already_set &) { caught = true; } T_REQUIRE(!caught); From 8160c8ac3169900ce1530dcd1dd8dd8f9be174c9 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 16 May 2025 22:00:55 -0400 Subject: [PATCH 06/10] Update ci.yml --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9f7ae9617..ef554d4c2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,9 @@ jobs: python: - '3.8' - '3.13' + - '3.13t' - '3.14' + - '3.14t' - 'pypy-3.10' - 'pypy-3.11' - 'graalpy-24.2' From 7cbc3fc8beadeb184a2f15a839699dc66ac5393d Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 08:33:36 -0400 Subject: [PATCH 07/10] Fix this embedded GIL test for free-threading --- tests/test_embed/test_interpreter.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 8d2a5a505a..deb31ae50b 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -637,7 +637,13 @@ TEST_CASE("Threads") { for (auto i = 0; i < num_threads; ++i) { threads.emplace_back([&]() { py::gil_scoped_acquire gil{}; +#ifdef Py_GIL_DISABLED + Py_BEGIN_CRITICAL_SECTION(locals); locals["count"] = locals["count"].cast() + 1; + Py_END_CRITICAL_SECTION(); +#else + locals["count"] = locals["count"].cast() + 1; +#endif }); } From ca5ea1e85ab389853227b5ebeced3d6dced0e958 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 08:42:36 -0400 Subject: [PATCH 08/10] Oops, need to use ptr() here --- tests/test_embed/test_interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index deb31ae50b..baa8c890ed 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -638,7 +638,7 @@ TEST_CASE("Threads") { threads.emplace_back([&]() { py::gil_scoped_acquire gil{}; #ifdef Py_GIL_DISABLED - Py_BEGIN_CRITICAL_SECTION(locals); + Py_BEGIN_CRITICAL_SECTION(locals.ptr()); locals["count"] = locals["count"].cast() + 1; Py_END_CRITICAL_SECTION(); #else From f0e6ac257b4161672c4d9146a0d20dc1ef664150 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 21:14:28 -0400 Subject: [PATCH 09/10] This test created a subinterpreter when PYBIND11_SUBINTERPRETER_SUPPORT was off So the fix is really this test should not be run in these older versions at all. The hang was a GIL issue between the subinterpreters during pybind11::exception::what(). --- include/pybind11/pybind11.h | 2 +- tests/test_embed/test_interpreter.cpp | 25 ++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 7108343f6f..3b09b765ee 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1484,7 +1484,7 @@ class module_ : public object { using slots_array = std::array; /** \rst - Initialized a module def for use with multi-phase module initialization. + Initialize a module def for use with multi-phase module initialization. ``def`` should point to a statically allocated module_def. ``slots`` must already contain a Py_mod_exec or Py_mod_create slot and will be filled with diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index baa8c890ed..ebb6215f3d 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -336,6 +336,7 @@ TEST_CASE("Restart the interpreter") { REQUIRE(py_widget.attr("the_message").cast() == "Hello after restart"); } +#if defined(PYBIND11_SUBINTERPRETER_SUPPORT) TEST_CASE("Subinterpreter") { py::module_::import("external_module"); // in the main interpreter @@ -347,6 +348,10 @@ TEST_CASE("Subinterpreter") { REQUIRE(m.attr("add")(1, 2).cast() == 3); } + + auto main_int + = py::module_::import("external_module").attr("internals_at")().cast(); + REQUIRE(has_state_dict_internals_obj()); REQUIRE(has_pybind11_internals_static()); @@ -359,7 +364,6 @@ TEST_CASE("Subinterpreter") { // Subinterpreters get their own copy of builtins. REQUIRE_FALSE(has_state_dict_internals_obj()); -#if defined(PYBIND11_SUBINTERPRETER_SUPPORT) && PY_VERSION_HEX >= 0x030C0000 // internals hasn't been populated yet, but will be different for the subinterpreter REQUIRE_FALSE(has_pybind11_internals_static()); @@ -369,14 +373,12 @@ TEST_CASE("Subinterpreter") { py::detail::get_internals(); REQUIRE(has_pybind11_internals_static()); REQUIRE(get_details_as_uintptr() == ext_int); -#else - // This static is still defined - REQUIRE(has_pybind11_internals_static()); -#endif + REQUIRE(main_int != ext_int); // Modules tags should be gone. REQUIRE_FALSE(py::hasattr(py::module_::import("__main__"), "tag")); { + REQUIRE_NOTHROW(py::module_::import("widget_module")); auto m = py::module_::import("widget_module"); REQUIRE_FALSE(py::hasattr(m, "extension_module_tag")); @@ -397,7 +399,6 @@ TEST_CASE("Subinterpreter") { REQUIRE(has_state_dict_internals_obj()); } -#if defined(PYBIND11_SUBINTERPRETER_SUPPORT) TEST_CASE("Multiple Subinterpreters") { // Make sure the module is in the main interpreter and save its pointer auto *main_ext = py::module_::import("external_module").ptr(); @@ -527,13 +528,7 @@ TEST_CASE("Per-Subinterpreter GIL") { T_REQUIRE(caught); // widget_module did provide the per_interpreter_gil tag, so it this does not throw - try { - py::module_::import("widget_module"); - caught = false; - } catch (pybind11::error_already_set &) { - caught = true; - } - T_REQUIRE(!caught); + py::module_::import("widget_module"); T_REQUIRE(!py::hasattr(py::module_::import("external_module"), "multi_interp")); py::module_::import("external_module").attr("multi_interp") = std::to_string(num); @@ -557,8 +552,8 @@ TEST_CASE("Per-Subinterpreter GIL") { Py_EndInterpreter(sub); - PyThreadState_Swap( - main_tstate); // switch back so the scoped_acquire can release the GIL properly + // switch back so the scoped_acquire can release the GIL properly + PyThreadState_Swap(main_tstate); }; std::thread t1(thread_main, 1); From 8c99bfb2d49bd456944ec9a7ee914aa3d56846cd Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sun, 18 May 2025 01:06:51 -0400 Subject: [PATCH 10/10] fix: standard mutex for 3.13t Signed-off-by: Henry Schreiner --- tests/test_embed/test_interpreter.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index ebb6215f3d..6e4be7378a 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -627,15 +627,23 @@ TEST_CASE("Threads") { { py::gil_scoped_release gil_release{}; +#if defined(Py_GIL_DISABLED) && PY_VERSION_HEX < 0x030E0000 + std::mutex mutex; +#endif auto threads = std::vector(); for (auto i = 0; i < num_threads; ++i) { threads.emplace_back([&]() { py::gil_scoped_acquire gil{}; #ifdef Py_GIL_DISABLED +# if PY_VERSION_HEX < 0x030E0000 + std::lock_guard lock(mutex); + locals["count"] = locals["count"].cast() + 1; +# else Py_BEGIN_CRITICAL_SECTION(locals.ptr()); locals["count"] = locals["count"].cast() + 1; Py_END_CRITICAL_SECTION(); +# endif #else locals["count"] = locals["count"].cast() + 1; #endif