feat: change PYBIND11_EMBEDDED_MODULE to multiphase init (#5665)

* Move embedded modules to multiphase init

So that they too can support multi-interpreter and nogil tags

* Update the multiple interpreter test for embedded module changes

* Add a note to embedded module docs about the new tags

* Oops, missed a warning pop

* Remove unused variable

* Update ci.yml

* Fix this embedded GIL test for free-threading

* Oops, need to use ptr() here

* 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().

* fix: standard mutex for 3.13t

Signed-off-by: Henry Schreiner <[email protected]>

---------

Signed-off-by: Henry Schreiner <[email protected]>
Co-authored-by: Henry Schreiner <[email protected]>

Author: b-pass

.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'

docs/advanced/embedding.rst

@@ -212,6 +212,11 @@ naturally:
         assert(locals["message"].cast<std::string>() == "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
 ====================

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
 ==================================================================
 

include/pybind11/embed.h

@@ -38,24 +38,43 @@
             });
         }
  \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<void *>(&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)
+PYBIND11_WARNING_POP
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_BEGIN(detail)

include/pybind11/pybind11.h

@@ -1484,7 +1484,7 @@ class module_ : public object {
     using slots_array = std::array<PyModuleDef_Slot, 4>;
 
     /** \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

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_<Widget, PyWidget>(m, "Widget")
         .def(py::init<std::string>())
         .def_property_readonly("the_message", &Widget::the_message);
@@ -336,6 +336,7 @@ TEST_CASE("Restart the interpreter") {
     REQUIRE(py_widget.attr("the_message").cast<std::string>() == "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<int>() == 3);
     }
+
+    auto main_int
+        = py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>();
+
     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();
@@ -512,10 +513,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 +527,9 @@ TEST_CASE("Per-Subinterpreter GIL") {
         }
         T_REQUIRE(caught);
 
+        // widget_module did provide the per_interpreter_gil tag, so it this does not throw
+        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);
 
@@ -547,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);
@@ -622,12 +627,26 @@ 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<std::thread>();
         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<std::mutex> lock(mutex);
+                locals["count"] = locals["count"].cast<int>() + 1;
+#    else
+                Py_BEGIN_CRITICAL_SECTION(locals.ptr());
                 locals["count"] = locals["count"].cast<int>() + 1;
+                Py_END_CRITICAL_SECTION();
+#    endif
+#else
+                locals["count"] = locals["count"].cast<int>() + 1;
+#endif
             });
         }