GH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline. (GH-104579)

4 files changed, 89 insertions, 4 deletions

diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
index 4411603..0571919 100644
--- a/Lib/test/test_sys_settrace.py
+++ b/Lib/test/test_sys_settrace.py
@@ -1568,6 +1568,62 @@ class TraceTestCase(unittest.TestCase):
 
         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-12-13-30-04.gh-issue-102818.rnv1mH.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-12-13-30-04.gh-issue-102818.rnv1mH.rst
new file mode 100644
index 0000000..5cb77bf
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-12-13-30-04.gh-issue-102818.rnv1mH.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
index c29d29c..1be1dd6 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -3062,6 +3062,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 *
 clear_managed_dict(PyObject *self, PyObject *obj)
 {
@@ -3352,6 +3379,7 @@ static PyMethodDef TestMethods[] = {
     {"gen_get_code", gen_get_code, 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},
     {"test_macros", test_macros, METH_NOARGS, NULL},
     {"clear_managed_dict", clear_managed_dict, METH_O, NULL},
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index c116c40..7510be5 100644
--- 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;
 }