Factor out new_interpreter().

python · ericsnowcurrently · Apr 2, 2024 · Mar 7, 2024 · Mar 21, 2024 · Mar 21, 2024
commit 1173cd1cbb77b8abfe8d6d2079b9523788ec11c3
@@ -2327,7 +2327,7 @@ def test_linked_lifecycle_initial(self):
         get_refcount = _testinternalcapi.get_interpreter_refcount
 
         # A new interpreter will start out not linked, with a refcount of 0.
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
         linked = is_linked(interpid)
         refcount = get_refcount(interpid)
@@ -2342,7 +2342,7 @@ def test_linked_lifecycle_never_linked(self):
         incref = (lambda id: _interpreters._incref(id, implieslink=False))
         decref = _interpreters._decref
 
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
 
         # Incref will not automatically link it.
@@ -2367,7 +2367,7 @@ def test_linked_lifecycle_link_unlink(self):
         link = _testinternalcapi.link_interpreter_refcount
         unlink = _testinternalcapi.unlink_interpreter_refcount
 
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
 
         # Linking at refcount 0 does not destroy the interpreter.
@@ -2392,7 +2392,7 @@ def test_linked_lifecycle_link_incref_decref(self):
         incref = (lambda id: _interpreters._incref(id, implieslink=False))
         decref = _interpreters._decref
 
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
 
         # Linking it will not change the refcount.
@@ -2418,7 +2418,7 @@ def test_linked_lifecycle_incref_link(self):
         get_refcount = _testinternalcapi.get_interpreter_refcount
         incref = (lambda id: _interpreters._incref(id, implieslink=False))
 
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
 
         incref(interpid)
@@ -2441,7 +2441,7 @@ def test_linked_lifecycle_link_incref_unlink_decref(self):
         incref = (lambda id: _interpreters._incref(id, implieslink=False))
         decref = _interpreters._decref
 
-        interpid = _testinternalcapi.new_interpreter()
+        interpid = self.new_interpreter()
         self.add_interp_cleanup(interpid)
 
         link(interpid)

diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
@@ -1451,50 +1451,6 @@ unused_interpreter_id(PyObject *self, PyObject *Py_UNUSED(ignored))
     return PyLong_FromLongLong(interpid);
 }
 
-static PyObject *
-new_interpreter(PyObject *self, PyObject *Py_UNUSED(ignored))
-{
-    // Unlike _interpreters.create(), we do not automatically link
-    // the interpreter to its refcount.
-    PyThreadState *save_tstate = PyThreadState_Get();
-    const PyInterpreterConfig config = \
-            (PyInterpreterConfig)_PyInterpreterConfig_INIT;
-    PyThreadState *tstate = NULL;
-    PyStatus status = Py_NewInterpreterFromConfig(&tstate, &config);
-    PyThreadState_Swap(save_tstate);
-    if (PyStatus_Exception(status)) {
-        _PyErr_SetFromPyStatus(status);
-        return NULL;
-    }
-    PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate);
-
-    if (_PyInterpreterState_IDInitref(interp) < 0) {
-        goto error;
-    }
-
-    int64_t interpid = PyInterpreterState_GetID(interp);
-    if (interpid < 0) {
-        goto error;
-    }
-    PyObject *idobj = PyLong_FromLongLong(interpid);
-    if (idobj == NULL) {
-        goto error;
-    }
-
-    PyThreadState_Swap(tstate);
-    PyThreadState_Clear(tstate);
-    PyThreadState_Swap(save_tstate);
-    PyThreadState_Delete(tstate);
-
-    return idobj;
-
-error:
-    save_tstate = PyThreadState_Swap(tstate);
-    Py_EndInterpreter(tstate);
-    PyThreadState_Swap(save_tstate);
-    return NULL;
-}
-
 static PyObject *
 interpreter_exists(PyObject *self, PyObject *idobj)
 {
@@ -1799,7 +1755,6 @@ static PyMethodDef module_functions[] = {
      METH_VARARGS | METH_KEYWORDS},
     {"normalize_interp_id", normalize_interp_id, METH_O},
     {"unused_interpreter_id", unused_interpreter_id, METH_NOARGS},
-    {"new_interpreter", new_interpreter, METH_NOARGS},
     {"interpreter_exists", interpreter_exists, METH_O},
     {"get_interpreter_refcount", get_interpreter_refcount, METH_O},
     {"link_interpreter_refcount", link_interpreter_refcount,     METH_O},

diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c
@@ -407,6 +407,59 @@ config_from_object(PyObject *configobj, PyInterpreterConfig *config)
 }
 
 
