From 1c420e138fd828895b6bd3c44ef99156e8796095 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 5 May 2023 14:04:55 -0600 Subject: [PATCH] gh-104108: Add the Py_mod_multiple_interpreters Module Def Slot (gh-104148) I'll be adding a value to indicate support for per-interpreter GIL in gh-99114. --- Include/moduleobject.h | 7 +- Lib/test/test_import/__init__.py | 82 ++++++++++++++----- ...-05-03-17-46-47.gh-issue-104108.GOxAYt.rst | 6 ++ Modules/_testmultiphase.c | 19 +++++ Objects/moduleobject.c | 30 +++++++ 5 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-05-03-17-46-47.gh-issue-104108.GOxAYt.rst diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 555564ec73b..7ac6f6e8a4a 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -78,11 +78,16 @@ struct PyModuleDef_Slot { #define Py_mod_create 1 #define Py_mod_exec 2 +#define Py_mod_multiple_interpreters 3 #ifndef Py_LIMITED_API -#define _Py_mod_LAST_SLOT 2 +#define _Py_mod_LAST_SLOT 3 #endif +/* for Py_mod_multiple_interpreters: */ +#define Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED ((void *)0) +#define Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED ((void *)1) + #endif /* New in 3.5 */ struct PyModuleDef { diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 41dfdaabe24..9211639b016 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1652,26 +1652,44 @@ class SubinterpImportTests(unittest.TestCase): os.set_blocking(r, False) return (r, w) - def import_script(self, name, fd, check_override=None): + def import_script(self, name, fd, filename=None, check_override=None): override_text = '' if check_override is not None: override_text = f''' - import _imp - _imp._override_multi_interp_extensions_check({check_override}) - ''' - return textwrap.dedent(f''' - import os, sys - {override_text} - try: - import {name} - except ImportError as exc: - text = 'ImportError: ' + str(exc) - else: - text = 'okay' - os.write({fd}, text.encode('utf-8')) - ''') + import _imp + _imp._override_multi_interp_extensions_check({check_override}) + ''' + if filename: + return textwrap.dedent(f''' + from importlib.util import spec_from_loader, module_from_spec + from importlib.machinery import ExtensionFileLoader + import os, sys + {override_text} + loader = ExtensionFileLoader({name!r}, {filename!r}) + spec = spec_from_loader({name!r}, loader) + try: + module = module_from_spec(spec) + loader.exec_module(module) + except ImportError as exc: + text = 'ImportError: ' + str(exc) + else: + text = 'okay' + os.write({fd}, text.encode('utf-8')) + ''') + else: + return textwrap.dedent(f''' + import os, sys + {override_text} + try: + import {name} + except ImportError as exc: + text = 'ImportError: ' + str(exc) + else: + text = 'okay' + os.write({fd}, text.encode('utf-8')) + ''') - def run_here(self, name, *, + def run_here(self, name, filename=None, *, check_singlephase_setting=False, check_singlephase_override=None, isolated=False, @@ -1700,26 +1718,30 @@ class SubinterpImportTests(unittest.TestCase): ) r, w = self.pipe() - script = self.import_script(name, w, check_singlephase_override) + script = self.import_script(name, w, filename, + check_singlephase_override) ret = run_in_subinterp_with_config(script, **kwargs) self.assertEqual(ret, 0) return os.read(r, 100) - def check_compatible_here(self, name, *, strict=False, isolated=False): + def check_compatible_here(self, name, filename=None, *, + strict=False, + isolated=False, + ): # Verify that the named module may be imported in a subinterpreter. # (See run_here() for more info.) - out = self.run_here(name, + out = self.run_here(name, filename, check_singlephase_setting=strict, isolated=isolated, ) self.assertEqual(out, b'okay') - def check_incompatible_here(self, name, *, isolated=False): + def check_incompatible_here(self, name, filename=None, *, isolated=False): # Differences from check_compatible_here(): # * verify that import fails # * "strict" is always True - out = self.run_here(name, + out = self.run_here(name, filename, check_singlephase_setting=True, isolated=isolated, ) @@ -1820,6 +1842,24 @@ class SubinterpImportTests(unittest.TestCase): with self.subTest(f'{module}: strict, fresh'): self.check_compatible_fresh(module, strict=True) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_multi_init_extension_non_isolated_compat(self): + modname = '_test_non_isolated' + filename = _testmultiphase.__file__ + loader = ExtensionFileLoader(modname, filename) + spec = importlib.util.spec_from_loader(modname, loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + sys.modules[modname] = module + + require_extension(module) + with self.subTest(f'{modname}: isolated'): + self.check_incompatible_here(modname, filename, isolated=True) + with self.subTest(f'{modname}: not isolated'): + self.check_incompatible_here(modname, filename, isolated=False) + with self.subTest(f'{modname}: not strict'): + self.check_compatible_here(modname, filename, strict=False) + def test_python_compat(self): module = 'threading' require_pure_python(module) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-03-17-46-47.gh-issue-104108.GOxAYt.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-03-17-46-47.gh-issue-104108.GOxAYt.rst new file mode 100644 index 00000000000..dad84363649 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-03-17-46-47.gh-issue-104108.GOxAYt.rst @@ -0,0 +1,6 @@ +Multi-phase init extension modules may now indicate whether or not they +actually support multiple interpreters. By default such modules are +expected to support use in multiple interpreters. In the uncommon case that +one does not, it may use the new ``Py_mod_multiple_interpreters`` module def +slot. A value of ``0`` means the module does not support them. ``1`` means +it does. The default is ``1``. diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index cf8990a2df0..bc7d8b64a94 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -884,3 +884,22 @@ PyInit__test_module_state_shared(void) } return module; } + + +/* multiple interpreters supports */ + +static PyModuleDef_Slot non_isolated_slots[] = { + {Py_mod_exec, execfunc}, + {Py_mod_multiple_interpreters, Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED}, + {0, NULL}, +}; + +static PyModuleDef non_isolated_def = TEST_MODULE_DEF("_test_non_isolated", + non_isolated_slots, + testexport_methods); + +PyMODINIT_FUNC +PyInit__test_non_isolated(void) +{ + return PyModuleDef_Init(&non_isolated_def); +} diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index ffcd90ed122..63f4226dd9e 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -245,6 +245,8 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio PyObject *(*create)(PyObject *, PyModuleDef*) = NULL; PyObject *nameobj; PyObject *m = NULL; + int has_multiple_interpreters_slot = 0; + void *multiple_interpreters = (void *)0; int has_execution_slots = 0; const char *name; int ret; @@ -287,6 +289,17 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio case Py_mod_exec: has_execution_slots = 1; break; + case Py_mod_multiple_interpreters: + if (has_multiple_interpreters_slot) { + PyErr_Format( + PyExc_SystemError, + "module %s has more than one 'multiple interpreters' slots", + name); + goto error; + } + multiple_interpreters = cur_slot->value; + has_multiple_interpreters_slot = 1; + break; default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); PyErr_Format( @@ -297,6 +310,20 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio } } + /* By default, multi-phase init modules are expected + to work under multiple interpreters. */ + if (!has_multiple_interpreters_slot) { + multiple_interpreters = Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED; + } + if (multiple_interpreters == Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!_Py_IsMainInterpreter(interp) + && _PyImport_CheckSubinterpIncompatibleExtensionAllowed(name) < 0) + { + goto error; + } + } + if (create) { m = create(spec, def); if (m == NULL) { @@ -421,6 +448,9 @@ PyModule_ExecDef(PyObject *module, PyModuleDef *def) return -1; } break; + case Py_mod_multiple_interpreters: + /* handled in PyModule_FromDefAndSpec2 */ + break; default: PyErr_Format( PyExc_SystemError,