Allow multiphase modules to be re-imported (#5782)

* Failing unit test

* Potential fix for the issue of re-importing a multi-phase module

- When a module is successfully imported and exec'd, save its handle in a dict in the interpreter state
- Use a special Py_mod_create slot to look in the cache and return the cached handle if it is in the cache
- Don't re-run the user exec function if the module is in the interpreter's cache (implying it was already successfully imported)

* Oops, need to inline these.

* Clang-Tidy fixes

* Oops, debug code

* Add xfail for this GraalPy bug

* Remove static from these function defs, it was a cut-and-paste error in the first place.

* Fix test comment

* Proper error handling

* Oops

* Split up this line, but still just ignore failure .. if the module doesn't have the right properties to check the cache then just allow exec to run.

* Clean up - already looked up the name, just use that.

* Some compilers complain if the pointer isn't taken here, weird.

* Allow attribute errors to be thrown here, will be converted to import errors by the exception handler.

* Remove bogus incref, unconditionally expect a __spec__.name on the module

* Add PR to test comment

* style: pre-commit fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: b-pass

include/pybind11/detail/common.h

@@ -455,7 +455,10 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across
     int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) {                                    \
         try {                                                                                     \
             auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm);                       \
-            PYBIND11_CONCAT(pybind11_init_, name)(m);                                             \
+            if (!pybind11::detail::get_cached_module(m.attr("__spec__").attr("name"))) {          \
+                PYBIND11_CONCAT(pybind11_init_, name)(m);                                         \
+                pybind11::detail::cache_completed_module(m);                                      \
+            }                                                                                     \
             return 0;                                                                             \
         }                                                                                         \
         PYBIND11_CATCH_INIT_EXCEPTIONS                                                            \

include/pybind11/pybind11.h

@@ -1329,18 +1329,71 @@ inline void *multi_interp_slot(F &&, O &&...o) {
 }
 #endif
 
+/*
+Return a borrowed reference to the named module if it has been successfully initialized within this
+interpreter before. nullptr if it has not been successfully initialized.
+*/
+inline PyObject *get_cached_module(pybind11::str const &nameobj) {
+    dict state = detail::get_python_state_dict();
+    if (!state.contains("__pybind11_module_cache")) {
+        return nullptr;
+    }
+    dict cache = state["__pybind11_module_cache"];
+    if (!cache.contains(nameobj)) {
+        return nullptr;
+    }
+    return cache[nameobj].ptr();
+}
+
+/*
+Add successfully initialized a module object to the internal cache.
+
+The module must have a __spec__ attribute with a name attribute.
+*/
+inline void cache_completed_module(pybind11::object const &mod) {
+    dict state = detail::get_python_state_dict();
+    if (!state.contains("__pybind11_module_cache")) {
+        state["__pybind11_module_cache"] = dict();
+    }
+    state["__pybind11_module_cache"][mod.attr("__spec__").attr("name")] = mod;
+}
+
+/*
+A Py_mod_create slot function which will return the previously created module from the cache if one
+exists, and otherwise will create a new module object.
+*/
+inline PyObject *cached_create_module(PyObject *spec, PyModuleDef *) {
+    (void) &cache_completed_module; // silence unused-function warnings, it is used in a macro
+
+    auto nameobj = getattr(reinterpret_borrow<object>(spec), "name", none());
+    if (nameobj.is_none()) {
+        set_error(PyExc_ImportError, "module spec is missing a name");
+        return nullptr;
+    }
+
+    auto *mod = get_cached_module(nameobj);
+    if (mod) {
+        Py_INCREF(mod);
+    } else {
+        mod = PyModule_NewObject(nameobj.ptr());
+    }
+    return mod;
+}
+
 /// 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<PyModuleDef_Slot, 4>;
+using slots_array = std::array<PyModuleDef_Slot, 5>;
 
 /// Initialize an array of slots based on the supplied exec slot and options.
 template <typename... Options>
-static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept {
+inline slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept {
     /* NOTE: slots_array MUST be large enough to hold all possible options.  If you add an option
     here, you MUST also increase the size of slots_array in the type alias above! */
     slots_array slots;
     size_t next_slot = 0;
 
+    slots[next_slot++] = {Py_mod_create, reinterpret_cast<void *>(&cached_create_module)};
+
     if (exec_fn != nullptr) {
         slots[next_slot++] = {Py_mod_exec, reinterpret_cast<void *>(exec_fn)};
     }

tests/test_modules.py

@@ -71,6 +71,23 @@ def test_importing():
     assert OD is OrderedDict
 
 
+def test_reimport():
+    import sys
+
+    import pybind11_tests as x
+
+    del sys.modules["pybind11_tests"]
+
+    # exercise pybind11::detail::get_cached_module()
+    import pybind11_tests as y
+
+    assert x is y
+
+
+@pytest.mark.xfail(
+    "env.GRAALPY",
+    reason="TODO should be fixed on GraalPy side (failure was introduced by pr #5782)",
+)
 def test_pydoc():
     """Pydoc needs to be able to provide help() for everything inside a pybind11 module"""
     import pydoc