pythongh-76785: Avoid Pickled TracebackException for Propagated Subinterpreter Exceptions (pythongh-113036)

We need the TracebackException of uncaught exceptions for a single purpose: the error display.  Thus we only need to pass the formatted error display between interpreters.  Passing a pickled TracebackException is overkill.

Author: ericsnowcurrently

Include/internal/pycore_crossinterp.h

@@ -188,8 +188,7 @@ typedef struct _excinfo {
         const char *module;
     } type;
     const char *msg;
-    const char *pickled;
-    Py_ssize_t pickled_len;
+    const char *errdisplay;
 } _PyXI_excinfo;
 
 

Lib/test/support/interpreters/__init__.py

@@ -56,7 +56,7 @@ def __init__(self, excinfo):
 
     def __str__(self):
         try:
-            formatted = ''.join(self.excinfo.tbexc.format()).rstrip()
+            formatted = self.excinfo.errdisplay
         except Exception:
             return super().__str__()
         else:

Python/crossinterp.c

@@ -945,113 +945,105 @@ _xidregistry_fini(struct _xidregistry *registry)
 /*************************/
 
 static const char *
-_copy_raw_string(const char *str, Py_ssize_t len)
+_copy_string_obj_raw(PyObject *strobj, Py_ssize_t *p_size)
 {
-    size_t size = len + 1;
-    if (len <= 0) {
-        size = strlen(str) + 1;
-    }
-    char *copied = PyMem_RawMalloc(size);
-    if (copied == NULL) {
-        return NULL;
-    }
-    if (len <= 0) {
-        strcpy(copied, str);
-    }
-    else {
-        memcpy(copied, str, size);
-    }
-    return copied;
-}
-
-static const char *
-_copy_string_obj_raw(PyObject *strobj)
-{
-    const char *str = PyUnicode_AsUTF8(strobj);
+    Py_ssize_t size = -1;
+    const char *str = PyUnicode_AsUTF8AndSize(strobj, &size);
     if (str == NULL) {
         return NULL;
     }
 
-    char *copied = PyMem_RawMalloc(strlen(str)+1);
+    char *copied = PyMem_RawMalloc(size+1);
     if (copied == NULL) {
         PyErr_NoMemory();
         return NULL;
     }
     strcpy(copied, str);
+    if (p_size != NULL) {
+        *p_size = size;
+    }
     return copied;
 }
 
 
 static int
