gh-141780: Make PyModule_FromSlotsAndSpec enable GIL if needed (GH-141785)

diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
index d64a18bb09e08f85fcf88fbee9645d9b973f3ece..4c8b8c0ed868d6c1958db5b483fc804b2082d555 100644 (file)
--- a/Include/internal/pycore_import.h
+++ b/Include/internal/pycore_import.h
@@ -128,11 +128,18 @@ PyAPI_FUNC(int) _PyImport_ClearExtension(PyObject *name, PyObject *filename);
 // state of the module argument:
 // - If module is NULL or a PyModuleObject with md_gil == Py_MOD_GIL_NOT_USED,
 //   call _PyEval_DisableGIL().
-// - Otherwise, call _PyEval_EnableGILPermanent(). If the GIL was not already
-//   enabled permanently, issue a warning referencing the module's name.
+// - Otherwise, call _PyImport_EnableGILAndWarn
 //
 // This function may raise an exception.
 extern int _PyImport_CheckGILForModule(PyObject *module, PyObject *module_name);
+// Assuming that the GIL is enabled from a call to
+// _PyEval_EnableGILTransient(), call _PyEval_EnableGILPermanent().
+// If the GIL was not already enabled permanently, issue a warning referencing
+// the module's name.
+// Leave a message in verbose mode.
+//
+// This function may raise an exception.
+extern int _PyImport_EnableGILAndWarn(PyThreadState *, PyObject *module_name);
 #endif
 
 #ifdef __cplusplus

diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py
index 7ec23e637d7de6391270f5d842a0f6231e5d5fe0..823e2ab6b2ef0d2efad7ac5a50128b037afcf2d4 100644 (file)
--- a/Lib/test/test_capi/test_module.py
+++ b/Lib/test/test_capi/test_module.py
@@ -3,7 +3,7 @@
 
 import unittest
 import types
-from test.support import import_helper, subTests
+from test.support import import_helper, subTests, requires_gil_enabled
 
 # Skip this test if the _testcapi module isn't available.
 _testcapi = import_helper.import_module('_testcapi')
@@ -25,6 +25,7 @@ def def_and_token(mod):
     )
 
 class TestModFromSlotsAndSpec(unittest.TestCase):
+    @requires_gil_enabled("empty slots re-enable GIL")
     def test_empty(self):
         mod = _testcapi.module_from_slots_empty(FakeSpec())
         self.assertIsInstance(mod, types.ModuleType)

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index fd9750eae804457e3280138539fcf4903e349384..271361ae8164494e669e4cfd7ed1e215453bb31a 100644 (file)
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -65,8 +65,10 @@ except ImportError:
     _testmultiphase = None
 try:
     import _interpreters
+    import concurrent.interpreters
 except ModuleNotFoundError:
     _interpreters = None
+    concurrent = None
 try:
     import _testinternalcapi
 except ImportError:
@@ -3407,6 +3409,39 @@ class ModexportTests(unittest.TestCase):
 
         self.assertEqual(module.__name__, modname)
 
+    @requires_subinterpreters
+    def test_from_modexport_gil_used(self):
+        # Test that a module with Py_MOD_GIL_USED (re-)enables the GIL.
+        # Do this in a new interpreter to avoid interfering with global state.
+        modname = '_test_from_modexport_gil_used'
+        filename = _testmultiphase.__file__
+        interp = concurrent.interpreters.create()
+        self.addCleanup(interp.close)
+        queue = concurrent.interpreters.create_queue()
+        interp.prepare_main(
+            modname=modname,
+            filename=filename,
+            queue=queue,
+        )
+        enabled_before = sys._is_gil_enabled()
+        interp.exec(f"""if True:
+            import sys
+            from test.support.warnings_helper import check_warnings
+            from {__name__} import import_extension_from_file
+            with check_warnings((".*GIL..has been enabled.*", RuntimeWarning),
+                                quiet=True):
+                module = import_extension_from_file(modname, filename,
+                                                    put_in_sys_modules=False)
+            queue.put(module.__name__)
+            queue.put(sys._is_gil_enabled())
+        """)
+
+        self.assertEqual(queue.get(), modname)
+        self.assertEqual(queue.get(), True)
+        self.assertTrue(queue.empty())
+
+        self.assertEqual(enabled_before, sys._is_gil_enabled())
+
     def test_from_modexport_null(self):
         modname = '_test_from_modexport_null'
         filename = _testmultiphase.__file__
