From 335d8d429738eb741bc7e2ec3a4b5c8c08a9e3fd Mon Sep 17 00:00:00 2001 From: b-pass Date: Wed, 19 Mar 2025 21:14:07 -0400 Subject: [PATCH 1/5] Change PYBIND11_MODULE to use multi-phase init Use slots to specify advanced init options (currently just Py_GIL_NOT_USED). Adds a new function `initialize_multiphase_module_def` to module_, similar to the existing (unchanged) create_extension_module. --- include/pybind11/detail/common.h | 23 +++++++---- include/pybind11/embed.h | 1 + include/pybind11/pybind11.h | 66 ++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 5e225f8c19..b7a1fc9c36 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -372,11 +372,9 @@ #define PYBIND11_CATCH_INIT_EXCEPTIONS \ catch (pybind11::error_already_set & e) { \ pybind11::raise_from(e, PyExc_ImportError, "initialization failed"); \ - return nullptr; \ } \ catch (const std::exception &e) { \ ::pybind11::set_error(PyExc_ImportError, e.what()); \ - return nullptr; \ } /** \rst @@ -404,6 +402,7 @@ return pybind11_init(); \ } \ PYBIND11_CATCH_INIT_EXCEPTIONS \ + return nullptr; \ } \ PyObject *pybind11_init() @@ -447,23 +446,33 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE(name, variable, ...) \ - static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \ - PYBIND11_MAYBE_UNUSED; \ - PYBIND11_MAYBE_UNUSED \ + static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \ + static std::vector PYBIND11_CONCAT(pybind11_module_slots_, name); \ + static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ PYBIND11_PLUGIN_IMPL(name) { \ PYBIND11_CHECK_PYTHON_VERSION \ PYBIND11_ENSURE_INTERNALS_READY \ - auto m = ::pybind11::module_::create_extension_module( \ + auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ + slots.clear(); \ + slots.emplace_back(PyModuleDef_Slot{ \ + Py_mod_exec, reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}); \ + auto m = ::pybind11::module_::initialize_multiphase_module_def( \ PYBIND11_TOSTRING(name), \ nullptr, \ &PYBIND11_CONCAT(pybind11_module_def_, name), \ + slots, \ ##__VA_ARGS__); \ + return m.ptr(); \ + } \ + 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 -1; \ } \ void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ & (variable)) PYBIND11_WARNING_POP diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 0af777033c..1b95c32a7f 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -49,6 +49,7 @@ return m.ptr(); \ } \ PYBIND11_CATCH_INIT_EXCEPTIONS \ + return nullptr; \ } \ PYBIND11_EMBEDDED_MODULE_IMPL(name) \ ::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \ diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 82c758af60..bdf683c540 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1255,6 +1255,22 @@ class mod_gil_not_used { bool flag_; }; +PYBIND11_NAMESPACE_BEGIN(detail) + +inline bool gil_not_used_option() { return false; } +template +bool gil_not_used_option(F &&, O &&...o); +template +inline bool gil_not_used_option(mod_gil_not_used f, O &&...o) { + return f.flag() || gil_not_used_option(o...); +} +template +inline bool gil_not_used_option(F &&, O &&...o) { + return gil_not_used_option(o...); +} + +PYBIND11_NAMESPACE_END(detail) + /// Wrapper for Python extension modules class module_ : public object { public: @@ -1389,6 +1405,56 @@ class module_ : public object { // For Python 2, reinterpret_borrow was correct. return reinterpret_borrow(m); } + + /** \rst + Initialized 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 + additional slots (and the empty terminator slot) from the supplied options. + \endrst */ + template + static object initialize_multiphase_module_def(const char *name, + const char *doc, + module_def *def, + std::vector &slots, + Options &&...options) { + // remove zero end sentinel, if present + while (!slots.empty() && slots.back().slot == 0) { + slots.pop_back(); + } + + bool nogil PYBIND11_MAYBE_UNUSED = detail::gil_not_used_option(options...); + if (nogil) { +#if defined(Py_mod_gil) && defined(Py_GIL_DISABLED) + slots.emplace_back(PyModuleDef_Slot{Py_mod_gil, Py_MOD_GIL_NOT_USED}); +#endif + } + + // must have a zero end sentinel + slots.emplace_back(PyModuleDef_Slot{0, nullptr}); + + // module_def is PyModuleDef + // Placement new (not an allocation). + def = new (def) + PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, + /* m_name */ name, + /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, + /* m_size */ 0, + /* m_methods */ nullptr, + /* m_slots */ slots.data(), + /* m_traverse */ nullptr, + /* m_clear */ nullptr, + /* m_free */ nullptr}; + auto *m = PyModuleDef_Init(def); + if (m == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Internal error in module_::initialize_multiphase_module_def()"); + } + return reinterpret_borrow(m); + } }; PYBIND11_NAMESPACE_BEGIN(detail) From 1dc99003838ac957da40e80588380a3ff8a0538f Mon Sep 17 00:00:00 2001 From: b-pass Date: Thu, 20 Mar 2025 18:47:11 -0400 Subject: [PATCH 2/5] Avoid dynamic allocation and non-trivial destruction ... by using an std::array for the slots. --- include/pybind11/detail/common.h | 8 ++++---- include/pybind11/pybind11.h | 31 ++++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index b7a1fc9c36..bacc5a60db 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -447,16 +447,16 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \ - static std::vector PYBIND11_CONCAT(pybind11_module_slots_, 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_ &); \ PYBIND11_PLUGIN_IMPL(name) { \ PYBIND11_CHECK_PYTHON_VERSION \ PYBIND11_ENSURE_INTERNALS_READY \ auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ - slots.clear(); \ - slots.emplace_back(PyModuleDef_Slot{ \ - Py_mod_exec, reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}); \ + slots[0] \ + = {Py_mod_exec, reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ + slots[1] = {0, nullptr}; \ auto m = ::pybind11::module_::initialize_multiphase_module_def( \ PYBIND11_TOSTRING(name), \ nullptr, \ diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index bdf683c540..8e60eaf2cf 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1406,33 +1406,46 @@ class module_ : public object { return reinterpret_borrow(m); } + /// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for + /// the sentinel (0) end slot. + using slots_array = std::array; + /** \rst Initialized 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 - additional slots (and the empty terminator slot) from the supplied options. + additional slots from the supplied options (and the empty sentinel slot). \endrst */ template static object initialize_multiphase_module_def(const char *name, const char *doc, module_def *def, - std::vector &slots, + slots_array &slots, Options &&...options) { - // remove zero end sentinel, if present - while (!slots.empty() && slots.back().slot == 0) { - slots.pop_back(); + size_t next_slot = 0; + size_t term_slot = slots.size() - 1; + + // find the end of the supplied slots + while (next_slot < term_slot && slots[next_slot].slot != 0) { + ++next_slot; } bool nogil PYBIND11_MAYBE_UNUSED = detail::gil_not_used_option(options...); if (nogil) { #if defined(Py_mod_gil) && defined(Py_GIL_DISABLED) - slots.emplace_back(PyModuleDef_Slot{Py_mod_gil, Py_MOD_GIL_NOT_USED}); + if (next_slot >= term_slot) { + pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); + } + slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}); #endif } - // must have a zero end sentinel - slots.emplace_back(PyModuleDef_Slot{0, nullptr}); + // slots must have a zero end sentinel + if (next_slot > term_slot) { + pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); + } + slots[next_slot++] = {0, nullptr}; // module_def is PyModuleDef // Placement new (not an allocation). @@ -1442,7 +1455,7 @@ class module_ : public object { /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, /* m_size */ 0, /* m_methods */ nullptr, - /* m_slots */ slots.data(), + /* m_slots */ &slots[0], /* m_traverse */ nullptr, /* m_clear */ nullptr, /* m_free */ nullptr}; From 422b08f3e1a2d84bb9b87dfb08b2107c08b663d1 Mon Sep 17 00:00:00 2001 From: b-pass Date: Thu, 20 Mar 2025 19:21:18 -0400 Subject: [PATCH 3/5] Oops, stray cut and paste character --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 8e60eaf2cf..9d1e88060e 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1437,7 +1437,7 @@ class module_ : public object { if (next_slot >= term_slot) { pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); } - slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}); + slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}; #endif } From 554e1b2e725a1f6bbbe0f5d084adbd7c3878566c Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 21 Mar 2025 07:14:43 -0400 Subject: [PATCH 4/5] Remove assignment from placement new, change size fo slots array --- include/pybind11/pybind11.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9d1e88060e..7824ca4342 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1378,7 +1378,7 @@ class module_ : public object { = mod_gil_not_used(false)) { // module_def is PyModuleDef // Placement new (not an allocation). - def = new (def) + new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, /* m_name */ name, /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, @@ -1408,7 +1408,7 @@ class module_ : public object { /// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for /// the sentinel (0) end slot. - using slots_array = std::array; + using slots_array = std::array; /** \rst Initialized a module def for use with multi-phase module initialization. @@ -1449,7 +1449,7 @@ class module_ : public object { // module_def is PyModuleDef // Placement new (not an allocation). - def = new (def) + new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, /* m_name */ name, /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, From 850c587f72f8b454d4532fac2a0c9c4121a1b221 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 21 Mar 2025 11:16:38 +0000 Subject: [PATCH 5/5] style: pre-commit fixes --- include/pybind11/pybind11.h | 38 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 7824ca4342..9499fa7048 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1378,16 +1378,15 @@ class module_ : public object { = mod_gil_not_used(false)) { // module_def is PyModuleDef // Placement new (not an allocation). - new (def) - PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, - /* m_name */ name, - /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, - /* m_size */ -1, - /* m_methods */ nullptr, - /* m_slots */ nullptr, - /* m_traverse */ nullptr, - /* m_clear */ nullptr, - /* m_free */ nullptr}; + new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, + /* m_name */ name, + /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, + /* m_size */ -1, + /* m_methods */ nullptr, + /* m_slots */ nullptr, + /* m_traverse */ nullptr, + /* m_clear */ nullptr, + /* m_free */ nullptr}; auto *m = PyModule_Create(def); if (m == nullptr) { if (PyErr_Occurred()) { @@ -1449,16 +1448,15 @@ class module_ : public object { // module_def is PyModuleDef // Placement new (not an allocation). - new (def) - PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, - /* m_name */ name, - /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, - /* m_size */ 0, - /* m_methods */ nullptr, - /* m_slots */ &slots[0], - /* m_traverse */ nullptr, - /* m_clear */ nullptr, - /* m_free */ nullptr}; + new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, + /* m_name */ name, + /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, + /* m_size */ 0, + /* m_methods */ nullptr, + /* m_slots */ &slots[0], + /* m_traverse */ nullptr, + /* m_clear */ nullptr, + /* m_free */ nullptr}; auto *m = PyModuleDef_Init(def); if (m == nullptr) { if (PyErr_Occurred()) {