gh-98608: Change _Py_NewInterpreter() to _Py_NewInterpreterFromConfig() (gh-98609)

ericsnowcurrently · web-flow · commit f32369480df5 · 2022-10-26T11:16:30.000-06:00
(see #98608) This change does the following: 1. change the argument to a new `_PyInterpreterConfig` struct 2. rename the function to `_Py_NewInterpreterFromConfig()`, inspired by `Py_InitializeFromConfig()` (takes a `_PyInterpreterConfig` instead of `isolated_subinterpreter`) 3. split up the boolean `isolated_subinterpreter` into the corresponding multiple granular settings * allow_fork * allow_subprocess * allow_threads 4. add `PyInterpreterState.feature_flags` to store those settings 5. add a function for checking if a feature is enabled on an opaque `PyInterpreterState *` 6. drop `PyConfig._isolated_interpreter` The existing default (see `Py_NewInterpeter()` and `Py_Initialize*()`) allows fork, subprocess, and threads and the optional "isolated" interpreter (see the `_xxsubinterpreters` module) disables all three. None of that changes here; the defaults are preserved. Note that the given `_PyInterpreterConfig` will not be used outside `_Py_NewInterpreterFromConfig()`, nor preserved. This contrasts with how `PyConfig` is currently preserved, used, and even modified outside `Py_InitializeFromConfig()`. I'd rather just avoid that mess from the start for `_PyInterpreterConfig`. We can preserve it later if we find an actual need. This change allows us to follow up with a number of improvements (e.g. stop disallowing subprocess and support disallowing exec instead). (Note that this PR adds "private" symbols. We'll probably make them public, and add docs, in a separate change.)
diff --git a/Doc/c-api/init_config.rst b/Doc/c-api/init_config.rst
@@ -1571,8 +1571,6 @@ Private provisional API:
 
 * :c:member:`PyConfig._init_main`: if set to ``0``,
   :c:func:`Py_InitializeFromConfig` stops at the "Core" initialization phase.
-* :c:member:`PyConfig._isolated_interpreter`: if non-zero,
-  disallow threads, subprocesses and fork.
 
 .. c:function:: PyStatus _Py_InitializeMain(void)
 
diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h
@@ -213,10 +213,6 @@ typedef struct PyConfig {
     // If equal to 0, stop Python initialization before the "main" phase.
     int _init_main;
 
-    // If non-zero, disallow threads, subprocesses, and fork.
-    // Default: 0.
-    int _isolated_interpreter;
-
     // If non-zero, we believe we're running from a source tree.
     int _is_python_build;
 } PyConfig;
@@ -245,6 +241,21 @@ PyAPI_FUNC(PyStatus) PyConfig_SetWideStringList(PyConfig *config,
     Py_ssize_t length, wchar_t **items);
 
 
+/* --- PyInterpreterConfig ------------------------------------ */
+
+typedef struct {
+    int allow_fork;
+    int allow_subprocess;
+    int allow_threads;
+} _PyInterpreterConfig;
+
+#define _PyInterpreterConfig_LEGACY_INIT \
+    { \
+        .allow_fork = 1, \
+        .allow_subprocess = 1, \
+        .allow_threads = 1, \
+    }
+
 /* --- Helper functions --------------------------------------- */
 
 /* Get the original command line arguments, before Python modified them.
diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h
@@ -62,4 +62,5 @@ PyAPI_FUNC(int) _Py_CoerceLegacyLocale(int warn);
 PyAPI_FUNC(int) _Py_LegacyLocaleDetected(int warn);
 PyAPI_FUNC(char *) _Py_SetLocaleFromEnv(int category);
 
-PyAPI_FUNC(PyThreadState *) _Py_NewInterpreter(int isolated_subinterpreter);
+PyAPI_FUNC(PyThreadState *) _Py_NewInterpreterFromConfig(
+    const _PyInterpreterConfig *);
diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h
@@ -3,11 +3,38 @@
 #endif
 
 
+/*
+Runtime Feature Flags
+
+Each flag indicate whether or not a specific runtime feature
+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)
+
+/* Set if os.fork() is allowed. */
+#define Py_RTFLAGS_FORK         (1UL << 15)
+
+/* Set if subprocesses are allowed. */
+#define Py_RTFLAGS_SUBPROCESS   (1UL << 16)
+
+
+PyAPI_FUNC(int) _PyInterpreterState_HasFeature(PyInterpreterState *interp,
+                                               unsigned long feature);
+
+
+/* private interpreter helpers */
+
 PyAPI_FUNC(int) _PyInterpreterState_RequiresIDRef(PyInterpreterState *);
 PyAPI_FUNC(void) _PyInterpreterState_RequireIDRef(PyInterpreterState *, int);
 
 PyAPI_FUNC(PyObject *) _PyInterpreterState_GetMainModule(PyInterpreterState *);
 
+
 /* State unique per thread */
 
 /* Py_tracefunc return -1 when raising an exception, or 0 for success. */
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -143,6 +143,7 @@ struct _is {
 #ifdef HAVE_DLOPEN
     int dlopenflags;
 #endif
+    unsigned long feature_flags;
 
     PyObject *dict;  /* Stores per-interpreter state */
 
diff --git a/Lib/test/_test_embed_set_config.py b/Lib/test/_test_embed_set_config.py
@@ -84,7 +84,6 @@ def test_set_invalid(self):
             'skip_source_first_line',
             '_install_importlib',
             '_init_main',
-            '_isolated_interpreter',
         ]
         if MS_WINDOWS:
             options.append('legacy_windows_stdio')
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
@@ -1793,6 +1793,22 @@ def run_in_subinterp(code):
     Run code in a subinterpreter. Raise unittest.SkipTest if the tracemalloc
     module is enabled.
     """
+    _check_tracemalloc()
+    import _testcapi
+    return _testcapi.run_in_subinterp(code)
+
+
+def run_in_subinterp_with_config(code, **config):
+    """
+    Run code in a subinterpreter. Raise unittest.SkipTest if the tracemalloc
+    module is enabled.
+    """
+    _check_tracemalloc()
+    import _testcapi
+    return _testcapi.run_in_subinterp_with_config(code, **config)
+
+
+def _check_tracemalloc():
     # Issue #10915, #15751: PyGILState_*() functions don't work with
     # sub-interpreters, the tracemalloc module uses these functions internally
     try:
@@ -1804,8 +1820,6 @@ def run_in_subinterp(code):
             raise unittest.SkipTest("run_in_subinterp() cannot be used "
                                      "if tracemalloc module is tracing "
                                      "memory allocations")
-    import _testcapi
-    return _testcapi.run_in_subinterp(code)
 
 
 def check_free_after_iterating(test, iter, cls, args=()):
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
@@ -1096,6 +1096,45 @@ def test_py_config_isoloated_per_interpreter(self):
         # test fails, assume that the environment in this process may
         # be altered and suspect.
 
+    @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
+    def test_configured_settings(self):
+        """
+        The config with which an interpreter is created corresponds
+        1-to-1 with the new interpreter's settings.  This test verifies
+        that they match.
+        """
+        import json
+
+        THREADS = 1<<10
+        FORK = 1<<15
+        SUBPROCESS = 1<<16
+
+        features = ['fork', 'subprocess', '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,
+        }.items():
+            kwargs = dict(zip(kwlist, config))
+            expected = {
+                'feature_flags': expected,
+            }
+            with self.subTest(config):
+                r, w = os.pipe()
+                script = textwrap.dedent(f'''
+                    import _testinternalcapi, json, os
+                    settings = _testinternalcapi.get_interp_settings()
+                    with os.fdopen({w}, "w") as stdin:
+                        json.dump(settings, stdin)
+                    ''')
+                with os.fdopen(r) as stdout:
+                    support.run_in_subinterp_with_config(script, **kwargs)
+                    out = stdout.read()
+                settings = json.loads(out)
+
+                self.assertEqual(settings, expected)
+
     def test_mutate_exception(self):
         """
         Exceptions saved in global module state get shared between
diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -496,7 +496,6 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
         'check_hash_pycs_mode': 'default',
         'pathconfig_warnings': 1,
         '_init_main': 1,
-        '_isolated_interpreter': 0,
         'use_frozen_modules': not support.Py_DEBUG,
         'safe_path': 0,
         '_is_python_build': IGNORE_CONFIG,
@@ -881,8 +880,6 @@ def test_init_from_config(self):
 
             'check_hash_pycs_mode': 'always',
             'pathconfig_warnings': 0,
-
-            '_isolated_interpreter': 1,
         }
         self.check_all_configs("test_init_from_config", config, preconfig,
                                api=API_COMPAT)
