From e45408d26224d58d8fcfe8537a29ab140c834c8d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Nov 2024 10:42:15 -0700 Subject: [PATCH 1/3] Clean up if init_exceptions() fails. --- Python/crossinterp_exceptions.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Python/crossinterp_exceptions.h b/Python/crossinterp_exceptions.h index 278511da615c75..f3f313d081a3df 100644 --- a/Python/crossinterp_exceptions.h +++ b/Python/crossinterp_exceptions.h @@ -61,29 +61,41 @@ _get_not_shareable_error_type(PyInterpreterState *interp) static int init_exceptions(PyInterpreterState *interp) { + // Initialize static types. + int base_initialized = 0; + int notfound_initialized = 0; PyTypeObject *base = (PyTypeObject *)PyExc_Exception; - // builtin static types - _PyExc_InterpreterError.tp_base = base; _PyExc_InterpreterError.tp_traverse = base->tp_traverse; _PyExc_InterpreterError.tp_clear = base->tp_clear; if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterError) < 0) { - return -1; + goto error; } + base_initialized = 1; _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { - return -1; + goto error; } + notfound_initialized = 1; - // heap types + // Initialize heap types. // We would call _init_not_shareable_error_type() here too, // but that leads to ref leaks return 0; + +error: + if (base_initialized) { + _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); + } + if (notfound_initialized) { + _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); + } + return -1; } static void From 7b665a68276254a50e3865fa6227c5161a70aaaa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Nov 2024 11:01:18 -0700 Subject: [PATCH 2/3] Initialize heap types in init_exceptions(). --- Python/crossinterp.c | 21 ++++---- Python/crossinterp_exceptions.h | 92 ++++++++++++++++++--------------- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 2daba99988c12a..bfbd21c4b4a7ab 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -17,10 +17,8 @@ /* exceptions */ /**************/ -static int init_exceptions(PyInterpreterState *); -static void fini_exceptions(PyInterpreterState *); -static int _init_not_shareable_error_type(PyInterpreterState *); -static void _fini_not_shareable_error_type(PyInterpreterState *); +static int init_exceptions(PyInterpreterState *, int); +static void fini_exceptions(PyInterpreterState *, int); static PyObject * _get_not_shareable_error_type(PyInterpreterState *); #include "crossinterp_exceptions.h" @@ -1783,8 +1781,11 @@ _PyXI_Init(PyInterpreterState *interp) xid_lookup_init(&_PyXI_GET_STATE(interp)->data_lookup); // Initialize exceptions (heap types). - if (_init_not_shareable_error_type(interp) < 0) { - return _PyStatus_ERR("failed to initialize NotShareableError"); + // We would initialize static types here too but that leads to ref leaks. + // Instead, we do it in _PyXI_InitTypes(). + if (init_exceptions(interp, 0) < 0) { + PyErr_PrintEx(0); + return _PyStatus_ERR("failed to initialize exceptions"); } return _PyStatus_OK(); @@ -1797,7 +1798,9 @@ void _PyXI_Fini(PyInterpreterState *interp) { // Finalize exceptions (heap types). - _fini_not_shareable_error_type(interp); + // We would finalize static types here too but that leads to ref leaks. + // Instead, we do it in _PyXI_FiniTypes(). + fini_exceptions(interp, 0); // Finalize the XID lookup state (e.g. registry). xid_lookup_fini(&_PyXI_GET_STATE(interp)->data_lookup); @@ -1809,7 +1812,7 @@ _PyXI_Fini(PyInterpreterState *interp) PyStatus _PyXI_InitTypes(PyInterpreterState *interp) { - if (init_exceptions(interp) < 0) { + if (init_exceptions(interp, 1) < 0) { PyErr_PrintEx(0); return _PyStatus_ERR("failed to initialize an exception type"); } @@ -1819,7 +1822,7 @@ _PyXI_InitTypes(PyInterpreterState *interp) void _PyXI_FiniTypes(PyInterpreterState *interp) { - fini_exceptions(interp); + fini_exceptions(interp, 1); } diff --git a/Python/crossinterp_exceptions.h b/Python/crossinterp_exceptions.h index f3f313d081a3df..b047e58f765dbe 100644 --- a/Python/crossinterp_exceptions.h +++ b/Python/crossinterp_exceptions.h @@ -25,29 +25,6 @@ static PyTypeObject _PyExc_InterpreterNotFoundError = { }; PyObject *PyExc_InterpreterNotFoundError = (PyObject *)&_PyExc_InterpreterNotFoundError; -/* NotShareableError extends ValueError */ - -static int -_init_not_shareable_error_type(PyInterpreterState *interp) -{ - const char *name = "interpreters.NotShareableError"; - PyObject *base = PyExc_ValueError; - PyObject *ns = NULL; - PyObject *exctype = PyErr_NewException(name, base, ns); - if (exctype == NULL) { - return -1; - } - - _PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError = exctype; - return 0; -} - -static void -_fini_not_shareable_error_type(PyInterpreterState *interp) -{ - Py_CLEAR(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError); -} - static PyObject * _get_not_shareable_error_type(PyInterpreterState *interp) { @@ -59,27 +36,44 @@ _get_not_shareable_error_type(PyInterpreterState *interp) /* lifecycle */ static int -init_exceptions(PyInterpreterState *interp) +init_exceptions(PyInterpreterState *interp, int statictypes) { + int heaptypes = (!statictypes); + // Initialize static types. int base_initialized = 0; int notfound_initialized = 0; - PyTypeObject *base = (PyTypeObject *)PyExc_Exception; - - _PyExc_InterpreterError.tp_base = base; - _PyExc_InterpreterError.tp_traverse = base->tp_traverse; - _PyExc_InterpreterError.tp_clear = base->tp_clear; - if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterError) < 0) { - goto error; + if (statictypes) { + PyTypeObject *base = (PyTypeObject *)PyExc_Exception; + + _PyExc_InterpreterError.tp_base = base; + _PyExc_InterpreterError.tp_traverse = base->tp_traverse; + _PyExc_InterpreterError.tp_clear = base->tp_clear; + if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterError) < 0) { + goto error; + } + base_initialized = 1; + + _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; + _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; + if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { + goto error; + } + notfound_initialized = 1; } - base_initialized = 1; - _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; - _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; - if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { - goto error; + // Initialize heap types. + if (heaptypes) { + /* NotShareableError extends ValueError */ + const char *name = "interpreters.NotShareableError"; + PyObject *base = PyExc_ValueError; + PyObject *ns = NULL; + PyObject *exctype = PyErr_NewException(name, base, ns); + if (exctype == NULL) { + goto error; + } + _PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError = exctype; } - notfound_initialized = 1; // Initialize heap types. @@ -89,19 +83,31 @@ init_exceptions(PyInterpreterState *interp) return 0; error: - if (base_initialized) { - _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); + if (heaptypes) { + Py_CLEAR(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError); } if (notfound_initialized) { _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); } + if (base_initialized) { + _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); + } return -1; } static void -fini_exceptions(PyInterpreterState *interp) +fini_exceptions(PyInterpreterState *interp, int statictypes) { - // Likewise with _fini_not_shareable_error_type(). - _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); - _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); + int heaptypes = (!statictypes); + + // Finalize heap types. + if (heaptypes) { + Py_CLEAR(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError); + } + + // Finalize static types. + if (statictypes) { + _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); + _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); + } } From e5e0a6cc04a98a5f72f4d41d077442ef631b914c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Nov 2024 13:35:44 -0700 Subject: [PATCH 3/3] Add _PyXI_state_t.exceptions, init_static_exctypes(), fini_static_exctypes(), etc. --- Include/internal/pycore_crossinterp.h | 10 ++- Modules/_interpretersmodule.c | 2 +- Python/crossinterp.c | 35 +++++--- Python/crossinterp_exceptions.h | 117 ++++++++++++-------------- 4 files changed, 83 insertions(+), 81 deletions(-) diff --git a/Include/internal/pycore_crossinterp.h b/Include/internal/pycore_crossinterp.h index e91e911feb38cc..a7e71efc5daa49 100644 --- a/Include/internal/pycore_crossinterp.h +++ b/Include/internal/pycore_crossinterp.h @@ -11,6 +11,7 @@ extern "C" { #include "pycore_lock.h" // PyMutex #include "pycore_pyerrors.h" + /**************/ /* exceptions */ /**************/ @@ -163,8 +164,13 @@ struct _xi_state { // heap types _PyXIData_lookup_t data_lookup; - // heap types - PyObject *PyExc_NotShareableError; + struct xi_exceptions { + // static types + PyObject *PyExc_InterpreterError; + PyObject *PyExc_InterpreterNotFoundError; + // heap types + PyObject *PyExc_NotShareableError; + } exceptions; }; extern PyStatus _PyXI_Init(PyInterpreterState *interp); diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index 95acdd69e53260..31930b1227f96a 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -1502,7 +1502,7 @@ module_exec(PyObject *mod) goto error; } PyObject *PyExc_NotShareableError = \ - _PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError; + _PyInterpreterState_GetXIState(interp)->exceptions.PyExc_NotShareableError; if (PyModule_AddType(mod, (PyTypeObject *)PyExc_NotShareableError) < 0) { goto error; } diff --git a/Python/crossinterp.c b/Python/crossinterp.c index bfbd21c4b4a7ab..b7aa8da8ac550e 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -17,9 +17,11 @@ /* exceptions */ /**************/ -static int init_exceptions(PyInterpreterState *, int); -static void fini_exceptions(PyInterpreterState *, int); -static PyObject * _get_not_shareable_error_type(PyInterpreterState *); +typedef struct xi_exceptions exceptions_t; +static int init_static_exctypes(exceptions_t *, PyInterpreterState *); +static void fini_static_exctypes(exceptions_t *, PyInterpreterState *); +static int init_heap_exctypes(exceptions_t *); +static void fini_heap_exctypes(exceptions_t *); #include "crossinterp_exceptions.h" @@ -203,7 +205,8 @@ static inline void _set_xid_lookup_failure(PyInterpreterState *interp, PyObject *obj, const char *msg) { - PyObject *exctype = _get_not_shareable_error_type(interp); + exceptions_t *state = &_PyInterpreterState_GetXIState(interp)->exceptions; + PyObject *exctype = state->PyExc_NotShareableError; assert(exctype != NULL); if (msg != NULL) { assert(obj == NULL); @@ -1603,7 +1606,9 @@ _propagate_not_shareable_error(_PyXI_session *session) return; } PyInterpreterState *interp = PyInterpreterState_Get(); - if (PyErr_ExceptionMatches(_get_not_shareable_error_type(interp))) { + exceptions_t *state = &_PyInterpreterState_GetXIState(interp)->exceptions; + assert(state->PyExc_NotShareableError != NULL); + if (PyErr_ExceptionMatches(state->PyExc_NotShareableError)) { // We want to propagate the exception directly. session->_error_override = _PyXI_ERR_NOT_SHAREABLE; session->error_override = &session->_error_override; @@ -1780,10 +1785,9 @@ _PyXI_Init(PyInterpreterState *interp) } xid_lookup_init(&_PyXI_GET_STATE(interp)->data_lookup); - // Initialize exceptions (heap types). - // We would initialize static types here too but that leads to ref leaks. - // Instead, we do it in _PyXI_InitTypes(). - if (init_exceptions(interp, 0) < 0) { + // Initialize exceptions.(heap types). + // See _PyXI_InitTypes() for the static types. + if (init_heap_exctypes(&_PyXI_GET_STATE(interp)->exceptions) < 0) { PyErr_PrintEx(0); return _PyStatus_ERR("failed to initialize exceptions"); } @@ -1798,9 +1802,8 @@ void _PyXI_Fini(PyInterpreterState *interp) { // Finalize exceptions (heap types). - // We would finalize static types here too but that leads to ref leaks. - // Instead, we do it in _PyXI_FiniTypes(). - fini_exceptions(interp, 0); + // See _PyXI_FiniTypes() for the static types. + fini_heap_exctypes(&_PyXI_GET_STATE(interp)->exceptions); // Finalize the XID lookup state (e.g. registry). xid_lookup_fini(&_PyXI_GET_STATE(interp)->data_lookup); @@ -1812,17 +1815,21 @@ _PyXI_Fini(PyInterpreterState *interp) PyStatus _PyXI_InitTypes(PyInterpreterState *interp) { - if (init_exceptions(interp, 1) < 0) { + if (init_static_exctypes(&_PyXI_GET_STATE(interp)->exceptions, interp) < 0) { PyErr_PrintEx(0); return _PyStatus_ERR("failed to initialize an exception type"); } + // We would initialize heap types here too but that leads to ref leaks. + // Instead, we intialize them in _PyXI_Init(). return _PyStatus_OK(); } void _PyXI_FiniTypes(PyInterpreterState *interp) { - fini_exceptions(interp, 1); + // We would finalize heap types here too but that leads to ref leaks. + // Instead, we finalize them in _PyXI_Fini(). + fini_static_exctypes(&_PyXI_GET_STATE(interp)->exceptions, interp); } diff --git a/Python/crossinterp_exceptions.h b/Python/crossinterp_exceptions.h index b047e58f765dbe..3cb45d2067710b 100644 --- a/Python/crossinterp_exceptions.h +++ b/Python/crossinterp_exceptions.h @@ -25,89 +25,78 @@ static PyTypeObject _PyExc_InterpreterNotFoundError = { }; PyObject *PyExc_InterpreterNotFoundError = (PyObject *)&_PyExc_InterpreterNotFoundError; -static PyObject * -_get_not_shareable_error_type(PyInterpreterState *interp) -{ - assert(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError != NULL); - return _PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError; -} - /* lifecycle */ static int -init_exceptions(PyInterpreterState *interp, int statictypes) +init_static_exctypes(exceptions_t *state, PyInterpreterState *interp) { - int heaptypes = (!statictypes); - - // Initialize static types. - int base_initialized = 0; - int notfound_initialized = 0; - if (statictypes) { - PyTypeObject *base = (PyTypeObject *)PyExc_Exception; - - _PyExc_InterpreterError.tp_base = base; - _PyExc_InterpreterError.tp_traverse = base->tp_traverse; - _PyExc_InterpreterError.tp_clear = base->tp_clear; - if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterError) < 0) { - goto error; - } - base_initialized = 1; - - _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; - _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; - if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { - goto error; - } - notfound_initialized = 1; + assert(state == &_PyXI_GET_STATE(interp)->exceptions); + PyTypeObject *base = (PyTypeObject *)PyExc_Exception; + + // PyExc_InterpreterError + _PyExc_InterpreterError.tp_base = base; + _PyExc_InterpreterError.tp_traverse = base->tp_traverse; + _PyExc_InterpreterError.tp_clear = base->tp_clear; + if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterError) < 0) { + goto error; } + state->PyExc_InterpreterError = (PyObject *)&_PyExc_InterpreterError; - // Initialize heap types. - if (heaptypes) { - /* NotShareableError extends ValueError */ - const char *name = "interpreters.NotShareableError"; - PyObject *base = PyExc_ValueError; - PyObject *ns = NULL; - PyObject *exctype = PyErr_NewException(name, base, ns); - if (exctype == NULL) { - goto error; - } - _PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError = exctype; + // PyExc_InterpreterNotFoundError + _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; + _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; + if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { + goto error; } - - // Initialize heap types. - - // We would call _init_not_shareable_error_type() here too, - // but that leads to ref leaks + state->PyExc_InterpreterNotFoundError = + (PyObject *)&_PyExc_InterpreterNotFoundError; return 0; error: - if (heaptypes) { - Py_CLEAR(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError); - } - if (notfound_initialized) { + fini_static_exctypes(state, interp); + return -1; +} + +static void +fini_static_exctypes(exceptions_t *state, PyInterpreterState *interp) +{ + assert(state == &_PyXI_GET_STATE(interp)->exceptions); + if (state->PyExc_InterpreterNotFoundError != NULL) { + state->PyExc_InterpreterNotFoundError = NULL; _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); } - if (base_initialized) { + if (state->PyExc_InterpreterError != NULL) { + state->PyExc_InterpreterError = NULL; _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); } - return -1; } -static void -fini_exceptions(PyInterpreterState *interp, int statictypes) +static int +init_heap_exctypes(exceptions_t *state) { - int heaptypes = (!statictypes); - - // Finalize heap types. - if (heaptypes) { - Py_CLEAR(_PyInterpreterState_GetXIState(interp)->PyExc_NotShareableError); + PyObject *exctype; + + /* NotShareableError extends ValueError */ + const char *name = "interpreters.NotShareableError"; + PyObject *base = PyExc_ValueError; + PyObject *ns = NULL; + exctype = PyErr_NewException(name, base, ns); + if (exctype == NULL) { + goto error; } + state->PyExc_NotShareableError = exctype; - // Finalize static types. - if (statictypes) { - _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterNotFoundError); - _PyStaticType_FiniBuiltin(interp, &_PyExc_InterpreterError); - } + return 0; + +error: + fini_heap_exctypes(state); + return -1; +} + +static void +fini_heap_exctypes(exceptions_t *state) +{ + Py_CLEAR(state->PyExc_NotShareableError); }