+static PyInterpreterState *
+new_interpreter(PyInterpreterConfig *config, PyObject **p_idobj,  PyThreadState **p_tstate)
+{
+    PyThreadState *save_tstate = PyThreadState_Get();
+    assert(save_tstate != NULL);
+    PyThreadState *tstate = NULL;
+    // XXX Possible GILState issues?
+    PyStatus status = Py_NewInterpreterFromConfig(&tstate, config);
+    PyThreadState_Swap(save_tstate);
+    if (PyStatus_Exception(status)) {
+        /* Since no new thread state was created, there is no exception to
+           propagate; raise a fresh one after swapping in the old thread
+           state. */
+        _PyErr_SetFromPyStatus(status);
+        return NULL;
+    }
+    assert(tstate != NULL);
+    PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate);
+
+    if (_PyInterpreterState_IDInitref(interp) < 0) {
+        goto error;
+    }
+
+    if (p_idobj != NULL) {
+        // We create the object using the original interpreter.
+        PyObject *idobj = get_interpid_obj(interp);
+        if (idobj == NULL) {
+            goto error;
+        }
+        *p_idobj = idobj;
+    }
+
+    if (p_tstate != NULL) {
+        *p_tstate = tstate;
+    }
+    else {
+        PyThreadState_Swap(tstate);
+        PyThreadState_Clear(tstate);
+        PyThreadState_Swap(save_tstate);
+        PyThreadState_Delete(tstate);
+    }
+
+    return interp;
+
+error:
+    // XXX Possible GILState issues?
+    save_tstate = PyThreadState_Swap(tstate);
+    Py_EndInterpreter(tstate);
+    PyThreadState_Swap(save_tstate);
+    return NULL;
+}
+
+
 static int
 _run_script(PyObject *ns, const char *codestr, Py_ssize_t codestrlen, int flags)
 {
@@ -534,43 +587,21 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    // Create and initialize the new interpreter.
-    PyThreadState *save_tstate = PyThreadState_Get();
-    assert(save_tstate != NULL);
-    // XXX Possible GILState issues?
-    PyThreadState *tstate = NULL;
-    PyStatus status = Py_NewInterpreterFromConfig(&tstate, &config);
-    PyThreadState_Swap(save_tstate);
-    if (PyStatus_Exception(status)) {
-        /* Since no new thread state was created, there is no exception to
-           propagate; raise a fresh one after swapping in the old thread
-           state. */
-        _PyErr_SetFromPyStatus(status);
+    PyObject *idobj = NULL;
+    PyInterpreterState *interp = new_interpreter(&config, &idobj, NULL);
+    if (interp == NULL) {
+        // XXX Move the chained exception to interpreters.create()?
         PyObject *exc = PyErr_GetRaisedException();
+        assert(exc != NULL);
         PyErr_SetString(PyExc_RuntimeError, "interpreter creation failed");
         _PyErr_ChainExceptions1(exc);
         return NULL;
     }
-    assert(tstate != NULL);
-
-    PyInterpreterState *interp = PyThreadState_GetInterpreter(tstate);
-    PyObject *idobj = get_interpid_obj(interp);
-    if (idobj == NULL) {
-        // XXX Possible GILState issues?
-        save_tstate = PyThreadState_Swap(tstate);
-        Py_EndInterpreter(tstate);
-        PyThreadState_Swap(save_tstate);
-        return NULL;
-    }
-
-    PyThreadState_Swap(tstate);
-    PyThreadState_Clear(tstate);
-    PyThreadState_Swap(save_tstate);
-    PyThreadState_Delete(tstate);
 
     return idobj;
 }
 
+
 PyDoc_STRVAR(create_doc,
 "create() -> ID\n\
 \n\