From 25f6ff5d3e92305659db62e7f7545f823f0dbd05 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 15:00:54 -0400 Subject: [PATCH] gh-117649: Raise ImportError for unsupported modules in free-threaded build (#117651) The free-threaded build does not currently support the combination of single-phase init modules and non-isolated subinterpreters. Ensure that `check_multi_interp_extensions` is always `True` for subinterpreters in the free-threaded build so that importing these modules raises an `ImportError`. --- Include/cpython/pylifecycle.h | 11 ++++++- Lib/test/support/__init__.py | 6 ++++ Lib/test/test_capi/test_misc.py | 40 +++++++++++++++++++++----- Lib/test/test_import/__init__.py | 38 +++++++++++++++--------- Lib/test/test_importlib/test_util.py | 4 +++ Lib/test/test_interpreters/test_api.py | 13 ++++----- Lib/test/test_threading.py | 3 +- Python/import.c | 7 +++++ Python/pylifecycle.c | 13 ++++++++- 9 files changed, 103 insertions(+), 32 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index d425a233f71000..e46dfe59ec4630 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -63,6 +63,15 @@ typedef struct { .gil = PyInterpreterConfig_OWN_GIL, \ } +// gh-117649: The free-threaded build does not currently support single-phase +// init extensions in subinterpreters. For now, we ensure that +// `check_multi_interp_extensions` is always `1`, even in the legacy config. +#ifdef Py_GIL_DISABLED +# define _PyInterpreterConfig_LEGACY_CHECK_MULTI_INTERP_EXTENSIONS 1 +#else +# define _PyInterpreterConfig_LEGACY_CHECK_MULTI_INTERP_EXTENSIONS 0 +#endif + #define _PyInterpreterConfig_LEGACY_INIT \ { \ .use_main_obmalloc = 1, \ @@ -70,7 +79,7 @@ typedef struct { .allow_exec = 1, \ .allow_threads = 1, \ .allow_daemon_threads = 1, \ - .check_multi_interp_extensions = 0, \ + .check_multi_interp_extensions = _PyInterpreterConfig_LEGACY_CHECK_MULTI_INTERP_EXTENSIONS, \ .gil = PyInterpreterConfig_SHARED_GIL, \ } diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 4bf2d7b5142da9..be3f93ab2e5fd1 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -842,6 +842,12 @@ def requires_gil_enabled(msg="needs the GIL enabled"): """Decorator for skipping tests on the free-threaded build.""" return unittest.skipIf(Py_GIL_DISABLED, msg) +def expected_failure_if_gil_disabled(): + """Expect test failure if the GIL is disabled.""" + if Py_GIL_DISABLED: + return unittest.expectedFailure + return lambda test_case: test_case + if Py_GIL_DISABLED: _header = 'PHBBInP' else: diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 35d6a209122a99..8cdecaf3626401 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -26,6 +26,8 @@ from test.support import threading_helper from test.support import warnings_helper from test.support import requires_limited_api +from test.support import requires_gil_enabled, expected_failure_if_gil_disabled +from test.support import Py_GIL_DISABLED from test.support.script_helper import assert_python_failure, assert_python_ok, run_python_until_end try: import _posixsubprocess @@ -2023,15 +2025,30 @@ def test_configured_settings(self): kwlist[-2] = 'check_multi_interp_extensions' kwlist[-1] = 'own_gil' - # expected to work - for config, expected in { + expected_to_work = { (True, True, True, True, True, True, True): (ALL_FLAGS, True), (True, False, False, False, False, False, False): (OBMALLOC, False), (False, False, False, True, False, True, False): (THREADS | EXTENSIONS, False), - }.items(): + } + + expected_to_fail = { + (False, False, False, False, False, False, False), + } + + # gh-117649: The free-threaded build does not currently allow + # setting check_multi_interp_extensions to False. + if Py_GIL_DISABLED: + for config in list(expected_to_work.keys()): + kwargs = dict(zip(kwlist, config)) + if not kwargs['check_multi_interp_extensions']: + del expected_to_work[config] + expected_to_fail.add(config) + + # expected to work + for config, expected in expected_to_work.items(): kwargs = dict(zip(kwlist, config)) exp_flags, exp_gil = expected expected = { @@ -2055,9 +2072,7 @@ def test_configured_settings(self): self.assertEqual(settings, expected) # expected to fail - for config in [ - (False, False, False, False, False, False, False), - ]: + for config in expected_to_fail: kwargs = dict(zip(kwlist, config)) with self.subTest(config): script = textwrap.dedent(f''' @@ -2070,6 +2085,9 @@ def test_configured_settings(self): @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") + # gh-117649: The free-threaded build does not currently allow overriding + # the check_multi_interp_extensions setting. + @expected_failure_if_gil_disabled() def test_overridden_setting_extensions_subinterp_check(self): """ PyInterpreterConfig.check_multi_interp_extensions can be overridden @@ -2165,6 +2183,9 @@ def test_mutate_exception(self): self.assertFalse(hasattr(binascii.Error, "foobar")) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + # gh-117649: The free-threaded build does not currently support sharing + # extension module state between interpreters. + @expected_failure_if_gil_disabled() def test_module_state_shared_in_global(self): """ bpo-44050: Extension module state should be shared between interpreters @@ -2223,7 +2244,7 @@ class InterpreterConfigTests(unittest.TestCase): allow_exec=True, allow_threads=True, allow_daemon_threads=True, - check_multi_interp_extensions=False, + check_multi_interp_extensions=bool(Py_GIL_DISABLED), gil='shared', ), 'empty': types.SimpleNamespace( @@ -2386,6 +2407,8 @@ def test_interp_init(self): check_multi_interp_extensions=False ), ] + if Py_GIL_DISABLED: + invalid.append(dict(check_multi_interp_extensions=False)) def match(config, override_cases): ns = vars(config) for overrides in override_cases: @@ -2427,6 +2450,8 @@ def new_interp(config): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' + if Py_GIL_DISABLED: + expected.check_multi_interp_extensions = False interpid, *_ = _interpreters.get_main() config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) @@ -2448,6 +2473,7 @@ def new_interp(config): 'empty', use_main_obmalloc=True, gil='shared', + check_multi_interp_extensions=bool(Py_GIL_DISABLED), ) with new_interp(orig) as interpid: config = _interpreters.get_config(interpid) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 6678548a0ffaca..4726619b08edc4 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -30,7 +30,8 @@ from test.support import os_helper from test.support import ( STDLIB_DIR, swap_attr, swap_item, cpython_only, is_apple_mobile, is_emscripten, - is_wasi, run_in_subinterp, run_in_subinterp_with_config, Py_TRACE_REFS) + is_wasi, run_in_subinterp, run_in_subinterp_with_config, Py_TRACE_REFS, + requires_gil_enabled, Py_GIL_DISABLED) from test.support.import_helper import ( forget, make_legacy_pyc, unlink, unload, ready_to_import, DirsOnSysPath, CleanImport, import_module) @@ -158,6 +159,9 @@ def meth(self, _meth=meth): finally: restore__testsinglephase() meth = cpython_only(meth) + # gh-117649: free-threaded build does not currently support single-phase + # init modules in subinterpreters. + meth = requires_gil_enabled(meth) return unittest.skipIf(_testsinglephase is None, 'test requires _testsinglephase module')(meth) @@ -1876,8 +1880,9 @@ def test_builtin_compat(self): # since they still don't implement multi-phase init. module = '_imp' require_builtin(module) - with self.subTest(f'{module}: not strict'): - self.check_compatible_here(module, strict=False) + if not Py_GIL_DISABLED: + with self.subTest(f'{module}: not strict'): + self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): self.check_compatible_here(module, strict=True) @@ -1888,8 +1893,9 @@ def test_frozen_compat(self): require_frozen(module, skip=True) if __import__(module).__spec__.origin != 'frozen': raise unittest.SkipTest(f'{module} is unexpectedly not frozen') - with self.subTest(f'{module}: not strict'): - self.check_compatible_here(module, strict=False) + if not Py_GIL_DISABLED: + with self.subTest(f'{module}: not strict'): + self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): self.check_compatible_here(module, strict=True) @@ -1908,8 +1914,9 @@ def test_single_init_extension_compat(self): def test_multi_init_extension_compat(self): module = '_testmultiphase' require_extension(module) - with self.subTest(f'{module}: not strict'): - self.check_compatible_here(module, strict=False) + if not Py_GIL_DISABLED: + with self.subTest(f'{module}: not strict'): + self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): self.check_compatible_here(module, strict=True) with self.subTest(f'{module}: strict, fresh'): @@ -1930,8 +1937,9 @@ def test_multi_init_extension_non_isolated_compat(self): 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) + if not Py_GIL_DISABLED: + with self.subTest(f'{modname}: not strict'): + self.check_compatible_here(modname, filename, strict=False) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_multi_init_extension_per_interpreter_gil_compat(self): @@ -1949,16 +1957,18 @@ def test_multi_init_extension_per_interpreter_gil_compat(self): with self.subTest(f'{modname}: not isolated, strict'): self.check_compatible_here(modname, filename, strict=True, isolated=False) - with self.subTest(f'{modname}: not isolated, not strict'): - self.check_compatible_here(modname, filename, - strict=False, isolated=False) + if not Py_GIL_DISABLED: + with self.subTest(f'{modname}: not isolated, not strict'): + self.check_compatible_here(modname, filename, + strict=False, isolated=False) @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_python_compat(self): module = 'threading' require_pure_python(module) - with self.subTest(f'{module}: not strict'): - self.check_compatible_here(module, strict=False) + if not Py_GIL_DISABLED: + with self.subTest(f'{module}: not strict'): + self.check_compatible_here(module, strict=False) with self.subTest(f'{module}: strict, not fresh'): self.check_compatible_here(module, strict=True) with self.subTest(f'{module}: strict, fresh'): diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index 115cb7a56c98f7..f0583c5fd0196f 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -682,6 +682,9 @@ def ensure_destroyed(): raise ImportError(excsnap.msg) @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") + # gh-117649: single-phase init modules are not currently supported in + # subinterpreters in the free-threaded build + @support.expected_failure_if_gil_disabled() def test_single_phase_init_module(self): script = textwrap.dedent(''' from importlib.util import _incompatible_extension_module_restrictions @@ -706,6 +709,7 @@ def test_single_phase_init_module(self): self.run_with_own_gil(script) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + @support.requires_gil_enabled("gh-117649: not supported in free-threaded build") def test_incomplete_multi_phase_init_module(self): # Apple extensions must be distributed as frameworks. This requires # a specialist loader. diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index abf66a7cde796c..769bb7bfdb5a32 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -10,6 +10,7 @@ from test.support import import_helper # Raise SkipTest if subinterpreters not supported. _interpreters = import_helper.import_module('_xxsubinterpreters') +from test.support import Py_GIL_DISABLED from test.support import interpreters from test.support.interpreters import ( InterpreterError, InterpreterNotFoundError, ExecutionFailed, @@ -1162,7 +1163,7 @@ def test_new_config(self): allow_exec=True, allow_threads=True, allow_daemon_threads=True, - check_multi_interp_extensions=False, + check_multi_interp_extensions=bool(Py_GIL_DISABLED), gil='shared', ), 'empty': types.SimpleNamespace( @@ -1361,6 +1362,7 @@ def test_create(self): with self.subTest('custom'): orig = _interpreters.new_config('empty') orig.use_main_obmalloc = True + orig.check_multi_interp_extensions = bool(Py_GIL_DISABLED) orig.gil = 'shared' interpid = _interpreters.create(orig) config = _interpreters.get_config(interpid) @@ -1410,13 +1412,8 @@ def test_get_config(self): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' - interpid, *_ = _interpreters.get_main() - config = _interpreters.get_config(interpid) - self.assert_ns_equal(config, expected) - - with self.subTest('main'): - expected = _interpreters.new_config('legacy') - expected.gil = 'own' + if Py_GIL_DISABLED: + expected.check_multi_interp_extensions = False interpid, *_ = _interpreters.get_main() config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index a7701fa285aee2..a712ed10f022d6 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1527,6 +1527,7 @@ def func(): {before_start} t.start() """) + check_multi_interp_extensions = bool(support.Py_GIL_DISABLED) script = textwrap.dedent(f""" import test.support test.support.run_in_subinterp_with_config( @@ -1536,7 +1537,7 @@ def func(): allow_exec=True, allow_threads={allowed}, allow_daemon_threads={daemon_allowed}, - check_multi_interp_extensions=False, + check_multi_interp_extensions={check_multi_interp_extensions}, own_gil=False, ) """) diff --git a/Python/import.c b/Python/import.c index 6544a84d895d4a..b040c7d5c0f7f5 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3696,9 +3696,16 @@ _imp__override_multi_interp_extensions_check_impl(PyObject *module, "cannot be used in the main interpreter"); return NULL; } +#ifdef Py_GIL_DISABLED + PyErr_SetString(PyExc_RuntimeError, + "_imp._override_multi_interp_extensions_check() " + "cannot be used in the free-threaded build"); + return NULL; +#else int oldvalue = OVERRIDE_MULTI_INTERP_EXTENSIONS_CHECK(interp); OVERRIDE_MULTI_INTERP_EXTENSIONS_CHECK(interp) = override; return PyLong_FromLong(oldvalue); +#endif } #ifdef HAVE_DYNAMIC_LOADING diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 4e83b1671a5029..efb25878312d85 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -559,6 +559,15 @@ init_interp_settings(PyInterpreterState *interp, return _PyStatus_ERR("per-interpreter obmalloc does not support " "single-phase init extension modules"); } +#ifdef Py_GIL_DISABLED + if (!_Py_IsMainInterpreter(interp) && + !config->check_multi_interp_extensions) + { + return _PyStatus_ERR("The free-threaded build does not support " + "single-phase init extension modules in " + "subinterpreters"); + } +#endif if (config->allow_fork) { interp->feature_flags |= Py_RTFLAGS_FORK; @@ -647,8 +656,10 @@ pycore_create_interpreter(_PyRuntimeState *runtime, } PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; - // The main interpreter always has its own GIL. + // The main interpreter always has its own GIL and supports single-phase + // init extensions. config.gil = PyInterpreterConfig_OWN_GIL; + config.check_multi_interp_extensions = 0; status = init_interp_settings(interp, &config); if (_PyStatus_EXCEPTION(status)) { return status;