Skip to content

feat: change PYBIND11_EMBEDDED_MODULE to multiphase init #5665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 5 additions & 0 deletions docs/advanced/embedding.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
====================
Expand Down
4 changes: 4 additions & 0 deletions docs/advanced/misc.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
==================================================================

Expand All @@ -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
==================================================================

Expand Down
35 changes: 27 additions & 8 deletions include/pybind11/embed.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 30 additions & 11 deletions tests/test_embed/test_interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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

Expand All @@ -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());

Expand All @@ -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());

Expand All @@ -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"));

Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand All @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double-locking here (GIL in line 637, C++ mutex in line 640) has the potential to deadlock, because the code in line 641 calls into the Python C API, which may release (and reacquire) the GIL (see docs/advanced/deadlock.md). — I can believe that this will be extremely rare, but we should leave a comment here to explain that we're aware of the risk, and that we're knowingly accepting it.

See also: https://chatgpt.com/share/682a07ec-dd70-8008-92ea-e722ff92a055

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this can deadlock. These specific concrete types and calls will not release/reacquire the GIL (PyDict_Get/SetItem with string keys and long object values).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These specific concrete types

I believe that, but examples and tests are often copy-pasted, and then something else is done here.

The comment I'm suggesting is to make people aware of the pitfall. This is not a safe pattern in general.

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

Expand Down
Loading