pythongh-108634: init_interpreter() returns Py_Status

* Replace Py_FatalError() with PyStatus in init_interpreter() and _PyObject_InitState(). * PyInterpreterState_New() falls Py_FatalError() if init_interpreter() failed. * init_runtime() cannot be called with _initialized=1: add an assertion in the caller. * _PyErr_SetFromPyStatus() now raises RuntimeError, instead of ValueError. It can no call PyErr_NoMemory() if it detects _PyStatus_NO_MEMORY().
vstinner · Sep 1, 2023 · 905c4c4 · 905c4c4
1 parent 3edcf74
commit 905c4c4
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 51 deletions.
diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h
@@ -22,15 +22,16 @@ struct pyruntimestate;
 #endif
 
 #define _PyStatus_OK() \
-    (PyStatus){._type = _PyStatus_TYPE_OK,}
+    (PyStatus){._type = _PyStatus_TYPE_OK}
     /* other fields are set to 0 */
 #define _PyStatus_ERR(ERR_MSG) \
     (PyStatus){ \
         ._type = _PyStatus_TYPE_ERROR, \
         .func = _PyStatus_GET_FUNC(), \
         .err_msg = (ERR_MSG)}
         /* other fields are set to 0 */
-#define _PyStatus_NO_MEMORY() _PyStatus_ERR("memory allocation failed")
+#define _PyStatus_NO_MEMORY_ERRMSG "memory allocation failed"
+#define _PyStatus_NO_MEMORY() _PyStatus_ERR(_PyStatus_NO_MEMORY_ERRMSG)
 #define _PyStatus_EXIT(EXITCODE) \
     (PyStatus){ \
         ._type = _PyStatus_TYPE_EXIT, \
@@ -45,7 +46,7 @@ struct pyruntimestate;
     do { (err).func = _PyStatus_GET_FUNC(); } while (0)
 
 // Export for '_testinternalcapi' shared extension
-PyAPI_FUNC(PyObject *) _PyErr_SetFromPyStatus(PyStatus status);
+PyAPI_FUNC(void) _PyErr_SetFromPyStatus(PyStatus status);
 
 
 /* --- PyWideStringList ------------------------------------------------ */

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -311,6 +311,10 @@ might not be allowed in the current interpreter (i.e. os.fork() would fail).
 extern int _PyInterpreterState_HasFeature(PyInterpreterState *interp,
                                           unsigned long feature);
 
+PyAPI_FUNC(PyStatus) _PyInterpreterState_New(
+    PyThreadState *tstate,
+    PyInterpreterState **pinterp);
+
 
 #ifdef __cplusplus
 }

diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
@@ -182,7 +182,7 @@ _PyType_HasFeature(PyTypeObject *type, unsigned long feature) {
 
 extern void _PyType_InitCache(PyInterpreterState *interp);
 
-extern void _PyObject_InitState(PyInterpreterState *interp);
+extern PyStatus _PyObject_InitState(PyInterpreterState *interp);
 extern void _PyObject_FiniState(PyInterpreterState *interp);
 extern bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj);
 

diff --git a/Objects/object.c b/Objects/object.c
@@ -2034,7 +2034,7 @@ PyObject _Py_NotImplementedStruct = {
 };
 
 
-void
+PyStatus
 _PyObject_InitState(PyInterpreterState *interp)
 {
 #ifdef Py_TRACE_REFS
@@ -2048,9 +2048,10 @@ _PyObject_InitState(PyInterpreterState *interp)
         _Py_hashtable_hash_ptr, _Py_hashtable_compare_direct,
         NULL, NULL, &alloc);
     if (REFCHAIN(interp) == NULL) {
-        Py_FatalError("_PyObject_InitState() memory allocation failure");
+        return _PyStatus_NO_MEMORY();
     }
 #endif
+    return _PyStatus_OK();
 }
 
 void

diff --git a/Python/initconfig.c b/Python/initconfig.c
@@ -335,21 +335,34 @@ int PyStatus_IsExit(PyStatus status)
 int PyStatus_Exception(PyStatus status)
 { return _PyStatus_EXCEPTION(status); }
 