-_pickle_object(PyObject *obj, const char **p_pickled, Py_ssize_t *p_len)
+_convert_exc_to_TracebackException(PyObject *exc, PyObject **p_tbexc)
 {
-    assert(!PyErr_Occurred());
-    PyObject *picklemod = PyImport_ImportModule("_pickle");
-    if (picklemod == NULL) {
-        PyErr_Clear();
-        picklemod = PyImport_ImportModule("pickle");
-        if (picklemod == NULL) {
-            return -1;
-        }
+    PyObject *args = NULL;
+    PyObject *kwargs = NULL;
+    PyObject *create = NULL;
+
+    // This is inspired by _PyErr_Display().
+    PyObject *tbmod = PyImport_ImportModule("traceback");
+    if (tbmod == NULL) {
+        return -1;
     }
-    PyObject *dumps = PyObject_GetAttrString(picklemod, "dumps");
-    Py_DECREF(picklemod);
-    if (dumps == NULL) {
+    PyObject *tbexc_type = PyObject_GetAttrString(tbmod, "TracebackException");
+    Py_DECREF(tbmod);
+    if (tbexc_type == NULL) {
         return -1;
     }
-    PyObject *pickledobj = PyObject_CallOneArg(dumps, obj);
-    Py_DECREF(dumps);
-    if (pickledobj == NULL) {
+    create = PyObject_GetAttrString(tbexc_type, "from_exception");
+    Py_DECREF(tbexc_type);
+    if (create == NULL) {
         return -1;
     }
 
-    char *pickled = NULL;
-    Py_ssize_t len = 0;
-    if (PyBytes_AsStringAndSize(pickledobj, &pickled, &len) < 0) {
-        Py_DECREF(pickledobj);
-        return -1;
+    args = PyTuple_Pack(1, exc);
+    if (args == NULL) {
+        goto error;
     }
-    const char *copied = _copy_raw_string(pickled, len);
-    Py_DECREF(pickledobj);
-    if (copied == NULL) {
-        return -1;
+
+    kwargs = PyDict_New();
+    if (kwargs == NULL) {
+        goto error;
+    }
+    if (PyDict_SetItemString(kwargs, "save_exc_type", Py_False) < 0) {
+        goto error;
+    }
+    if (PyDict_SetItemString(kwargs, "lookup_lines", Py_False) < 0) {
+        goto error;
+    }
+
+    PyObject *tbexc = PyObject_Call(create, args, kwargs);
+    Py_DECREF(args);
+    Py_DECREF(kwargs);
+    Py_DECREF(create);
+    if (tbexc == NULL) {
+        goto error;
     }
 
-    *p_pickled = copied;
-    *p_len = len;
+    *p_tbexc = tbexc;
     return 0;
+
+error:
+    Py_XDECREF(args);
+    Py_XDECREF(kwargs);
+    Py_XDECREF(create);
+    return -1;
 }
 
-static int
-_unpickle_object(const char *pickled, Py_ssize_t size, PyObject **p_obj)
+
+static const char *
+_format_TracebackException(PyObject *tbexc)
 {
-    assert(!PyErr_Occurred());
-    PyObject *picklemod = PyImport_ImportModule("_pickle");
-    if (picklemod == NULL) {
-        PyErr_Clear();
-        picklemod = PyImport_ImportModule("pickle");
-        if (picklemod == NULL) {
-            return -1;
-        }
-    }
-    PyObject *loads = PyObject_GetAttrString(picklemod, "loads");
-    Py_DECREF(picklemod);
-    if (loads == NULL) {
-        return -1;
-    }
-    PyObject *pickledobj = PyBytes_FromStringAndSize(pickled, size);
-    if (pickledobj == NULL) {
-        Py_DECREF(loads);
-        return -1;
+    PyObject *lines = PyObject_CallMethod(tbexc, "format", NULL);
+    if (lines == NULL) {
+        return NULL;
     }
-    PyObject *obj = PyObject_CallOneArg(loads, pickledobj);
-    Py_DECREF(loads);
-    Py_DECREF(pickledobj);
-    if (obj == NULL) {
-        return -1;
+    PyObject *formatted_obj = PyUnicode_Join(&_Py_STR(empty), lines);
+    Py_DECREF(lines);
+    if (formatted_obj == NULL) {
+        return NULL;
     }
-    *p_obj = obj;
-    return 0;
+
+    Py_ssize_t size = -1;
+    const char *formatted = _copy_string_obj_raw(formatted_obj, &size);
+    Py_DECREF(formatted_obj);
+    // We remove trailing the newline added by TracebackException.format().
+    assert(formatted[size-1] == '\n');
+    ((char *)formatted)[size-1] = '\0';
+    return formatted;
 }
 
 
@@ -1101,7 +1093,7 @@ _excinfo_init_type(struct _excinfo_type *info, PyObject *exc)
     if (strobj == NULL) {
         return -1;
     }
-    info->name = _copy_string_obj_raw(strobj);
+    info->name = _copy_string_obj_raw(strobj, NULL);
     Py_DECREF(strobj);
     if (info->name == NULL) {
         return -1;
@@ -1112,7 +1104,7 @@ _excinfo_init_type(struct _excinfo_type *info, PyObject *exc)
     if (strobj == NULL) {
         return -1;
     }
-    info->qualname = _copy_string_obj_raw(strobj);
+    info->qualname = _copy_string_obj_raw(strobj, NULL);
     Py_DECREF(strobj);
     if (info->name == NULL) {
         return -1;
@@ -1123,7 +1115,7 @@ _excinfo_init_type(struct _excinfo_type *info, PyObject *exc)
     if (strobj == NULL) {
         return -1;
     }
-    info->module = _copy_string_obj_raw(strobj);
+    info->module = _copy_string_obj_raw(strobj, NULL);
     Py_DECREF(strobj);
     if (info->name == NULL) {
         return -1;
@@ -1188,8 +1180,8 @@ _PyXI_excinfo_Clear(_PyXI_excinfo *info)
     if (info->msg != NULL) {
         PyMem_RawFree((void *)info->msg);
     }
-    if (info->pickled != NULL) {
-        PyMem_RawFree((void *)info->pickled);
+    if (info->errdisplay != NULL) {
+        PyMem_RawFree((void *)info->errdisplay);
     }
     *info = (_PyXI_excinfo){{NULL}};
 }
@@ -1226,63 +1218,6 @@ _PyXI_excinfo_format(_PyXI_excinfo *info)
     }
 }
 
-static int
-_convert_exc_to_TracebackException(PyObject *exc, PyObject **p_tbexc)
-{
-    PyObject *args = NULL;
-    PyObject *kwargs = NULL;
-    PyObject *create = NULL;
-
-    // This is inspired by _PyErr_Display().
-    PyObject *tbmod = PyImport_ImportModule("traceback");
-    if (tbmod == NULL) {
-        return -1;
-    }
-    PyObject *tbexc_type = PyObject_GetAttrString(tbmod, "TracebackException");
-    Py_DECREF(tbmod);
-    if (tbexc_type == NULL) {
-        return -1;
-    }
-    create = PyObject_GetAttrString(tbexc_type, "from_exception");
-    Py_DECREF(tbexc_type);
-    if (create == NULL) {
-        return -1;
-    }
-
-    args = PyTuple_Pack(1, exc);
-    if (args == NULL) {
-        goto error;
-    }
-
-    kwargs = PyDict_New();
-    if (kwargs == NULL) {
-        goto error;
-    }
-    if (PyDict_SetItemString(kwargs, "save_exc_type", Py_False) < 0) {
-        goto error;
-    }
-    if (PyDict_SetItemString(kwargs, "lookup_lines", Py_False) < 0) {
-        goto error;
-    }
-
-    PyObject *tbexc = PyObject_Call(create, args, kwargs);
-    Py_DECREF(args);
-    Py_DECREF(kwargs);
-    Py_DECREF(create);
-    if (tbexc == NULL) {
-        goto error;
-    }
-
-    *p_tbexc = tbexc;
-    return 0;
-
-error:
-    Py_XDECREF(args);
-    Py_XDECREF(kwargs);
-    Py_XDECREF(create);
-    return -1;
-}
-
 static const char *
 _PyXI_excinfo_InitFromException(_PyXI_excinfo *info, PyObject *exc)
 {
@@ -1305,7 +1240,7 @@ _PyXI_excinfo_InitFromException(_PyXI_excinfo *info, PyObject *exc)
         failure = "error while formatting exception";
         goto error;
     }
-    info->msg = _copy_string_obj_raw(msgobj);
+    info->msg = _copy_string_obj_raw(msgobj, NULL);
     Py_DECREF(msgobj);
     if (info->msg == NULL) {
         failure = "error while copying exception message";
@@ -1321,13 +1256,14 @@ _PyXI_excinfo_InitFromException(_PyXI_excinfo *info, PyObject *exc)
         PyErr_Clear();
     }
     else {
-        if (_pickle_object(tbexc, &info->pickled, &info->pickled_len) < 0) {
+        info->errdisplay = _format_TracebackException(tbexc);
+        Py_DECREF(tbexc);
+        if (info->errdisplay == NULL) {
 #ifdef Py_DEBUG
-            PyErr_FormatUnraisable("Exception ignored while pickling TracebackException");
+            PyErr_FormatUnraisable("Exception ignored while formating TracebackException");
 #endif
             PyErr_Clear();
         }
-        Py_DECREF(tbexc);
     }
 
     return NULL;
@@ -1342,8 +1278,9 @@ static void
 _PyXI_excinfo_Apply(_PyXI_excinfo *info, PyObject *exctype)
 {
     PyObject *tbexc = NULL;
-    if (info->pickled != NULL) {
-        if (_unpickle_object(info->pickled, info->pickled_len, &tbexc) < 0) {
+    if (info->errdisplay != NULL) {
+        tbexc = PyUnicode_FromString(info->errdisplay);
+        if (tbexc == NULL) {
             PyErr_Clear();
         }
     }
@@ -1354,9 +1291,9 @@ _PyXI_excinfo_Apply(_PyXI_excinfo *info, PyObject *exctype)
 
     if (tbexc != NULL) {
         PyObject *exc = PyErr_GetRaisedException();
-        if (PyObject_SetAttrString(exc, "_tbexc", tbexc) < 0) {
+        if (PyObject_SetAttrString(exc, "_errdisplay", tbexc) < 0) {
 #ifdef Py_DEBUG
-            PyErr_FormatUnraisable("Exception ignored when setting _tbexc");
+            PyErr_FormatUnraisable("Exception ignored when setting _errdisplay");
 #endif
             PyErr_Clear();
         }
@@ -1468,13 +1405,13 @@ _PyXI_excinfo_AsObject(_PyXI_excinfo *info)
         goto error;
     }
 
-    if (info->pickled != NULL) {
-        PyObject *tbexc = NULL;
-        if (_unpickle_object(info->pickled, info->pickled_len, &tbexc) < 0) {
+    if (info->errdisplay != NULL) {
+        PyObject *tbexc = PyUnicode_FromString(info->errdisplay);
+        if (tbexc == NULL) {
             PyErr_Clear();
         }
         else {
-            res = PyObject_SetAttrString(ns, "tbexc", tbexc);
+            res = PyObject_SetAttrString(ns, "errdisplay", tbexc);
             Py_DECREF(tbexc);
             if (res < 0) {
                 goto error;
@@ -1646,7 +1583,7 @@ _sharednsitem_is_initialized(_PyXI_namespace_item *item)
 static int
 _sharednsitem_init(_PyXI_namespace_item *item, PyObject *key)
 {
-    item->name = _copy_string_obj_raw(key);
+    item->name = _copy_string_obj_raw(key, NULL);
     if (item->name == NULL) {
         assert(!_sharednsitem_is_initialized(item));
         return -1;