Skip to content

Commit

Permalink
gh-98610: Adjust the Optional Restrictions on Subinterpreters (GH-98618)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
ericsnowcurrently authored Oct 31, 2022
1 parent 3b86538 commit 4702552
Show file tree
Hide file tree
Showing 15 changed files with 220 additions and 47 deletions.
14 changes: 12 additions & 2 deletions Include/cpython/initconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 --------------------------------------- */
Expand Down
13 changes: 7 additions & 6 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
57 changes: 56 additions & 1 deletion Lib/test/test__xxsubinterpreters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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
Expand Down
11 changes: 6 additions & 5 deletions Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
5 changes: 3 additions & 2 deletions Lib/test/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
56 changes: 56 additions & 0 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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`.
11 changes: 2 additions & 9 deletions Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
24 changes: 17 additions & 7 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -3231,42 +3231,52 @@ 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();

PyThreadState_Swap(NULL);

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) {
Expand Down
20 changes: 20 additions & 0 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 4702552

Please sign in to comment.