]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-98610: Adjust the Optional Restrictions on Subinterpreters (GH-98618)
authorEric Snow <ericsnowcurrently@gmail.com>
Mon, 31 Oct 2022 19:35:54 +0000 (13:35 -0600)
committerGitHub <noreply@github.com>
Mon, 31 Oct 2022 19:35:54 +0000 (12:35 -0700)
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`.

15 files changed:
Include/cpython/initconfig.h
Include/cpython/pystate.h
Lib/test/test__xxsubinterpreters.py
Lib/test/test_capi.py
Lib/test/test_embed.py
Lib/test/test_threading.py
Lib/threading.py
Misc/NEWS.d/next/C API/2022-10-24-12-09-17.gh-issue-98610.PLX2Np.rst [new file with mode: 0644]
Modules/_posixsubprocess.c
Modules/_testcapimodule.c
Modules/_threadmodule.c
Modules/_winapi.c
Modules/_xxsubinterpretersmodule.c
Modules/posixmodule.c
Python/pylifecycle.c

index 64748cf78beb7238a50ca888f6fc1123f8235e12..6ce42b4c09502f62e8842695e76aa670abd4ed9d 100644 (file)
@@ -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 --------------------------------------- */
index 7996bd34eac9d725b0d5e8322b1cf0d70b051a1f..70c23427807e52d27b5005001e8e9a9c8170ab3a 100644 (file)
@@ -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,
index f20aae8e21c66fafff1788ec28df73c0453c43eb..66f29b95af10c3f5456ad985cdc41d269f55a52b 100644 (file)
@@ -801,7 +801,7 @@ class RunStringTests(TestBase):
         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 @@ class RunStringTests(TestBase):
 
         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
index 2a35576fc57e36b0e625a1291ad44a284a433864..49f207ed953c2d02cb4b81da9a4ba901dfcc01d0 100644 (file)
@@ -1148,15 +1148,16 @@ class SubinterpreterTest(unittest.TestCase):
         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 = {
index 930f76342f0e1447411d1eb067a7948d36479084..fa9815c681e0ae15119aaa942ebf8044b9c51ac7 100644 (file)
@@ -1655,11 +1655,12 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
 
     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',
index c6649962331464a671d28ca1dce4e2da952b57e4..13ba5068ae20d75e419ac2349ebb709b87fbf42a 100644 (file)
@@ -1305,6 +1305,62 @@ class SubinterpThreadingTests(BaseTestCase):
         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
index d030e1243623f981bdd3a63e82179365b5ce63dd..723bd58bf5a68b4bee57c768188234ec80056556 100644 (file)
@@ -33,6 +33,7 @@ __all__ = ['get_ident', 'active_count', 'Condition', 'current_thread',
 
 # 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 Thread:
         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 @@ class Thread:
     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 @@ class _MainThread(Thread):
 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 (file)
index 0000000..05bcfae
--- /dev/null
@@ -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`.
index 8275b116093d5eb7caaa18447346249a4695eb20..717b1cf2202105ce79706ba82954ebf6bb250236 100644 (file)
@@ -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();
index bade0db64f5193db32854c59ca64818a3778b998..19ceb108ed4e36357b69e0fdf500876cb8bcea4f 100644 (file)
@@ -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) {
index 93b3b8d85d66049da1ec3e245fc11b71dabd9254..5968d4e2e0ee152c32333a4cec2fee2237d19a89 100644 (file)
@@ -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,
index 2a916cc9f46767a5e7d0981e4ffddc7c1681c606..71e74d155cdc00e961b9ce78a7423648968467f9 100644 (file)
@@ -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);
 
index f38de57f69e2bebb502c57b3065a1bcecee31c21..9d979ef113e6866cda4c9a88d2fcdbd990e210da 100644 (file)
@@ -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);
index a5eb8663121184e94e4edb5cb0c9684299d327ce..d863f9f63248f59918f60012c89d2dc1c28379da 100644 (file)
@@ -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. */
index d26ae74a0f17f5d4ee363a56627d24f1a360d84b..e64849296c57f7815c1ff7d64cd06349b76dc1e6 100644 (file)
@@ -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;
+    }
 }