Skip to content

Commit

Permalink
pythongh-117649: Raise ImportError for unsupported modules in free-th…
Browse files Browse the repository at this point in the history
…readed build (python#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`.
  • Loading branch information
colesbury authored Apr 11, 2024
1 parent 39d381f commit 25f6ff5
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 32 deletions.
11 changes: 10 additions & 1 deletion Include/cpython/pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,23 @@ 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, \
.allow_fork = 1, \
.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, \
}

Expand Down
6 changes: 6 additions & 0 deletions Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
40 changes: 33 additions & 7 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand All @@ -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'''
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
38 changes: 24 additions & 14 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

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

Expand All @@ -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'):
Expand All @@ -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):
Expand All @@ -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'):
Expand Down
4 changes: 4 additions & 0 deletions Lib/test/test_importlib/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
13 changes: 5 additions & 8 deletions Lib/test/test_interpreters/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
)
""")
Expand Down
7 changes: 7 additions & 0 deletions Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 25f6ff5

Please sign in to comment.