From 9ff3d32748ea650cc4d7d6eb7fb83a5ab559b8f4 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 8 Apr 2024 18:05:43 +0000 Subject: [PATCH 1/8] gh-117649: Raise ImportError for unsupported modules in free-threaded build The free-threaded build does not currently support the combination of single-phase init modules and legacy, non-isolated subinterpreters. Note that with isolated interpreters, single-phase init modules already trigger `ImportError`. --- Lib/test/test_capi/test_misc.py | 3 +++ Lib/test/test_import/__init__.py | 6 +++++- Lib/test/test_importlib/test_util.py | 19 +++++++++++++++---- Python/import.c | 13 +++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 2f2bf03749f834..bcbe3d592c4e97 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -26,6 +26,7 @@ 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 from test.support.script_helper import assert_python_failure, assert_python_ok, run_python_until_end try: import _posixsubprocess @@ -2070,6 +2071,7 @@ def test_configured_settings(self): @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") + @requires_gil_enabled("gh-117649: test does not work in free-threaded build") def test_overridden_setting_extensions_subinterp_check(self): """ PyInterpreterConfig.check_multi_interp_extensions can be overridden @@ -2165,6 +2167,7 @@ def test_mutate_exception(self): self.assertFalse(hasattr(binascii.Error, "foobar")) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + @requires_gil_enabled("gh-117649: test does not work in free-threaded build") def test_module_state_shared_in_global(self): """ bpo-44050: Extension module state should be shared between interpreters diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 6678548a0ffaca..439a38b0a0ed11 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) 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) diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index 115cb7a56c98f7..bdcf677c977d6a 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -11,6 +11,7 @@ import re import string import sys +import sysconfig from test import support import textwrap import types @@ -688,10 +689,20 @@ def test_single_phase_init_module(self): with _incompatible_extension_module_restrictions(disable_check=True): import _testsinglephase ''') - with self.subTest('check disabled, shared GIL'): - self.run_with_shared_gil(script) - with self.subTest('check disabled, per-interpreter GIL'): - self.run_with_own_gil(script) + if not sysconfig.get_config_var('Py_GIL_DISABLED'): + with self.subTest('check disabled, shared GIL'): + self.run_with_shared_gil(script) + with self.subTest('check disabled, per-interpreter GIL'): + self.run_with_own_gil(script) + else: + # gh-117649: Py_GIL_DISABLED builds do not support legacy + # single-phase init extensions within subinterpreters. + with self.subTest('check disabled, shared GIL'): + with self.assertRaises(ImportError): + self.run_with_shared_gil(script) + with self.subTest('check disabled, per-interpreter GIL'): + with self.assertRaises(ImportError): + self.run_with_own_gil(script) script = textwrap.dedent(f''' from importlib.util import _incompatible_extension_module_restrictions diff --git a/Python/import.c b/Python/import.c index 6544a84d895d4a..590fd4024ce07c 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1244,6 +1244,19 @@ import_find_extension(PyThreadState *tstate, PyObject *name, return NULL; } + /* gh-117649: The free-threaded build does not currently support + single-phase init modules in subinterpreters. */ +#ifdef Py_GIL_DISABLED + if (def->m_size == -1 && !_Py_IsMainInterpreter(tstate->interp)) { + return PyErr_Format( + PyExc_ImportError, + "module %s does not support the combination of free-threading " + "and subinterpreters", + name_buf); + return NULL; + } +#endif + PyObject *mod, *mdict; PyObject *modules = MODULES(tstate->interp); From ff57551ce3495e05e0145a3c56aa5c8c2fae6cb5 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 8 Apr 2024 22:19:49 +0000 Subject: [PATCH 2/8] Remove unreachable statement --- Python/import.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 590fd4024ce07c..16dbbfa2b2b642 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1253,7 +1253,6 @@ import_find_extension(PyThreadState *tstate, PyObject *name, "module %s does not support the combination of free-threading " "and subinterpreters", name_buf); - return NULL; } #endif From 73ee12dc548c7e6b87a029b9170c86e961d647ab Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 9 Apr 2024 20:08:41 +0000 Subject: [PATCH 3/8] Update PR to use check_multi_interp_extensions. As suggested by Petr in the corresponding issue. --- Include/cpython/pylifecycle.h | 8 +++++- Lib/test/test_capi/test_misc.py | 10 +++++++- Lib/test/test_import/__init__.py | 34 +++++++++++++++----------- Lib/test/test_importlib/test_util.py | 3 +-- Lib/test/test_interpreters/test_api.py | 6 +++-- Lib/test/test_threading.py | 3 ++- Python/import.c | 19 ++++++-------- Python/pylifecycle.c | 13 +++++++++- 8 files changed, 62 insertions(+), 34 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index d425a233f71000..e18171544b7269 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -63,6 +63,12 @@ typedef struct { .gil = PyInterpreterConfig_OWN_GIL, \ } +#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 +76,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/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index bcbe3d592c4e97..9ab9ddde8521e0 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -27,6 +27,7 @@ from test.support import warnings_helper from test.support import requires_limited_api from test.support import requires_gil_enabled +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 @@ -2034,6 +2035,9 @@ def test_configured_settings(self): (THREADS | EXTENSIONS, False), }.items(): kwargs = dict(zip(kwlist, config)) + if Py_GIL_DISABLED and not kwargs['check_multi_interp_extensions']: + # Skip unsupported configuration + continue exp_flags, exp_gil = expected expected = { 'feature_flags': exp_flags, @@ -2226,7 +2230,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( @@ -2389,6 +2393,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: @@ -2430,6 +2436,7 @@ def new_interp(config): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' + expected.check_multi_interp_extensions = False interpid = _interpreters.get_main() config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) @@ -2451,6 +2458,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 439a38b0a0ed11..4726619b08edc4 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -31,7 +31,7 @@ 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, - requires_gil_enabled) + 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) @@ -1880,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) @@ -1892,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) @@ -1912,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'): @@ -1934,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): @@ -1953,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 bdcf677c977d6a..9ebcc689125fce 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -11,7 +11,6 @@ import re import string import sys -import sysconfig from test import support import textwrap import types @@ -689,7 +688,7 @@ def test_single_phase_init_module(self): with _incompatible_extension_module_restrictions(disable_check=True): import _testsinglephase ''') - if not sysconfig.get_config_var('Py_GIL_DISABLED'): + if not support.Py_GIL_DISABLED: with self.subTest('check disabled, shared GIL'): self.run_with_shared_gil(script) with self.subTest('check disabled, per-interpreter GIL'): diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index a326b39fd234c7..7dbe975c1ac62c 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -9,7 +9,7 @@ from test.support import import_helper # Raise SkipTest if subinterpreters not supported. _interpreters = import_helper.import_module('_xxsubinterpreters') -from test.support import interpreters +from test.support import interpreters, Py_GIL_DISABLED from test.support.interpreters import InterpreterNotFoundError from .utils import _captured_script, _run_output, _running, TestBase @@ -983,7 +983,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( @@ -1071,6 +1071,7 @@ def test_get_config(self): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' + expected.check_multi_interp_extensions = False interpid = _interpreters.get_main() config = _interpreters.get_config(interpid) self.assert_ns_equal(config, expected) @@ -1121,6 +1122,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) 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 16dbbfa2b2b642..b040c7d5c0f7f5 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1244,18 +1244,6 @@ import_find_extension(PyThreadState *tstate, PyObject *name, return NULL; } - /* gh-117649: The free-threaded build does not currently support - single-phase init modules in subinterpreters. */ -#ifdef Py_GIL_DISABLED - if (def->m_size == -1 && !_Py_IsMainInterpreter(tstate->interp)) { - return PyErr_Format( - PyExc_ImportError, - "module %s does not support the combination of free-threading " - "and subinterpreters", - name_buf); - } -#endif - PyObject *mod, *mdict; PyObject *modules = MODULES(tstate->interp); @@ -3708,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 1d315b80d88ce0..872256a3138216 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -558,6 +558,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; @@ -644,8 +653,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; From c1c1b3b740af51f216588a6a64dfddaa396ca7c5 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 9 Apr 2024 21:42:34 +0000 Subject: [PATCH 4/8] Skip two more tests that use `_imp._override_multi_interp_extensions_check` --- Lib/test/test_importlib/test_util.py | 20 ++++++-------------- Python/import.c | 2 ++ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index 9ebcc689125fce..268175fa143edb 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -682,26 +682,17 @@ def ensure_destroyed(): raise ImportError(excsnap.msg) @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") + @support.requires_gil_enabled("gh-117649: not supported in free-threaded build") def test_single_phase_init_module(self): script = textwrap.dedent(''' from importlib.util import _incompatible_extension_module_restrictions with _incompatible_extension_module_restrictions(disable_check=True): import _testsinglephase ''') - if not support.Py_GIL_DISABLED: - with self.subTest('check disabled, shared GIL'): - self.run_with_shared_gil(script) - with self.subTest('check disabled, per-interpreter GIL'): - self.run_with_own_gil(script) - else: - # gh-117649: Py_GIL_DISABLED builds do not support legacy - # single-phase init extensions within subinterpreters. - with self.subTest('check disabled, shared GIL'): - with self.assertRaises(ImportError): - self.run_with_shared_gil(script) - with self.subTest('check disabled, per-interpreter GIL'): - with self.assertRaises(ImportError): - self.run_with_own_gil(script) + with self.subTest('check disabled, shared GIL'): + self.run_with_shared_gil(script) + with self.subTest('check disabled, per-interpreter GIL'): + self.run_with_own_gil(script) script = textwrap.dedent(f''' from importlib.util import _incompatible_extension_module_restrictions @@ -716,6 +707,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/Python/import.c b/Python/import.c index b040c7d5c0f7f5..9f31cb6c6f8486 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3689,6 +3689,7 @@ _imp__override_multi_interp_extensions_check_impl(PyObject *module, int override) /*[clinic end generated code: output=3ff043af52bbf280 input=e086a2ea181f92ae]*/ { + printf("_imp__override_multi_interp_extensions_check_impl: %d\n", override); PyInterpreterState *interp = _PyInterpreterState_GET(); if (_Py_IsMainInterpreter(interp)) { PyErr_SetString(PyExc_RuntimeError, @@ -3697,6 +3698,7 @@ _imp__override_multi_interp_extensions_check_impl(PyObject *module, return NULL; } #ifdef Py_GIL_DISABLED + printf("Setting exception\n"); PyErr_SetString(PyExc_RuntimeError, "_imp._override_multi_interp_extensions_check() " "cannot be used in the free-threaded build"); From c2b2eac58d677b88f047397088cb6c65885c80ee Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 13:34:27 -0400 Subject: [PATCH 5/8] Remove debug print Co-authored-by: Petr Viktorin --- Python/import.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 9f31cb6c6f8486..f1090be3dc6a90 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3689,7 +3689,6 @@ _imp__override_multi_interp_extensions_check_impl(PyObject *module, int override) /*[clinic end generated code: output=3ff043af52bbf280 input=e086a2ea181f92ae]*/ { - printf("_imp__override_multi_interp_extensions_check_impl: %d\n", override); PyInterpreterState *interp = _PyInterpreterState_GET(); if (_Py_IsMainInterpreter(interp)) { PyErr_SetString(PyExc_RuntimeError, From 079ff14cb7e47c48375512844d6ec73e54883e06 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 17:34:50 +0000 Subject: [PATCH 6/8] Make conditional --- Lib/test/test_capi/test_misc.py | 3 ++- Lib/test/test_interpreters/test_api.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 9ab9ddde8521e0..79ed2f000bc440 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2436,7 +2436,8 @@ def new_interp(config): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' - expected.check_multi_interp_extensions = False + 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_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index 7dbe975c1ac62c..83901b8b3112f4 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -1071,7 +1071,8 @@ def test_get_config(self): with self.subTest('main'): expected = _interpreters.new_config('legacy') expected.gil = 'own' - expected.check_multi_interp_extensions = False + 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) From 4afbf9575a74f2af713bcc2f7cad62f9e5d1c873 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 11:55:34 -0400 Subject: [PATCH 7/8] Remove debug printf Co-authored-by: Petr Viktorin --- Python/import.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index f1090be3dc6a90..b040c7d5c0f7f5 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3697,7 +3697,6 @@ _imp__override_multi_interp_extensions_check_impl(PyObject *module, return NULL; } #ifdef Py_GIL_DISABLED - printf("Setting exception\n"); PyErr_SetString(PyExc_RuntimeError, "_imp._override_multi_interp_extensions_check() " "cannot be used in the free-threaded build"); From 788790ef3d2ff16f6dde5914bdbcdbac373b21fd Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 17:49:21 +0000 Subject: [PATCH 8/8] Use expected failures instead of skipping tests --- Include/cpython/pylifecycle.h | 3 +++ Lib/test/support/__init__.py | 6 +++++ Lib/test/test_capi/test_misc.py | 38 +++++++++++++++++++--------- Lib/test/test_importlib/test_util.py | 4 ++- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index e18171544b7269..e46dfe59ec4630 100644 --- a/Include/cpython/pylifecycle.h +++ b/Include/cpython/pylifecycle.h @@ -63,6 +63,9 @@ 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 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 9fe7ca81a8d67f..8cdecaf3626401 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -26,7 +26,7 @@ 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 +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: @@ -2025,19 +2025,31 @@ 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)) - if Py_GIL_DISABLED and not kwargs['check_multi_interp_extensions']: - # Skip unsupported configuration - continue exp_flags, exp_gil = expected expected = { 'feature_flags': exp_flags, @@ -2060,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''' @@ -2075,7 +2085,9 @@ def test_configured_settings(self): @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") - @requires_gil_enabled("gh-117649: test does not work in free-threaded build") + # 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 @@ -2171,7 +2183,9 @@ def test_mutate_exception(self): self.assertFalse(hasattr(binascii.Error, "foobar")) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") - @requires_gil_enabled("gh-117649: test does not work in free-threaded build") + # 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 diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index 268175fa143edb..f0583c5fd0196f 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -682,7 +682,9 @@ def ensure_destroyed(): raise ImportError(excsnap.msg) @unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module") - @support.requires_gil_enabled("gh-117649: not supported in free-threaded build") + # 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