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

This commit is contained in:
Petr Viktorin 2025-11-24 13:26:35 +01:00 committed by GitHub
parent 6462322840
commit bf66bce4ee
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 183 additions and 22 deletions

View file

@ -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

View file

@ -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)

View file

@ -65,8 +65,10 @@
_testmultiphase = None
try:
import _interpreters
import concurrent.interpreters
except ModuleNotFoundError:
_interpreters = None
concurrent = None
try:
import _testinternalcapi
except ImportError:
@ -3407,6 +3409,39 @@ def test_from_modexport(self):
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 @@ def test_from_modexport_create_nonmodule(self):
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 Sub(tp):
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

View file

@ -15,7 +15,8 @@
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()

View file

@ -0,0 +1,2 @@
Fix :c:macro:`Py_mod_gil` with API added in :pep:`793`:
:c:func:`!PyModule_FromSlotsAndSpec` and ``PyModExport`` hooks

View file

@ -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},
};

View file

@ -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;

View file

@ -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) {

View file

@ -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) {