https://github.com/python/cpython/commit/4702552885811d0af8f0e4545f494336801... commit: 4702552885811d0af8f0e4545f494336801ad4dd branch: main author: Eric Snow <ericsnowcurrently@gmail.com> committer: miss-islington <31488909+miss-islington@users.noreply.github.com> date: 2022-10-31T12:35:54-07:00 summary: gh-98610: Adjust the Optional Restrictions on Subinterpreters (GH-98618) Previously, the optional restrictions on subinterpreters were: disallow fork, subprocess, and threads. By default, we were disallowing all three for "isolated" interpreters. We always allowed all three for the main interpreter and those created through the legacy `Py_NewInterpreter()` API. Those settings were a bit conservative, so here we've adjusted the optional restrictions to: fork, exec, threads, and daemon threads. The default for "isolated" interpreters disables fork, exec, and daemon threads. Regular threads are allowed by default. We continue always allowing everything For the main interpreter and the legacy API. In the code, we add `_PyInterpreterConfig.allow_exec` and `_PyInterpreterConfig.allow_daemon_threads`. We also add `Py_RTFLAGS_DAEMON_THREADS` and `Py_RTFLAGS_EXEC`. files: A Misc/NEWS.d/next/C API/2022-10-24-12-09-17.gh-issue-98610.PLX2Np.rst M Include/cpython/initconfig.h M Include/cpython/pystate.h M Lib/test/test__xxsubinterpreters.py M Lib/test/test_capi.py M Lib/test/test_embed.py M Lib/test/test_threading.py M Lib/threading.py M Modules/_posixsubprocess.c M Modules/_testcapimodule.c M Modules/_threadmodule.c M Modules/_winapi.c M Modules/_xxsubinterpretersmodule.c M Modules/posixmodule.c M Python/pylifecycle.c diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index 64748cf78beb..6ce42b4c0950 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -245,15 +245,25 @@ PyAPI_FUNC(PyStatus) PyConfig_SetWideStringList(PyConfig *config, typedef struct { int allow_fork; - int allow_subprocess; + int allow_exec; int allow_threads; + int allow_daemon_threads; } _PyInterpreterConfig; +#define _PyInterpreterConfig_INIT \ + { \ + .allow_fork = 0, \ + .allow_exec = 0, \ + .allow_threads = 1, \ + .allow_daemon_threads = 0, \ + } + #define _PyInterpreterConfig_LEGACY_INIT \ { \ .allow_fork = 1, \ - .allow_subprocess = 1, \ + .allow_exec = 1, \ .allow_threads = 1, \ + .allow_daemon_threads = 1, \ } /* --- Helper functions --------------------------------------- */ diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 7996bd34eac9..70c23427807e 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -11,16 +11,17 @@ 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). */ -// We leave the first 10 for less-specific features. - /* Set if threads are allowed. */ -#define Py_RTFLAGS_THREADS (1UL << 10) +#define Py_RTFLAGS_THREADS (1UL << 10) + +/* Set if daemon threads are allowed. */ +#define Py_RTFLAGS_DAEMON_THREADS (1UL << 11) /* Set if os.fork() is allowed. */ -#define Py_RTFLAGS_FORK (1UL << 15) +#define Py_RTFLAGS_FORK (1UL << 15) -/* Set if subprocesses are allowed. */ -#define Py_RTFLAGS_SUBPROCESS (1UL << 16) +/* Set if os.exec*() is allowed. */ +#define Py_RTFLAGS_EXEC (1UL << 16) PyAPI_FUNC(int) _PyInterpreterState_HasFeature(PyInterpreterState *interp, diff --git a/Lib/test/test__xxsubinterpreters.py b/Lib/test/test__xxsubinterpreters.py index f20aae8e21c6..66f29b95af10 100644 --- a/Lib/test/test__xxsubinterpreters.py +++ b/Lib/test/test__xxsubinterpreters.py @@ -801,7 +801,7 @@ def f(): self.assertEqual(out, 'it worked!') def test_create_thread(self): - subinterp = interpreters.create(isolated=False) + subinterp = interpreters.create() script, file = _captured_script(""" import threading def f(): @@ -817,6 +817,61 @@ def f(): self.assertEqual(out, 'it worked!') + def test_create_daemon_thread(self): + with self.subTest('isolated'): + expected = 'spam spam spam spam spam' + subinterp = interpreters.create(isolated=True) + script, file = _captured_script(f""" + import threading + def f(): + print('it worked!', end='') + + try: + t = threading.Thread(target=f, daemon=True) + t.start() + t.join() + except RuntimeError: + print('{expected}', end='') + """) + with file: + interpreters.run_string(subinterp, script) + out = file.read() + + self.assertEqual(out, expected) + + with self.subTest('not isolated'): + subinterp = interpreters.create(isolated=False) + script, file = _captured_script(""" + import threading + def f(): + print('it worked!', end='') + + t = threading.Thread(target=f, daemon=True) + t.start() + t.join() + """) + with file: + interpreters.run_string(subinterp, script) + out = file.read() + + self.assertEqual(out, 'it worked!') + + def test_os_exec(self): + expected = 'spam spam spam spam spam' + subinterp = interpreters.create() + script, file = _captured_script(f""" + import os, sys + try: + os.execl(sys.executable) + except RuntimeError: + print('{expected}', end='') + """) + with file: + interpreters.run_string(subinterp, script) + out = file.read() + + self.assertEqual(out, expected) + @support.requires_fork() def test_fork(self): import tempfile diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 2a35576fc57e..49f207ed953c 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -1148,15 +1148,16 @@ def test_configured_settings(self): import json THREADS = 1<<10 + DAEMON_THREADS = 1<<11 FORK = 1<<15 - SUBPROCESS = 1<<16 + EXEC = 1<<16 - features = ['fork', 'subprocess', 'threads'] + features = ['fork', 'exec', 'threads', 'daemon_threads'] kwlist = [f'allow_{n}' for n in features] for config, expected in { - (True, True, True): FORK | SUBPROCESS | THREADS, - (False, False, False): 0, - (False, True, True): SUBPROCESS | THREADS, + (True, True, True, True): FORK | EXEC | THREADS | DAEMON_THREADS, + (False, False, False, False): 0, + (False, False, True, False): THREADS, }.items(): kwargs = dict(zip(kwlist, config)) expected = { diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 930f76342f0e..fa9815c681e0 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -1655,11 +1655,12 @@ def test_init_use_frozen_modules(self): def test_init_main_interpreter_settings(self): THREADS = 1<<10 + DAEMON_THREADS = 1<<11 FORK = 1<<15 - SUBPROCESS = 1<<16 + EXEC = 1<<16 expected = { # All optional features should be enabled. - 'feature_flags': THREADS | FORK | SUBPROCESS, + 'feature_flags': FORK | EXEC | THREADS | DAEMON_THREADS, } out, err = self.run_embedded_interpreter( 'test_init_main_interpreter_settings', diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index c66499623314..13ba5068ae20 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1305,6 +1305,62 @@ def f(): self.assertIn("Fatal Python error: Py_EndInterpreter: " "not the last thread", err.decode()) + def _check_allowed(self, before_start='', *, + allowed=True, + daemon_allowed=True, + daemon=False, + ): + subinterp_code = textwrap.dedent(f""" + import test.support + import threading + def func(): + print('this should not have run!') + t = threading.Thread(target=func, daemon={daemon}) + {before_start} + t.start() + """) + script = textwrap.dedent(f""" + import test.support + test.support.run_in_subinterp_with_config( + {subinterp_code!r}, + allow_fork=True, + allow_exec=True, + allow_threads={allowed}, + allow_daemon_threads={daemon_allowed}, + ) + """) + with test.support.SuppressCrashReport(): + _, _, err = assert_python_ok("-c", script) + return err.decode() + + @cpython_only + def test_threads_not_allowed(self): + err = self._check_allowed( + allowed=False, + daemon_allowed=False, + daemon=False, + ) + self.assertIn('RuntimeError', err) + + @cpython_only + def test_daemon_threads_not_allowed(self): + with self.subTest('via Thread()'): + err = self._check_allowed( + allowed=True, + daemon_allowed=False, + daemon=True, + ) + self.assertIn('RuntimeError', err) + + with self.subTest('via Thread.daemon setter'): + err = self._check_allowed( + 't.daemon = True', + allowed=True, + daemon_allowed=False, + daemon=False, + ) + self.assertIn('RuntimeError', err) + class ThreadingExceptionTests(BaseTestCase): # A RuntimeError should be raised if Thread.start() is called diff --git a/Lib/threading.py b/Lib/threading.py index d030e1243623..723bd58bf5a6 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -33,6 +33,7 @@ # Rename some stuff so "from threading import *" is safe _start_new_thread = _thread.start_new_thread +_daemon_threads_allowed = _thread.daemon_threads_allowed _allocate_lock = _thread.allocate_lock _set_sentinel = _thread._set_sentinel get_ident = _thread.get_ident @@ -899,6 +900,8 @@ class is implemented. self._args = args self._kwargs = kwargs if daemon is not None: + if daemon and not _daemon_threads_allowed(): + raise RuntimeError('daemon threads are disabled in this (sub)interpreter') self._daemonic = daemon else: self._daemonic = current_thread().daemon @@ -1226,6 +1229,8 @@ def daemon(self): def daemon(self, daemonic): if not self._initialized: raise RuntimeError("Thread.__init__() not called") + if daemonic and not _daemon_threads_allowed(): + raise RuntimeError('daemon threads are disabled in this interpreter') if self._started.is_set(): raise RuntimeError("cannot set daemon status of active thread") self._daemonic = daemonic @@ -1432,7 +1437,8 @@ def __init__(self): class _DummyThread(Thread): def __init__(self): - Thread.__init__(self, name=_newname("Dummy-%d"), daemon=True) + Thread.__init__(self, name=_newname("Dummy-%d"), + daemon=_daemon_threads_allowed()) self._started.set() self._set_ident() diff --git a/Misc/NEWS.d/next/C API/2022-10-24-12-09-17.gh-issue-98610.PLX2Np.rst b/Misc/NEWS.d/next/C API/2022-10-24-12-09-17.gh-issue-98610.PLX2Np.rst new file mode 100644 index 000000000000..05bcfae1b61b --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-10-24-12-09-17.gh-issue-98610.PLX2Np.rst @@ -0,0 +1,9 @@ +Some configurable capabilities of sub-interpreters have changed. +They always allow subprocesses (:mod:`subprocess`) now, whereas before +subprocesses could be optionally disaallowed for a sub-interpreter. +Instead :func:`os.exec` can now be disallowed. +Disallowing daemon threads is now supported. Disallowing all threads +is still allowed, but is never done by default. +Note that the optional restrictions are only available through +``_Py_NewInterpreterFromConfig()``, which isn't a public API. +They do not affect the main interpreter, nor :c:func:`Py_NewInterpreter`. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 8275b116093d..717b1cf22021 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -825,8 +825,8 @@ subprocess_fork_exec(PyObject *module, PyObject *args) &preexec_fn, &allow_vfork)) return NULL; - if ((preexec_fn != Py_None) && - (PyInterpreterState_Get() != PyInterpreterState_Main())) { + PyInterpreterState *interp = PyInterpreterState_Get(); + if ((preexec_fn != Py_None) && (interp != PyInterpreterState_Main())) { PyErr_SetString(PyExc_RuntimeError, "preexec_fn not supported within subinterpreters"); return NULL; @@ -841,13 +841,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args) return NULL; } - PyInterpreterState *interp = PyInterpreterState_Get(); - if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_SUBPROCESS)) { - PyErr_SetString(PyExc_RuntimeError, - "subprocess not supported for isolated subinterpreters"); - return NULL; - } - /* We need to call gc.disable() when we'll be calling preexec_fn */ if (preexec_fn != Py_None) { need_to_reenable_gc = PyGC_Disable(); diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index bade0db64f51..19ceb108ed4e 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3231,33 +3231,42 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) { const char *code; int allow_fork = -1; - int allow_subprocess = -1; + int allow_exec = -1; int allow_threads = -1; + int allow_daemon_threads = -1; int r; PyThreadState *substate, *mainstate; /* only initialise 'cflags.cf_flags' to test backwards compatibility */ PyCompilerFlags cflags = {0}; static char *kwlist[] = {"code", - "allow_fork", "allow_subprocess", "allow_threads", + "allow_fork", + "allow_exec", + "allow_threads", + "allow_daemon_threads", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "s$ppp:run_in_subinterp_with_config", kwlist, - &code, &allow_fork, &allow_subprocess, &allow_threads)) { + "s$pppp:run_in_subinterp_with_config", kwlist, + &code, &allow_fork, &allow_exec, + &allow_threads, &allow_daemon_threads)) { return NULL; } if (allow_fork < 0) { PyErr_SetString(PyExc_ValueError, "missing allow_fork"); return NULL; } - if (allow_subprocess < 0) { - PyErr_SetString(PyExc_ValueError, "missing allow_subprocess"); + if (allow_exec < 0) { + PyErr_SetString(PyExc_ValueError, "missing allow_exec"); return NULL; } if (allow_threads < 0) { PyErr_SetString(PyExc_ValueError, "missing allow_threads"); return NULL; } + if (allow_daemon_threads < 0) { + PyErr_SetString(PyExc_ValueError, "missing allow_daemon_threads"); + return NULL; + } mainstate = PyThreadState_Get(); @@ -3265,8 +3274,9 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs) const _PyInterpreterConfig config = { .allow_fork = allow_fork, - .allow_subprocess = allow_subprocess, + .allow_exec = allow_exec, .allow_threads = allow_threads, + .allow_daemon_threads = allow_daemon_threads, }; substate = _Py_NewInterpreterFromConfig(&config); if (substate == NULL) { diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 93b3b8d85d66..5968d4e2e0ee 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1102,6 +1102,24 @@ thread_run(void *boot_raw) // to open the libgcc_s.so library (ex: EMFILE error). } +static PyObject * +thread_daemon_threads_allowed(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + PyInterpreterState *interp = _PyInterpreterState_Get(); + if (interp->feature_flags & Py_RTFLAGS_DAEMON_THREADS) { + Py_RETURN_TRUE; + } + else { + Py_RETURN_FALSE; + } +} + +PyDoc_STRVAR(daemon_threads_allowed_doc, +"daemon_threads_allowed()\n\ +\n\ +Return True if daemon threads are allowed in the current interpreter,\n\ +and False otherwise.\n"); + static PyObject * thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) { @@ -1543,6 +1561,8 @@ static PyMethodDef thread_methods[] = { METH_VARARGS, start_new_doc}, {"start_new", (PyCFunction)thread_PyThread_start_new_thread, METH_VARARGS, start_new_doc}, + {"daemon_threads_allowed", (PyCFunction)thread_daemon_threads_allowed, + METH_NOARGS, daemon_threads_allowed_doc}, {"allocate_lock", thread_PyThread_allocate_lock, METH_NOARGS, allocate_doc}, {"allocate", thread_PyThread_allocate_lock, diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 2a916cc9f467..71e74d155cdc 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1089,13 +1089,6 @@ _winapi_CreateProcess_impl(PyObject *module, return NULL; } - PyInterpreterState *interp = PyInterpreterState_Get(); - if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_SUBPROCESS)) { - PyErr_SetString(PyExc_RuntimeError, - "subprocess not supported for isolated subinterpreters"); - return NULL; - } - ZeroMemory(&si, sizeof(si)); si.StartupInfo.cb = sizeof(si); diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index f38de57f69e2..9d979ef113e6 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -2003,11 +2003,9 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) // Create and initialize the new interpreter. PyThreadState *save_tstate = _PyThreadState_GET(); - const _PyInterpreterConfig config = { - .allow_fork = !isolated, - .allow_subprocess = !isolated, - .allow_threads = !isolated, - }; + const _PyInterpreterConfig config = isolated + ? (_PyInterpreterConfig)_PyInterpreterConfig_INIT + : (_PyInterpreterConfig)_PyInterpreterConfig_LEGACY_INIT; // XXX Possible GILState issues? PyThreadState *tstate = _Py_NewInterpreterFromConfig(&config); PyThreadState_Swap(save_tstate); diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index a5eb86631211..d863f9f63248 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -5773,6 +5773,13 @@ os_execv_impl(PyObject *module, path_t *path, PyObject *argv) EXECV_CHAR **argvlist; Py_ssize_t argc; + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_EXEC)) { + PyErr_SetString(PyExc_RuntimeError, + "exec not supported for isolated subinterpreters"); + return NULL; + } + /* execv has two arguments: (path, argv), where argv is a list or tuple of strings. */ @@ -5839,6 +5846,13 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env) EXECV_CHAR **envlist; Py_ssize_t argc, envc; + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_EXEC)) { + PyErr_SetString(PyExc_RuntimeError, + "exec not supported for isolated subinterpreters"); + return NULL; + } + /* execve has three arguments: (path, argv, env), where argv is a list or tuple of strings and env is a dictionary like posix.environ. */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index d26ae74a0f17..e64849296c57 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -615,15 +615,21 @@ static void init_interp_settings(PyInterpreterState *interp, const _PyInterpreterConfig *config) { assert(interp->feature_flags == 0); + if (config->allow_fork) { interp->feature_flags |= Py_RTFLAGS_FORK; } - if (config->allow_subprocess) { - interp->feature_flags |= Py_RTFLAGS_SUBPROCESS; + if (config->allow_exec) { + interp->feature_flags |= Py_RTFLAGS_EXEC; } + // Note that fork+exec is always allowed. + if (config->allow_threads) { interp->feature_flags |= Py_RTFLAGS_THREADS; } + if (config->allow_daemon_threads) { + interp->feature_flags |= Py_RTFLAGS_DAEMON_THREADS; + } }