Skip to content

Commit

Permalink
gh-98627: Add an Optional Check for Extension Module Subinterpreter C…
Browse files Browse the repository at this point in the history
…ompatibility (gh-99040)

Enforcing (optionally) the restriction set by PEP 489 makes sense. Furthermore, this sets the stage for a potential restriction related to a per-interpreter GIL.

This change includes the following:

* add tests for extension module subinterpreter compatibility
* add _PyInterpreterConfig.check_multi_interp_extensions
* add Py_RTFLAGS_MULTI_INTERP_EXTENSIONS
* add _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
* fail iff the module does not implement multi-phase init and the current interpreter is configured to check

#98627
  • Loading branch information
ericsnowcurrently authored Feb 16, 2023
1 parent 3dea4ba commit 89ac665
Show file tree
Hide file tree
Showing 15 changed files with 557 additions and 19 deletions.
3 changes: 3 additions & 0 deletions Include/cpython/initconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ typedef struct {
int allow_exec;
int allow_threads;
int allow_daemon_threads;
int check_multi_interp_extensions;
} _PyInterpreterConfig;

#define _PyInterpreterConfig_INIT \
Expand All @@ -256,6 +257,7 @@ typedef struct {
.allow_exec = 0, \
.allow_threads = 1, \
.allow_daemon_threads = 0, \
.check_multi_interp_extensions = 1, \
}

#define _PyInterpreterConfig_LEGACY_INIT \
Expand All @@ -264,6 +266,7 @@ typedef struct {
.allow_exec = 1, \
.allow_threads = 1, \
.allow_daemon_threads = 1, \
.check_multi_interp_extensions = 0, \
}

/* --- Helper functions --------------------------------------- */
Expand Down
3 changes: 3 additions & 0 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ is available in a given context. For example, forking the process
might not be allowed in the current interpreter (i.e. os.fork() would fail).
*/

/* Set if import should check a module for subinterpreter support. */
#define Py_RTFLAGS_MULTI_INTERP_EXTENSIONS (1UL << 8)

/* Set if threads are allowed. */
#define Py_RTFLAGS_THREADS (1UL << 10)