-PyObject*
+void
 _PyErr_SetFromPyStatus(PyStatus status)
 {
     if (!_PyStatus_IS_ERROR(status)) {
         PyErr_Format(PyExc_SystemError,
-                     "%s() expects an error PyStatus",
-                     _PyStatus_GET_FUNC());
+                     "_PyErr_SetFromPyStatus() status is not an error");
+        return;
     }
-    else if (status.func) {
-        PyErr_Format(PyExc_ValueError, "%s: %s", status.func, status.err_msg);
+
+    const char *err_msg = status.err_msg;
+    if (err_msg == NULL || strlen(err_msg) == 0) {
+        PyErr_Format(PyExc_SystemError,
+                     "_PyErr_SetFromPyStatus() status has no error message");
+        return;
+    }
+
+    if (strcmp(err_msg, _PyStatus_NO_MEMORY_ERRMSG) == 0) {
+        PyErr_NoMemory();
+        return;
+    }
+
+    const char *func = status.func;
+    if (func) {
+        PyErr_Format(PyExc_RuntimeError, "%s: %s", func, err_msg);
     }
     else {
-        PyErr_Format(PyExc_ValueError, "%s", status.err_msg);
+        PyErr_Format(PyExc_RuntimeError, "%s", err_msg);
     }
-    return NULL;
 }
 
 

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
@@ -629,10 +629,12 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
                           PyThreadState **tstate_p)
 {
     PyStatus status;
-    PyInterpreterState *interp = PyInterpreterState_New();
-    if (interp == NULL) {
-        return _PyStatus_ERR("can't make main interpreter");
+    PyInterpreterState *interp;
+    status = _PyInterpreterState_New(NULL, &interp);
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
     }
+    assert(interp != NULL);
     assert(_Py_IsMainInterpreter(interp));
 
     status = _PyConfig_Copy(&interp->config, src_config);

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -426,13 +426,11 @@ init_runtime(_PyRuntimeState *runtime,
              Py_ssize_t unicode_next_index,
              PyThread_type_lock locks[NUMLOCKS])
 {
-    if (runtime->_initialized) {
-        Py_FatalError("runtime already initialized");
-    }
-    assert(!runtime->preinitializing &&
-           !runtime->preinitialized &&
-           !runtime->core_initialized &&
-           !runtime->initialized);
+    assert(!runtime->preinitializing);
+    assert(!runtime->preinitialized);
+    assert(!runtime->core_initialized);
+    assert(!runtime->initialized);
+    assert(!runtime->_initialized);
 
     runtime->open_code_hook = open_code_hook;
     runtime->open_code_userdata = open_code_userdata;
@@ -476,6 +474,7 @@ _PyRuntimeState_Init(_PyRuntimeState *runtime)
         // Py_Initialize() must be running again.
         // Reset to _PyRuntimeState_INIT.
         memcpy(runtime, &initial, sizeof(*runtime));
+        assert(!runtime->_initialized);
     }
 
     if (gilstate_tss_init(runtime) != 0) {
@@ -647,14 +646,14 @@ free_interpreter(PyInterpreterState *interp)
    main interpreter.  We fix those fields here, in addition
    to the other dynamically initialized fields.
   */
-static void
+static PyStatus
 init_interpreter(PyInterpreterState *interp,
                  _PyRuntimeState *runtime, int64_t id,
                  PyInterpreterState *next,
                  PyThread_type_lock pending_lock)
 {
     if (interp->_initialized) {
-        Py_FatalError("interpreter already initialized");
+        return _PyStatus_ERR("interpreter already initialized");
     }
 
     assert(runtime != NULL);
@@ -675,7 +674,10 @@ init_interpreter(PyInterpreterState *interp,
         memcpy(&interp->obmalloc.pools.used, temp, sizeof(temp));
     }
 
-    _PyObject_InitState(interp);
+    PyStatus status = _PyObject_InitState(interp);
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
+    }
 
     _PyEval_InitState(interp, pending_lock);
     _PyGC_InitState(&interp->gc);
@@ -701,42 +703,44 @@ init_interpreter(PyInterpreterState *interp,
     }
     interp->f_opcode_trace_set = false;
     interp->_initialized = 1;
+    return _PyStatus_OK();
 }
 
-PyInterpreterState *
-PyInterpreterState_New(void)
+
+PyStatus
+_PyInterpreterState_New(PyThreadState *tstate, PyInterpreterState **pinterp)
 {
-    PyInterpreterState *interp;
+    *pinterp = NULL;
+
+    // Don't get runtime from tstate since tstate can be NULL
     _PyRuntimeState *runtime = &_PyRuntime;
-    PyThreadState *tstate = current_fast_get(runtime);
 
-    /* tstate is NULL when Py_InitializeFromConfig() calls
-       PyInterpreterState_New() to create the main interpreter. */
-    if (_PySys_Audit(tstate, "cpython.PyInterpreterState_New", NULL) < 0) {
-        return NULL;
+    // tstate is NULL when pycore_create_interpreter() calls
+    // _PyInterpreterState_New() to create the main interpreter.
+    if (tstate != NULL) {
+        if (_PySys_Audit(tstate, "cpython.PyInterpreterState_New", NULL) < 0) {
+            return _PyStatus_ERR("sys.audit failed");
+        }
     }
 
     PyThread_type_lock pending_lock = PyThread_allocate_lock();
     if (pending_lock == NULL) {
-        if (tstate != NULL) {
-            _PyErr_NoMemory(tstate);
-        }
-        return NULL;
+        return _PyStatus_NO_MEMORY();
     }
 
-    /* Don't get runtime from tstate since tstate can be NULL. */
-    struct pyinterpreters *interpreters = &runtime->interpreters;
-
     /* We completely serialize creation of multiple interpreters, since
        it simplifies things here and blocking concurrent calls isn't a problem.
        Regardless, we must fully block subinterpreter creation until
        after the main interpreter is created. */
     HEAD_LOCK(runtime);
 
+    struct pyinterpreters *interpreters = &runtime->interpreters;
     int64_t id = interpreters->next_id;
     interpreters->next_id += 1;
 
     // Allocate the interpreter and add it to the runtime state.
+    PyInterpreterState *interp;
+    PyStatus status;
     PyInterpreterState *old_head = interpreters->head;
     if (old_head == NULL) {
         // We are creating the main interpreter.
@@ -755,36 +759,59 @@ PyInterpreterState_New(void)
 
         interp = alloc_interpreter();
         if (interp == NULL) {
+            status = _PyStatus_NO_MEMORY();
             goto error;
         }
         // Set to _PyInterpreterState_INIT.
-        memcpy(interp, &initial._main_interpreter,
-               sizeof(*interp));
+        memcpy(interp, &initial._main_interpreter, sizeof(*interp));
 
         if (id < 0) {
             /* overflow or Py_Initialize() not called yet! */
-            if (tstate != NULL) {
-                _PyErr_SetString(tstate, PyExc_RuntimeError,
-                                 "failed to get an interpreter ID");
-            }
+            status = _PyStatus_ERR("failed to get an interpreter ID");
             goto error;
         }
     }
     interpreters->head = interp;
 
-    init_interpreter(interp, runtime, id, old_head, pending_lock);
+    status = init_interpreter(interp, runtime,
+                              id, old_head, pending_lock);
+    if (_PyStatus_EXCEPTION(status)) {
+        goto error;
+    }
+    pending_lock = NULL;
 
     HEAD_UNLOCK(runtime);
-    return interp;
+
+    assert(interp != NULL);
+    *pinterp = interp;
+    return _PyStatus_OK();
 
 error:
     HEAD_UNLOCK(runtime);
 
-    PyThread_free_lock(pending_lock);
+    if (pending_lock != NULL) {
+        PyThread_free_lock(pending_lock);
+    }
     if (interp != NULL) {
         free_interpreter(interp);
     }
-    return NULL;
+    return status;
+}
+
+
+PyInterpreterState *
+PyInterpreterState_New(void)
+{
+    // PyThreadState_Get() cannot return NULL
+    PyThreadState *tstate = PyThreadState_Get();
+
+    PyInterpreterState *interp;
+    PyStatus status = _PyInterpreterState_New(tstate, &interp);
+    if (_PyStatus_EXCEPTION(status)) {
+        _PyErr_SetFromPyStatus(status);
+    }
+    assert(interp != NULL);
+    return interp;
 }