From 976bcb2379709da57073a9e07b518ff51daa617a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 3 Apr 2024 10:58:39 -0600 Subject: [PATCH] gh-76785: Raise InterpreterError, Not RuntimeError (gh-117489) I had meant to switch everything to InterpreterError when I added it a while back. At the time I missed a few key spots. As part of this, I've added print-the-exception to _PyXI_InitTypes() and fixed an error case in `_PyStaticType_InitBuiltin(). --- Lib/test/support/interpreters/__init__.py | 4 ++-- Lib/test/test__xxsubinterpreters.py | 12 ++++++------ Lib/test/test_interpreters/test_api.py | 10 +++++----- Modules/_xxsubinterpretersmodule.c | 8 ++++---- Objects/typeobject.c | 1 + Python/crossinterp.c | 9 +++++---- Python/crossinterp_exceptions.h | 17 ++++++++++++++++- Python/pystate.c | 2 +- 8 files changed, 40 insertions(+), 23 deletions(-) diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py index 8316b5e4a93..8be4ee736aa 100644 --- a/Lib/test/support/interpreters/__init__.py +++ b/Lib/test/support/interpreters/__init__.py @@ -43,7 +43,7 @@ Uncaught in the interpreter: {formatted} """.strip() -class ExecutionFailed(RuntimeError): +class ExecutionFailed(InterpreterError): """An unhandled exception happened during execution. This is raised from Interpreter.exec() and Interpreter.call(). @@ -158,7 +158,7 @@ class Interpreter: """Finalize and destroy the interpreter. Attempting to destroy the current interpreter results - in a RuntimeError. + in an InterpreterError. """ return _interpreters.destroy(self._id) diff --git a/Lib/test/test__xxsubinterpreters.py b/Lib/test/test__xxsubinterpreters.py index f674771c27c..841077adbb0 100644 --- a/Lib/test/test__xxsubinterpreters.py +++ b/Lib/test/test__xxsubinterpreters.py @@ -80,7 +80,7 @@ def clean_up_interpreters(): continue try: interpreters.destroy(id) - except RuntimeError: + except interpreters.InterpreterError: pass # already destroyed @@ -464,11 +464,11 @@ class DestroyTests(TestBase): def test_main(self): main, = interpreters.list_all() - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): interpreters.destroy(main) def f(): - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): interpreters.destroy(main) t = threading.Thread(target=f) @@ -496,7 +496,7 @@ class DestroyTests(TestBase): import _xxsubinterpreters as _interpreters try: _interpreters.destroy({id}) - except RuntimeError: + except interpreters.InterpreterError: pass """) @@ -531,7 +531,7 @@ class DestroyTests(TestBase): self.assertTrue(interpreters.is_running(interp), msg=f"Interp {interp} should be running before destruction.") - with self.assertRaises(RuntimeError, + with self.assertRaises(interpreters.InterpreterError, msg=f"Should not be able to destroy interp {interp} while it's still running."): interpreters.destroy(interp) self.assertTrue(interpreters.is_running(interp)) @@ -676,7 +676,7 @@ class RunStringTests(TestBase): def test_already_running(self): with _running(self.id): - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): interpreters.run_string(self.id, 'print("spam")') def test_does_not_exist(self): diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index 2aa7f9bb61a..a326b39fd23 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -364,11 +364,11 @@ class TestInterpreterClose(TestBase): def test_main(self): main, = interpreters.list_all() - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): main.close() def f(): - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): main.close() t = threading.Thread(target=f) @@ -389,7 +389,7 @@ class TestInterpreterClose(TestBase): interp = interpreters.Interpreter({interp.id}) try: interp.close() - except RuntimeError: + except interpreters.InterpreterError: print('failed') """)) self.assertEqual(out.strip(), 'failed') @@ -424,7 +424,7 @@ class TestInterpreterClose(TestBase): main, = interpreters.list_all() interp = interpreters.create() with _running(interp): - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): interp.close() self.assertTrue(interp.is_running()) @@ -1103,7 +1103,7 @@ class LowLevelTests(TestBase): self.assert_ns_equal(config, default) with self.subTest('arg: \'empty\''): - with self.assertRaises(RuntimeError): + with self.assertRaises(interpreters.InterpreterError): # The "empty" config isn't viable on its own. _interpreters.create('empty') diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 9c2774e4f82..94b8ee35001 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -612,7 +612,7 @@ interp_create(PyObject *self, PyObject *args, PyObject *kwds) // XXX Move the chained exception to interpreters.create()? PyObject *exc = PyErr_GetRaisedException(); assert(exc != NULL); - PyErr_SetString(PyExc_RuntimeError, "interpreter creation failed"); + PyErr_SetString(PyExc_InterpreterError, "interpreter creation failed"); _PyErr_ChainExceptions1(exc); return NULL; } @@ -664,7 +664,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } if (interp == current) { - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(PyExc_InterpreterError, "cannot destroy the current interpreter"); return NULL; } @@ -673,7 +673,7 @@ interp_destroy(PyObject *self, PyObject *args, PyObject *kwds) /* XXX We *could* support destroying a running interpreter but aren't going to worry about it for now. */ if (is_running_main(interp)) { - PyErr_Format(PyExc_RuntimeError, "interpreter running"); + PyErr_Format(PyExc_InterpreterError, "interpreter running"); return NULL; } @@ -693,7 +693,7 @@ PyDoc_STRVAR(destroy_doc, \n\ Destroy the identified interpreter.\n\ \n\ -Attempting to destroy the current interpreter results in a RuntimeError.\n\ +Attempting to destroy the current interpreter raises InterpreterError.\n\ So does an unrecognized ID."); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 6df9986b82e..51ceb7d7de1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -7963,6 +7963,7 @@ _PyStaticType_InitBuiltin(PyInterpreterState *interp, PyTypeObject *self) res = type_ready(self, !ismain); END_TYPE_LOCK() if (res < 0) { + _PyStaticType_ClearWeakRefs(interp, self); static_builtin_state_clear(interp, self); } return res; diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 18dec4dd959..16efe9c3958 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -845,7 +845,7 @@ _PyXI_ApplyErrorCode(_PyXI_errcode code, PyInterpreterState *interp) return 0; case _PyXI_ERR_OTHER: // XXX msg? - PyErr_SetNone(PyExc_RuntimeError); + PyErr_SetNone(PyExc_InterpreterError); break; case _PyXI_ERR_NO_MEMORY: PyErr_NoMemory(); @@ -856,11 +856,11 @@ _PyXI_ApplyErrorCode(_PyXI_errcode code, PyInterpreterState *interp) _PyInterpreterState_FailIfRunningMain(interp); break; case _PyXI_ERR_MAIN_NS_FAILURE: - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(PyExc_InterpreterError, "failed to get __main__ namespace"); break; case _PyXI_ERR_APPLY_NS_FAILURE: - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(PyExc_InterpreterError, "failed to apply namespace to __main__"); break; case _PyXI_ERR_NOT_SHAREABLE: @@ -935,7 +935,7 @@ _PyXI_ApplyError(_PyXI_error *error) if (error->uncaught.type.name != NULL || error->uncaught.msg != NULL) { // __context__ will be set to a proxy of the propagated exception. PyObject *exc = PyErr_GetRaisedException(); - _PyXI_excinfo_Apply(&error->uncaught, PyExc_RuntimeError); + _PyXI_excinfo_Apply(&error->uncaught, PyExc_InterpreterError); PyObject *exc2 = PyErr_GetRaisedException(); PyException_SetContext(exc, exc2); PyErr_SetRaisedException(exc); @@ -1671,6 +1671,7 @@ PyStatus _PyXI_InitTypes(PyInterpreterState *interp) { if (init_exceptions(interp) < 0) { + PyErr_PrintEx(0); return _PyStatus_ERR("failed to initialize an exception type"); } return _PyStatus_OK(); diff --git a/Python/crossinterp_exceptions.h b/Python/crossinterp_exceptions.h index e418cf91d4a..0f324bac48a 100644 --- a/Python/crossinterp_exceptions.h +++ b/Python/crossinterp_exceptions.h @@ -5,6 +5,9 @@ static PyTypeObject _PyExc_InterpreterError = { PyVarObject_HEAD_INIT(NULL, 0) .tp_name = "interpreters.InterpreterError", .tp_doc = PyDoc_STR("A cross-interpreter operation failed"), + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, + //.tp_traverse = ((PyTypeObject *)PyExc_BaseException)->tp_traverse, + //.tp_clear = ((PyTypeObject *)PyExc_BaseException)->tp_clear, //.tp_base = (PyTypeObject *)PyExc_BaseException, }; PyObject *PyExc_InterpreterError = (PyObject *)&_PyExc_InterpreterError; @@ -15,6 +18,9 @@ static PyTypeObject _PyExc_InterpreterNotFoundError = { PyVarObject_HEAD_INIT(NULL, 0) .tp_name = "interpreters.InterpreterNotFoundError", .tp_doc = PyDoc_STR("An interpreter was not found"), + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, + //.tp_traverse = ((PyTypeObject *)PyExc_BaseException)->tp_traverse, + //.tp_clear = ((PyTypeObject *)PyExc_BaseException)->tp_clear, .tp_base = &_PyExc_InterpreterError, }; PyObject *PyExc_InterpreterNotFoundError = (PyObject *)&_PyExc_InterpreterNotFoundError; @@ -55,16 +61,25 @@ _get_not_shareable_error_type(PyInterpreterState *interp) static int init_exceptions(PyInterpreterState *interp) { + PyTypeObject *base = (PyTypeObject *)PyExc_BaseException; + // builtin static types - _PyExc_InterpreterError.tp_base = (PyTypeObject *)PyExc_BaseException; + + _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; } + + _PyExc_InterpreterNotFoundError.tp_traverse = base->tp_traverse; + _PyExc_InterpreterNotFoundError.tp_clear = base->tp_clear; if (_PyStaticType_InitBuiltin(interp, &_PyExc_InterpreterNotFoundError) < 0) { return -1; } // heap types + // We would call _init_not_shareable_error_type() here too, // but that leads to ref leaks diff --git a/Python/pystate.c b/Python/pystate.c index 925d1cff866..892e740493c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1073,7 +1073,7 @@ int _PyInterpreterState_FailIfRunningMain(PyInterpreterState *interp) { if (interp->threads.main != NULL) { - PyErr_SetString(PyExc_RuntimeError, + PyErr_SetString(PyExc_InterpreterError, "interpreter already running"); return -1; }