Expand Down
5 changes: 5 additions & 0 deletions Include/internal/pycore_import.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ struct _import_state {
/* override for config->use_frozen_modules (for tests)
(-1: "off", 1: "on", 0: no override) */
int override_frozen_modules;
int override_multi_interp_extensions_check;
#ifdef HAVE_DLOPEN
int dlopenflags;
#endif
Expand Down Expand Up @@ -153,6 +154,10 @@ PyAPI_DATA(const struct _frozen *) _PyImport_FrozenStdlib;
PyAPI_DATA(const struct _frozen *) _PyImport_FrozenTest;
extern const struct _module_alias * _PyImport_FrozenAliases;

PyAPI_FUNC(int) _PyImport_CheckSubinterpIncompatibleExtensionAllowed(
const char *name);


// for testing
PyAPI_FUNC(int) _PyImport_ClearExtension(PyObject *name, PyObject *filename);

Expand Down
18 changes: 18 additions & 0 deletions Lib/test/support/import_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,24 @@ def frozen_modules(enabled=True):
_imp._override_frozen_modules_for_tests(0)


@contextlib.contextmanager
def multi_interp_extensions_check(enabled=True):
"""Force legacy modules to be allowed in subinterpreters (or not).
("legacy" == single-phase init)
This only applies to modules that haven't been imported yet.
It overrides the PyInterpreterConfig.check_multi_interp_extensions
setting (see support.run_in_subinterp_with_config() and
_xxsubinterpreters.create()).
"""
old = _imp._override_multi_interp_extensions_check(1 if enabled else -1)
try:
yield
finally:
_imp._override_multi_interp_extensions_check(old)


def import_fresh_module(name, fresh=(), blocked=(), *,
deprecated=False,
usefrozen=False,
Expand Down
77 changes: 77 additions & 0 deletions Lib/test/test_capi/check_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# This script is used by test_misc.

import _imp
import _testinternalcapi
import json
import os
import sys


def import_singlephase():
assert '_testsinglephase' not in sys.modules
try:
import _testsinglephase
except ImportError:
sys.modules.pop('_testsinglephase')
return False
else:
del sys.modules['_testsinglephase']
return True


def check_singlephase(override):
# Check using the default setting.
settings_initial = _testinternalcapi.get_interp_settings()
allowed_initial = import_singlephase()
assert(_testinternalcapi.get_interp_settings() == settings_initial)

# Apply the override and check.
override_initial = _imp._override_multi_interp_extensions_check(override)
settings_after = _testinternalcapi.get_interp_settings()
allowed_after = import_singlephase()

# Apply the override again and check.
noop = {}
override_after = _imp._override_multi_interp_extensions_check(override)
settings_noop = _testinternalcapi.get_interp_settings()
if settings_noop != settings_after:
noop['settings_noop'] = settings_noop
allowed_noop = import_singlephase()
if allowed_noop != allowed_after:
noop['allowed_noop'] = allowed_noop

# Restore the original setting and check.
override_noop = _imp._override_multi_interp_extensions_check(override_initial)
if override_noop != override_after:
noop['override_noop'] = override_noop
settings_restored = _testinternalcapi.get_interp_settings()
allowed_restored = import_singlephase()

# Restore the original setting again.
override_restored = _imp._override_multi_interp_extensions_check(override_initial)
assert(_testinternalcapi.get_interp_settings() == settings_restored)

return dict({
'requested': override,
'override__initial': override_initial,
'override_after': override_after,
'override_restored': override_restored,
'settings__initial': settings_initial,
'settings_after': settings_after,
'settings_restored': settings_restored,
'allowed__initial': allowed_initial,
'allowed_after': allowed_after,
'allowed_restored': allowed_restored,
}, **noop)


def run_singlephase_check(override, outfd):
with os.fdopen(outfd, 'w') as outfile:
sys.stdout = outfile
sys.stderr = outfile
try:
results = check_singlephase(override)
json.dump(results, outfile)
finally:
sys.stdout = sys.__stdout__
sys.stderr = sys.__stderr__
98 changes: 93 additions & 5 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
import _testmultiphase
except ImportError:
_testmultiphase = None
try:
import _testsinglephase
except ImportError:
_testsinglephase = None

# Skip this test if the _testcapi module isn't available.
_testcapi = import_helper.import_module('_testcapi')
Expand Down Expand Up @@ -1297,17 +1301,20 @@ def test_configured_settings(self):
"""
import json

EXTENSIONS = 1<<8
THREADS = 1<<10
DAEMON_THREADS = 1<<11
FORK = 1<<15
EXEC = 1<<16

features = ['fork', 'exec', 'threads', 'daemon_threads']
features = ['fork', 'exec', 'threads', 'daemon_threads', 'extensions']
kwlist = [f'allow_{n}' for n in features]
kwlist[-1] = 'check_multi_interp_extensions'
for config, expected in {
(True, True, True, True): FORK | EXEC | THREADS | DAEMON_THREADS,
(False, False, False, False): 0,
(False, False, True, False): THREADS,
(True, True, True, True, True):
FORK | EXEC | THREADS | DAEMON_THREADS | EXTENSIONS,
(False, False, False, False, False): 0,
(False, False, True, False, True): THREADS | EXTENSIONS,
}.items():
kwargs = dict(zip(kwlist, config))
expected = {
Expand All @@ -1322,12 +1329,93 @@ def test_configured_settings(self):
json.dump(settings, stdin)
''')
with os.fdopen(r) as stdout:
support.run_in_subinterp_with_config(script, **kwargs)
ret = support.run_in_subinterp_with_config(script, **kwargs)
self.assertEqual(ret, 0)
out = stdout.read()
settings = json.loads(out)

self.assertEqual(settings, expected)

@unittest.skipIf(_testsinglephase is None, "test requires _testsinglephase module")
@unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
def test_overridden_setting_extensions_subinterp_check(self):
"""
PyInterpreterConfig.check_multi_interp_extensions can be overridden
with PyInterpreterState.override_multi_interp_extensions_check.
This verifies that the override works but does not modify
the underlying setting.
"""
import json

EXTENSIONS = 1<<8
THREADS = 1<<10
DAEMON_THREADS = 1<<11
FORK = 1<<15
EXEC = 1<<16
BASE_FLAGS = FORK | EXEC | THREADS | DAEMON_THREADS
base_kwargs = {
'allow_fork': True,
'allow_exec': True,
'allow_threads': True,
'allow_daemon_threads': True,
}

def check(enabled, override):
kwargs = dict(
base_kwargs,
check_multi_interp_extensions=enabled,
)
flags = BASE_FLAGS | EXTENSIONS if enabled else BASE_FLAGS
settings = {
'feature_flags': flags,
}

expected = {
'requested': override,
'override__initial': 0,
'override_after': override,
'override_restored': 0,
# The override should not affect the config or settings.
'settings__initial': settings,
'settings_after': settings,
'settings_restored': settings,
# These are the most likely values to be wrong.
'allowed__initial': not enabled,
'allowed_after': not ((override > 0) if override else enabled),
'allowed_restored': not enabled,
}

r, w = os.pipe()
script = textwrap.dedent(f'''
from test.test_capi.check_config import run_singlephase_check
run_singlephase_check({override}, {w})
''')
with os.fdopen(r) as stdout:
ret = support.run_in_subinterp_with_config(script, **kwargs)
self.assertEqual(ret, 0)
out = stdout.read()
results = json.loads(out)

self.assertEqual(results, expected)

self.maxDiff = None

# setting: check disabled
with self.subTest('config: check disabled; override: disabled'):
check(False, -1)
with self.subTest('config: check disabled; override: use config'):
check(False, 0)
with self.subTest('config: check disabled; override: enabled'):
check(False, 1)

# setting: check enabled
with self.subTest('config: check enabled; override: disabled'):
check(True, -1)
with self.subTest('config: check enabled; override: use config'):
check(True, 0)
with self.subTest('config: check enabled; override: enabled'):
check(True, 1)

def test_mutate_exception(self):
"""
Exceptions saved in global module state get shared between
Expand Down
4 changes: 3 additions & 1 deletion Lib/test/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -1656,13 +1656,15 @@ def test_init_use_frozen_modules(self):
api=API_PYTHON, env=env)

def test_init_main_interpreter_settings(self):
EXTENSIONS = 1<<8
THREADS = 1<<10
DAEMON_THREADS = 1<<11
FORK = 1<<15
EXEC = 1<<16
expected = {
# All optional features should be enabled.
'feature_flags': FORK | EXEC | THREADS | DAEMON_THREADS,
'feature_flags':
FORK | EXEC | THREADS | DAEMON_THREADS,
}
out, err = self.run_embedded_interpreter(
'test_init_main_interpreter_settings',
Expand Down
Loading

0 comments on commit 89ac665

Please sign in to comment.