@@ -1650,6 +1647,25 @@ def test_init_use_frozen_modules(self):
                 self.check_all_configs("test_init_use_frozen_modules", config,
                                        api=API_PYTHON, env=env)
 
+    def test_init_main_interpreter_settings(self):
+        THREADS = 1<<10
+        FORK = 1<<15
+        SUBPROCESS = 1<<16
+        expected = {
+            # All optional features should be enabled.
+            'feature_flags': THREADS | FORK | SUBPROCESS,
+        }
+        out, err = self.run_embedded_interpreter(
+            'test_init_main_interpreter_settings',
+        )
+        self.assertEqual(err, '')
+        try:
+            out = json.loads(out)
+        except json.JSONDecodeError:
+            self.fail(f'fail to decode stdout: {out!r}')
+
+        self.assertEqual(out, expected)
+
 
 class SetConfigTests(unittest.TestCase):
     def test_set_config(self):
diff --git a/Misc/NEWS.d/next/C API/2022-10-24-11-26-55.gh-issue-98608._Q2lNV.rst b/Misc/NEWS.d/next/C API/2022-10-24-11-26-55.gh-issue-98608._Q2lNV.rst
@@ -0,0 +1,4 @@
+A ``_PyInterpreterConfig`` has been added and ``_Py_NewInterpreter()`` has
+been renamed to ``_Py_NewInterpreterFromConfig()``.  The
+"isolated_subinterpreters" argument is now a granular config that captures
+the previous behavior.  Note that this is all "private" API.
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
@@ -842,8 +842,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
     }
 
     PyInterpreterState *interp = PyInterpreterState_Get();
