[3.11] GH-102818: Do not call PyTraceBack_Here in sys.settrace trampoline (GH-104650)

markshannon · web-flow · commit 7b3bc95067b1 · 2023-05-19T19:08:43.000+01:00
Backport of GH-104579
diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
@@ -1571,6 +1571,62 @@ def func():
 
         self.run_and_compare(func, EXPECTED_EVENTS)
 
+    def test_settrace_error(self):
+
+        raised = False
+        def error_once(frame, event, arg):
+            nonlocal raised
+            if not raised:
+                raised = True
+                raise Exception
+            return error
+
+        try:
+            sys._getframe().f_trace = error_once
+            sys.settrace(error_once)
+            len([])
+        except Exception as ex:
+            count = 0
+            tb = ex.__traceback__
+            print(tb)
+            while tb:
+                if tb.tb_frame.f_code.co_name == "test_settrace_error":
+                    count += 1
+                tb = tb.tb_next
+            if count == 0:
+                self.fail("Traceback is missing frame")
+            elif count > 1:
+                self.fail("Traceback has frame more than once")
+        else:
+            self.fail("No exception raised")
+        finally:
+            sys.settrace(None)
+
+    @support.cpython_only
+    def test_testcapi_settrace_error(self):
+
+        # Skip this test if the _testcapi module isn't available.
+        _testcapi = import_helper.import_module('_testcapi')
+
+        try:
+            _testcapi.settrace_to_error([])
+            len([])
+        except Exception as ex:
+            count = 0
+            tb = ex.__traceback__
+            while tb:
+                if tb.tb_frame.f_code.co_name == "test_testcapi_settrace_error":
+                    count += 1
+                tb = tb.tb_next
+            if count == 0:
+                self.fail("Traceback is missing frame")
+            elif count > 1:
+                self.fail("Traceback has frame more than once")
+        else:
+            self.fail("No exception raised")
+        finally:
+            sys.settrace(None)
+
     def test_very_large_function(self):
         # There is a separate code path when the number of lines > (1 << 15).
         d = {}
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-13-06-22-52.gh-issue-102818.HIX1Dr.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-13-06-22-52.gh-issue-102818.HIX1Dr.rst
@@ -0,0 +1,5 @@
+Do not add a frame to the traceback in the ``sys.setprofile`` and
+``sys.settrace`` trampoline functions. This ensures that frames are not
+duplicated if an exception is raised in the callback function, and ensures
+that frames are not omitted if a C callback is used and that does not add
+the frame.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -6345,6 +6345,33 @@ settrace_to_record(PyObject *self, PyObject *list)
     Py_RETURN_NONE;
 }
 
+static int
+error_func(PyObject *obj, PyFrameObject *f, int what, PyObject *arg)
+{
+    assert(PyList_Check(obj));
+    /* Only raise if list is empty, otherwise append None
+     * This ensures that we only raise once */
+    if (PyList_GET_SIZE(obj)) {
+        return 0;
+    }
+    if (PyList_Append(obj, Py_None)) {
+       return -1;
+    }
+    PyErr_SetString(PyExc_Exception, "an exception");
+    return -1;
+}
+
+static PyObject *
+settrace_to_error(PyObject *self, PyObject *list)
+{
+    if (!PyList_Check(list)) {
+        PyErr_SetString(PyExc_TypeError, "argument must be a list");
+        return NULL;
+    }
+    PyEval_SetTrace(error_func, list);
+    Py_RETURN_NONE;
+}
+
 static PyObject *negative_dictoffset(PyObject *, PyObject *);
 
 static PyObject *
@@ -6694,6 +6721,7 @@ static PyMethodDef TestMethods[] = {
     {"eval_get_func_desc", eval_get_func_desc, METH_O, NULL},
     {"get_feature_macros", get_feature_macros, METH_NOARGS, NULL},
     {"test_code_api", test_code_api, METH_NOARGS, NULL},
+    {"settrace_to_error", settrace_to_error, METH_O, NULL},
     {"settrace_to_record", settrace_to_record, METH_O, NULL},
     {"function_get_code", function_get_code, METH_O, NULL},
     {"function_get_globals", function_get_globals, METH_O, NULL},
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -950,10 +950,6 @@ call_trampoline(PyThreadState *tstate, PyObject* callback,
     PyObject *result = _PyObject_FastCallTstate(tstate, callback, stack, 3);
 
     PyFrame_LocalsToFast(frame, 1);
-    if (result == NULL) {
-        PyTraceBack_Here(frame);
-    }
-
     return result;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -950,10 +950,6 @@ call_trampoline(PyThreadState tstate, PyObject callback,`
`950`	`950`	`PyObject *result = _PyObject_FastCallTstate(tstate, callback, stack, 3);`
`951`	`951`
`952`	`952`	`PyFrame_LocalsToFast(frame, 1);`
`953`		`- if (result == NULL) {`
`954`		`- PyTraceBack_Here(frame);`
`955`		`- }`
`956`		`-`
`957`	`953`	`return result;`
`958`	`954`	`}`
`959`	`955`