@@ -3428,6 +3463,39 @@ class ModexportTests(unittest.TestCase):
                                             put_in_sys_modules=False)
         self.assertIsInstance(module, str)
 
+    @requires_subinterpreters
+    def test_from_modexport_create_nonmodule_gil_used(self):
+        # Test that a module with Py_MOD_GIL_USED (re-)enables the GIL.
+        # Do this in a new interpreter to avoid interfering with global state.
+        modname = '_test_from_modexport_create_nonmodule_gil_used'
+        filename = _testmultiphase.__file__
+        interp = concurrent.interpreters.create()
+        self.addCleanup(interp.close)
+        queue = concurrent.interpreters.create_queue()
+        interp.prepare_main(
+            modname=modname,
+            filename=filename,
+            queue=queue,
+        )
+        enabled_before = sys._is_gil_enabled()
+        interp.exec(f"""if True:
+            import sys
+            from test.support.warnings_helper import check_warnings
+            from {__name__} import import_extension_from_file
+            with check_warnings((".*GIL..has been enabled.*", RuntimeWarning),
+                                quiet=True):
+                module = import_extension_from_file(modname, filename,
+                                                    put_in_sys_modules=False)
+            queue.put(module)
+            queue.put(sys._is_gil_enabled())
+        """)
+
+        self.assertIsInstance(queue.get(), str)
+        self.assertEqual(queue.get(), True)
+        self.assertTrue(queue.empty())
+
+        self.assertEqual(enabled_before, sys._is_gil_enabled())
+
     def test_from_modexport_smoke(self):
         # General positive test for sundry features
         # (PyModule_FromSlotsAndSpec tests exercise these more carefully)
@@ -3456,6 +3524,7 @@ class ModexportTests(unittest.TestCase):
             pass
         self.assertEqual(_testcapi.pytype_getmodulebytoken(Sub, token), module)
 
+    @requires_gil_enabled("empty slots re-enable GIL")
     def test_from_modexport_empty_slots(self):
         # Module to test that:
         # - no slots are mandatory for PyModExport

diff --git a/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py
index edbe78545a2536a1c61cd49b67d5add27a8da061..efbec667317d5f6ed15cb223e9ea56601be1595b 100644 (file)
--- a/Lib/test/test_importlib/util.py
+++ b/Lib/test/test_importlib/util.py
@@ -15,7 +15,8 @@ import sys
 import tempfile
 import types
 
-_testsinglephase = import_helper.import_module("_testsinglephase")
+# gh-116303: Skip test module dependent tests if test modules are unavailable
+import_helper.import_module("_testmultiphase")
 
 
 BUILTINS = types.SimpleNamespace()

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-11-20-13-18-57.gh-issue-141780.xDrVNr.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-11-20-13-18-57.gh-issue-141780.xDrVNr.rst
new file mode 100644 (file)
index 0000000..8700ac1
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-11-20-13-18-57.gh-issue-141780.xDrVNr.rst
@@ -0,0 +1,2 @@
+Fix :c:macro:`Py_mod_gil` with API added in :pep:`793`:
+:c:func:`!PyModule_FromSlotsAndSpec` and ``PyModExport`` hooks

diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c
index 9349445351eacaa3d557a706bb70dfbcd85a0a16..7b5861bc08ed7741e8f23a8ffad1d8144727d598 100644 (file)
--- a/Modules/_testcapi/module.c
+++ b/Modules/_testcapi/module.c
@@ -27,6 +27,8 @@ module_from_slots_name(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_name, "currently ignored..."},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -37,6 +39,8 @@ module_from_slots_doc(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_doc, "the docstring"},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -47,6 +51,8 @@ module_from_slots_size(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_state_size, (void*)123},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec);
@@ -72,6 +78,8 @@ module_from_slots_methods(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_methods, a_methoddef_array},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -90,6 +98,8 @@ module_from_slots_gc(PyObject *self, PyObject *spec)
         {Py_mod_state_traverse, noop_traverse},
         {Py_mod_state_clear, noop_clear},
         {Py_mod_state_free, noop_free},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec);
