From bc2bb7b0238a486d727e619bbb67c8a55d259253 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 30 Aug 2023 04:57:50 +0200 Subject: [PATCH] gh-108634: PyInterpreterState_New() no longer calls Py_FatalError() PyInterpreterState_New() now raises a RuntimeError instead of calling Py_FatalError() on init_interpreter() failure. And pycore_create_interpreter() now reports status exception to its caller rather than calling Py_FatalError(). * Add _PyInterpreterState_New() function. * Replace Py_FatalError() with PyStatus in init_interpreter() and _PyObject_InitState(). * _PyErr_SetFromPyStatus() now raises RuntimeError, instead of ValueError. It can no call PyErr_NoMemory() if it detects _PyStatus_NO_MEMORY(). * init_runtime() cannot be called with _initialized=1: add an assertion in the caller instead. --- Include/internal/pycore_initconfig.h | 7 +- Include/internal/pycore_interp.h | 4 ++ Include/internal/pycore_object.h | 2 +- Objects/object.c | 5 +- Python/initconfig.c | 27 ++++++-- Python/pylifecycle.c | 8 ++- Python/pystate.c | 97 ++++++++++++++++++---------- 7 files changed, 99 insertions(+), 51 deletions(-) diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h index 6439101ff390ae4..c86988234f6a050 100644 --- a/Include/internal/pycore_initconfig.h +++ b/Include/internal/pycore_initconfig.h @@ -22,7 +22,7 @@ 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){ \ @@ -30,7 +30,8 @@ struct pyruntimestate; .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 index 9a75b002e5bc1e0..f171c546efd53c9 100644 --- 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 index d842816e6735534..daa06ebfbf91a48 100644 --- 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 index a4d7111a686bda2..7aeda50e9b27574 100644 --- 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 index 3281b3c6aea45fc..f9c5c64f9c87591 100644 --- 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 index ee5d4981da51c8e..64c74f433f222c8 100644 --- 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 index 4a8808f700eaa22..50d33d5f53bd1e3 100644 --- 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; }