-    const PyConfig *config = _PyInterpreterState_GetConfig(interp);
-    if (config->_isolated_interpreter) {
+    if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_SUBPROCESS)) {
         PyErr_SetString(PyExc_RuntimeError,
                         "subprocess not supported for isolated subinterpreters");
         return NULL;
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -3225,6 +3225,66 @@ run_in_subinterp(PyObject *self, PyObject *args)
     return PyLong_FromLong(r);
 }
 
+/* To run some code in a sub-interpreter. */
+static PyObject *
+run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
+{
+    const char *code;
+    int allow_fork = -1;
+    int allow_subprocess = -1;
+    int allow_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",
+                             NULL};
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs,
+                    "s$ppp:run_in_subinterp_with_config", kwlist,
+                    &code, &allow_fork, &allow_subprocess, &allow_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");
+        return NULL;
+    }
+    if (allow_threads < 0) {
+        PyErr_SetString(PyExc_ValueError, "missing allow_threads");
+        return NULL;
+    }
+
+    mainstate = PyThreadState_Get();
+
+    PyThreadState_Swap(NULL);
+
+    const _PyInterpreterConfig config = {
+        .allow_fork = allow_fork,
+        .allow_subprocess = allow_subprocess,
+        .allow_threads = allow_threads,
+    };
+    substate = _Py_NewInterpreterFromConfig(&config);
+    if (substate == NULL) {
+        /* Since no new thread state was created, there is no exception to
+           propagate; raise a fresh one after swapping in the old thread
+           state. */
+        PyThreadState_Swap(mainstate);
+        PyErr_SetString(PyExc_RuntimeError, "sub-interpreter creation failed");
+        return NULL;
+    }
+    r = PyRun_SimpleStringFlags(code, &cflags);
+    Py_EndInterpreter(substate);
+
+    PyThreadState_Swap(mainstate);
+
+    return PyLong_FromLong(r);
+}
+
 static int
 check_time_rounding(int round)
 {
@@ -5998,6 +6058,9 @@ static PyMethodDef TestMethods[] = {
      METH_NOARGS},
     {"crash_no_current_thread", crash_no_current_thread,         METH_NOARGS},
     {"run_in_subinterp",        run_in_subinterp,                METH_VARARGS},
+    {"run_in_subinterp_with_config",
+     _PyCFunction_CAST(run_in_subinterp_with_config),
+     METH_VARARGS | METH_KEYWORDS},
     {"pytime_object_to_time_t", test_pytime_object_to_time_t,  METH_VARARGS},
     {"pytime_object_to_timeval", test_pytime_object_to_timeval,  METH_VARARGS},
     {"pytime_object_to_timespec", test_pytime_object_to_timespec,  METH_VARARGS},
diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
@@ -550,6 +550,51 @@ _testinternalcapi_optimize_cfg_impl(PyObject *module, PyObject *instructions,
 }
 
 
+static PyObject *
+get_interp_settings(PyObject *self, PyObject *args)
+{
+    int interpid = -1;
+    if (!PyArg_ParseTuple(args, "|i:get_interp_settings", &interpid)) {
+        return NULL;
+    }
+
+    PyInterpreterState *interp = NULL;
+    if (interpid < 0) {
+        PyThreadState *tstate = _PyThreadState_GET();
+        interp = tstate ? tstate->interp : _PyInterpreterState_Main();
+    }
+    else if (interpid == 0) {
+        interp = _PyInterpreterState_Main();
+    }
+    else {
+        PyErr_Format(PyExc_NotImplementedError,
+                     "%zd", interpid);
+        return NULL;
+    }
+    assert(interp != NULL);
+
+    PyObject *settings = PyDict_New();
+    if (settings == NULL) {
+        return NULL;
+    }
+
+    /* Add the feature flags. */
+    PyObject *flags = PyLong_FromUnsignedLong(interp->feature_flags);
+    if (flags == NULL) {
+        Py_DECREF(settings);
+        return NULL;
+    }
+    int res = PyDict_SetItemString(settings, "feature_flags", flags);
+    Py_DECREF(flags);
+    if (res != 0) {
+        Py_DECREF(settings);
+        return NULL;
+    }
+
+    return settings;
+}
+
+
 static PyMethodDef TestMethods[] = {
     {"get_configs", get_configs, METH_NOARGS},
     {"get_recursion_depth", get_recursion_depth, METH_NOARGS},
@@ -569,6 +614,7 @@ static PyMethodDef TestMethods[] = {
     {"set_eval_frame_default", set_eval_frame_default, METH_NOARGS, NULL},
     {"set_eval_frame_record", set_eval_frame_record, METH_O, NULL},
     _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF
+    {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL},
     {NULL, NULL} /* sentinel */
 };
 
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
@@ -1128,7 +1128,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
     }
 
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    if (interp->config._isolated_interpreter) {
+    if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_THREADS)) {
         PyErr_SetString(PyExc_RuntimeError,
                         "thread is not supported for isolated subinterpreters");
         return NULL;
diff --git a/Modules/_winapi.c b/Modules/_winapi.c
@@ -1090,8 +1090,7 @@ _winapi_CreateProcess_impl(PyObject *module,
     }
 
     PyInterpreterState *interp = PyInterpreterState_Get();
-    const PyConfig *config = _PyInterpreterState_GetConfig(interp);
-    if (config->_isolated_interpreter) {
+    if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_SUBPROCESS)) {
         PyErr_SetString(PyExc_RuntimeError,
                         "subprocess not supported for isolated subinterpreters");
         return NULL;
diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
diff --git a/Python/initconfig.c b/Python/initconfig.c
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
diff --git a/Python/pystate.c b/Python/pystate.c

Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,6 @@ def test_set_invalid(self):`
`84`	`84`	`'skip_source_first_line',`
`85`	`85`	`'_install_importlib',`
`86`	`86`	`'_init_main',`
`87`		`- '_isolated_interpreter',`
`88`	`87`	`]`
`89`	`88`	`if MS_WINDOWS:`
`90`	`89`	`options.append('legacy_windows_stdio')`