@@ -118,6 +128,8 @@ module_from_slots_token(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_token, (void*)&test_token},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec);
@@ -144,6 +156,8 @@ module_from_slots_exec(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_exec, simple_exec},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec);
@@ -175,6 +189,8 @@ module_from_slots_create(PyObject *self, PyObject *spec)
 {
     PyModuleDef_Slot slots[] = {
         {Py_mod_create, create_attr_from_spec},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -205,6 +221,8 @@ module_from_slots_repeat_slot(PyObject *self, PyObject *spec)
     PyModuleDef_Slot slots[] = {
         {slot_id, "anything"},
         {slot_id, "anything else"},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -219,6 +237,8 @@ module_from_slots_null_slot(PyObject *self, PyObject *spec)
     }
     PyModuleDef_Slot slots[] = {
         {slot_id, NULL},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return PyModule_FromSlotsAndSpec(slots, spec);
@@ -233,6 +253,8 @@ module_from_def_slot(PyObject *self, PyObject *spec)
     }
     PyModuleDef_Slot slots[] = {
         {slot_id, "anything"},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     PyModuleDef def = {
@@ -280,6 +302,7 @@ module_from_def_multiple_exec(PyObject *self, PyObject *spec)
     static PyModuleDef_Slot slots[] = {
         {Py_mod_exec, simple_exec},
         {Py_mod_exec, another_exec},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
         {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };

diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c
index cd2d7b6559827789e32609ce0fce1916da5aca58..e286eaae820b2b1a075456a5650bf20d140000a2 100644 (file)
--- a/Modules/_testmultiphase.c
+++ b/Modules/_testmultiphase.c
@@ -1024,6 +1024,20 @@ PyModExport__test_from_modexport(void)
 {
     static PyModuleDef_Slot slots[] = {
         {Py_mod_name, "_test_from_modexport"},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
+        {0},
+    };
+    return slots;
+}
+
+PyMODEXPORT_FUNC
+PyModExport__test_from_modexport_gil_used(void)
+{
+    static PyModuleDef_Slot slots[] = {
+        {Py_mod_name, "_test_from_modexport_gil_used"},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_USED},
         {0},
     };
     return slots;
@@ -1073,6 +1087,21 @@ PyModExport__test_from_modexport_create_nonmodule(void)
     static PyModuleDef_Slot slots[] = {
         {Py_mod_name, "_test_from_modexport_create_nonmodule"},
         {Py_mod_create, modexport_create_string},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
+        {0},
+    };
+    return slots;
+}
+
+PyMODEXPORT_FUNC
+PyModExport__test_from_modexport_create_nonmodule_gil_used(void)
+{
+    static PyModuleDef_Slot slots[] = {
+        {Py_mod_name, "_test_from_modexport_create_nonmodule"},
+        {Py_mod_create, modexport_create_string},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_USED},
         {0},
     };
     return slots;
@@ -1165,6 +1194,8 @@ PyModExport__test_from_modexport_smoke(void)
         {Py_mod_methods, methods},
         {Py_mod_state_free, modexport_smoke_free},
         {Py_mod_token, (void*)&modexport_smoke_test_token},
+        {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+        {Py_mod_gil, Py_MOD_GIL_NOT_USED},
         {0},
     };
     return slots;

diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index 6c1c5f5eb89c0c71af6a775e60af658d846d9b14..5a0b16ba57242d6d4e2052023b698eb5a14055c1 100644 (file)
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -2,6 +2,7 @@
 
 #include "Python.h"
 #include "pycore_call.h"          // _PyObject_CallNoArgs()
+#include "pycore_ceval.h"         // _PyEval_EnableGILTransient()
 #include "pycore_dict.h"          // _PyDict_EnablePerThreadRefcounting()
 #include "pycore_fileutils.h"     // _Py_wgetcwd
 #include "pycore_import.h"        // _PyImport_GetNextModuleIndex()
@@ -522,6 +523,22 @@ module_from_def_and_spec(
 #undef COPY_COMMON_SLOT
     }
 
+#ifdef Py_GIL_DISABLED
+    // For modules created directly from slots (not from a def), we enable
+    // the GIL here (pairing `_PyEval_EnableGILTransient` with
+    // an immediate `_PyImport_EnableGILAndWarn`).
+    // For modules created from a def, the caller is responsible for this.
+    if (!original_def && requires_gil) {
+        PyThreadState *tstate = _PyThreadState_GET();
+        if (_PyEval_EnableGILTransient(tstate) < 0) {
+            goto error;
+        }
+        if (_PyImport_EnableGILAndWarn(tstate, nameobj) < 0) {
+            goto error;
+        }
+    }
+#endif
+
     /* By default, multi-phase init modules are expected
        to work under multiple interpreters. */
     if (!has_multiple_interpreters_slot) {

diff --git a/Python/import.c b/Python/import.c
index de183de44da843555789df8df8506062e15b52f3..e91c95b40d94bfd814eef04d4cfd4bf1fdb17b05 100644 (file)
--- a/Python/import.c
+++ b/Python/import.c
@@ -1561,25 +1561,11 @@ _PyImport_CheckGILForModule(PyObject* module, PyObject *module_name)
     if (!PyModule_Check(module) ||
         ((PyModuleObject *)module)->md_requires_gil)
     {
-        if (_PyEval_EnableGILPermanent(tstate)) {
-            int warn_result = PyErr_WarnFormat(
-                PyExc_RuntimeWarning,
-                1,
-                "The global interpreter lock (GIL) has been enabled to load "
-                "module '%U', which has not declared that it can run safely "
-                "without the GIL. To override this behavior and keep the GIL "
-                "disabled (at your own risk), run with PYTHON_GIL=0 or -Xgil=0.",
-                module_name
-            );
-            if (warn_result < 0) {
-                return warn_result;
-            }
+        if (PyModule_Check(module)) {
+            assert(((PyModuleObject *)module)->md_token_is_def);
         }
-
-        const PyConfig *config = _PyInterpreterState_GetConfig(tstate->interp);
-        if (config->enable_gil == _PyConfig_GIL_DEFAULT && config->verbose) {
-            PySys_FormatStderr("# loading module '%U', which requires the GIL\n",
-                               module_name);
+        if (_PyImport_EnableGILAndWarn(tstate, module_name) < 0) {
+            return -1;
         }
     }
     else {
@@ -1588,6 +1574,28 @@ _PyImport_CheckGILForModule(PyObject* module, PyObject *module_name)
 
     return 0;
 }
+
+int
+_PyImport_EnableGILAndWarn(PyThreadState *tstate, PyObject *module_name)
+{
+    if (_PyEval_EnableGILPermanent(tstate)) {
+        return PyErr_WarnFormat(
+            PyExc_RuntimeWarning,
+            1,
+            "The global interpreter lock (GIL) has been enabled to load "
+            "module '%U', which has not declared that it can run safely "
+            "without the GIL. To override this behavior and keep the GIL "
+            "disabled (at your own risk), run with PYTHON_GIL=0 or -Xgil=0.",
+            module_name
+        );
+    }
+    const PyConfig *config = _PyInterpreterState_GetConfig(tstate->interp);
+    if (config->enable_gil == _PyConfig_GIL_DEFAULT && config->verbose) {
+        PySys_FormatStderr("# loading module '%U', which requires the GIL\n",
+                            module_name);
+    }
+    return 0;
+}
 #endif
 
 static PyThreadState *
@@ -4796,6 +4804,8 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
     _PyImport_GetModuleExportHooks(&info, fp, &p0, &ex0);
     if (ex0) {
         mod = import_run_modexport(tstate, ex0, &info, spec);
+        // Modules created from slots handle GIL enablement (Py_mod_gil slot)
+        // when they're created.
         goto cleanup;
     }
     if (p0 == NULL) {