diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 3f83113bd7..5337e5eeb9 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -510,11 +510,19 @@ struct local_internals { }; /// Works like `get_internals`, but for things which are locally registered. +/// There is one variable storing the local internals for each module, but every embedded module +/// shares the same locals. inline local_internals &get_local_internals() { static local_internals locals; return locals; } +/// Clears the locally registered types and exception translators. +inline void clear_local_internals() { + detail::get_local_internals().registered_types_cpp.clear(); + detail::get_local_internals().registered_exception_translators.clear(); +} + /// Constructs a std::string with the given arguments, stores it in `internals`, and returns its /// `c_str()`. Such strings objects have a long storage duration -- the internal strings are only /// cleared when the program exits or after interpreter shutdown (when embedding), and so are diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 98cc3e4664..73371b5c0f 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -42,8 +42,11 @@ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \ 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)); \ + auto m = ::pybind11::detail::create_extension_module_impl( \ + PYBIND11_TOSTRING(name), \ + nullptr, \ + &PYBIND11_CONCAT(pybind11_module_def_, name), \ + true); \ try { \ PYBIND11_CONCAT(pybind11_init_, name)(m); \ return m.ptr(); \ @@ -200,8 +203,7 @@ inline void finalize_interpreter() { } // Local internals contains data managed by the current interpreter, so we must clear them to // avoid undefined behaviors when initializing another interpreter - detail::get_local_internals().registered_types_cpp.clear(); - detail::get_local_internals().registered_exception_translators.clear(); + detail::clear_local_internals(); Py_Finalize(); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 422d534c12..b0241597fc 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1127,6 +1127,14 @@ class cpp_function : public function { } }; +class module_; + +PYBIND11_NAMESPACE_BEGIN(detail) + +module_ create_extension_module_impl(const char *, const char *, PyModuleDef *, bool); + +PYBIND11_NAMESPACE_END(detail) + /// Wrapper for Python extension modules class module_ : public object { public: @@ -1221,32 +1229,45 @@ class module_ : public object { ``def`` should point to a statically allocated module_def. \endrst */ static module_ create_extension_module(const char *name, const char *doc, module_def *def) { - // 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 */ -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()) { - throw error_already_set(); - } - pybind11_fail("Internal error in module_::create_extension_module()"); - } - // TODO: Should be reinterpret_steal for Python 3, but Python also steals it again when - // returned from PyInit_... - // For Python 2, reinterpret_borrow was correct. - return reinterpret_borrow(m); + return detail::create_extension_module_impl(name, doc, def, false); } }; +PYBIND11_NAMESPACE_BEGIN(detail) + +// Initializes a 'PyModuleDef' instance and create an extension module out of it. +inline module_ create_extension_module_impl(const char *name, + const char *doc, + PyModuleDef *def, + bool is_embedded) { + // 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 */ -1, + /* m_methods */ nullptr, + /* m_slots */ nullptr, + /* m_traverse */ nullptr, + /* m_clear */ nullptr, + // Non-embedded modules must clean their own locals since they can only be accessed by them + /* m_free */ is_embedded ? nullptr : +[](void *) { detail::clear_local_internals(); }}; + auto *m = PyModule_Create(def); + if (m == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Internal error in module_::create_extension_module_impl()"); + } + // TODO: Should be reinterpret_steal for Python 3, but Python also steals it again when + // returned from PyInit_... + // For Python 2, reinterpret_borrow was correct. + return reinterpret_borrow(m); +} + +PYBIND11_NAMESPACE_END(detail) + // When inside a namespace (or anywhere as long as it's not the first item on a line), // C++20 allows "module" to be used. This is provided for backward compatibility, and for // simplicity, if someone wants to use py::module for example, that is perfectly safe. diff --git a/tests/test_embed/external_module.cpp b/tests/test_embed/external_module.cpp index 5c482fe061..78b20c6a07 100644 --- a/tests/test_embed/external_module.cpp +++ b/tests/test_embed/external_module.cpp @@ -13,8 +13,15 @@ PYBIND11_MODULE(external_module, m) { int v; }; + class B {}; + py::class_(m, "A").def(py::init()).def_readwrite("value", &A::v); + // PR #3798 : Local internals must be cleared on finalization so they can be registered again. + // This class is not explicitly used in the test, but it is still registered / unregistered + // when the interpreter stops and restarts. + py::class_(m, "B", py::module_local()); + m.def("internals_at", []() { return reinterpret_cast(&py::detail::get_internals()); }); } diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 1c45457a05..232c984ac7 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -184,6 +184,7 @@ TEST_CASE("Restart the interpreter") { == py::module_::import("external_module").attr("internals_at")().cast()); // Restart the interpreter. + // PR #3798 : Local internals must be cleared on finalization so they can be registered again. py::finalize_interpreter(); REQUIRE(Py_IsInitialized() == 0);