gh-118692: Avoid creating unnecessary StopIteration instances for monitoring (#119216)

Doc/c-api/monitoring.rst

@@ -121,10 +121,10 @@ See :mod:`sys.monitoring` for descriptions of the events.
    :c:func:`PyErr_GetRaisedException`).
 
 
-.. c:function:: int PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset)
+.. c:function:: int PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value)
 
-   Fire a ``STOP_ITERATION`` event with the current exception (as returned by
+   Fire a ``STOP_ITERATION`` event. If ``value`` is an instance of :exc:`StopIteration`, it is used. Otherwise,
-   :c:func:`PyErr_GetRaisedException`).
+   a new :exc:`StopIteration` instance is created with ``value`` as its argument.
 
 
 Managing the Monitoring State

Include/cpython/monitoring.h

@@ -101,7 +101,7 @@ PyAPI_FUNC(int)
 _PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset);
 
 PyAPI_FUNC(int)
-_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset);
+_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value);
 
 
 #define _PYMONITORING_IF_ACTIVE(STATE, X)  \
@@ -240,11 +240,11 @@ PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, int
 }
 
 static inline int
-PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset)
+PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value)
 {
     _PYMONITORING_IF_ACTIVE(
         state,
-        _PyMonitoring_FireStopIterationEvent(state, codelike, offset));
+        _PyMonitoring_FireStopIterationEvent(state, codelike, offset, value));
 }
 
 #undef _PYMONITORING_IF_ACTIVE

Include/internal/pycore_opcode_metadata.h

@@ -1060,8 +1060,8 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[268] = {
     [INSTRUMENTED_CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
     [INSTRUMENTED_CALL_FUNCTION_EX] = { true, INSTR_FMT_IX, 0 },
     [INSTRUMENTED_CALL_KW] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
-    [INSTRUMENTED_END_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG },
+    [INSTRUMENTED_END_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG },
-    [INSTRUMENTED_END_SEND] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG },
+    [INSTRUMENTED_END_SEND] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG },
     [INSTRUMENTED_FOR_ITER] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG },
     [INSTRUMENTED_INSTRUCTION] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG },
     [INSTRUMENTED_JUMP_BACKWARD] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG },

Lib/test/test_monitoring.py

@@ -1938,19 +1938,28 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase):
             ( 1, E.RAISE, capi.fire_event_raise, ValueError(2)),
             ( 1, E.EXCEPTION_HANDLED, capi.fire_event_exception_handled, ValueError(5)),
             ( 1, E.PY_UNWIND, capi.fire_event_py_unwind, ValueError(6)),
-            ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, ValueError(7)),
+            ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, 7),
+            ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, StopIteration(8)),
         ]
 
+        self.EXPECT_RAISED_EXCEPTION = [E.PY_THROW, E.RAISE, E.EXCEPTION_HANDLED, E.PY_UNWIND]
 
-    def check_event_count(self, event, func, args, expected):
+
+    def check_event_count(self, event, func, args, expected, callback_raises=None):
         class Counter:
-            def __init__(self):
+            def __init__(self, callback_raises):
+                self.callback_raises = callback_raises
                 self.count = 0
+
             def __call__(self, *args):
                 self.count += 1
+                if self.callback_raises:
+                    exc = self.callback_raises
+                    self.callback_raises = None
+                    raise exc
 
         try:
-            counter = Counter()
+            counter = Counter(callback_raises)
             sys.monitoring.register_callback(TEST_TOOL, event, counter)
             if event == E.C_RETURN or event == E.C_RAISE:
                 sys.monitoring.set_events(TEST_TOOL, E.CALL)
@@ -1987,8 +1996,9 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase):
 
     def test_missing_exception(self):
         for _, event, function, *args in self.cases:
-            if not (args and isinstance(args[-1], BaseException)):
+            if event not in self.EXPECT_RAISED_EXCEPTION:
                 continue
+            assert args and isinstance(args[-1], BaseException)
             offset = 0
             self.codelike = _testcapi.CodeLike(1)
             with self.subTest(function.__name__):
@@ -1997,6 +2007,17 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase):
                 expected = ValueError(f"Firing event {evt} with no exception set")
                 self.check_event_count(event, function, args_, expected)
 
+    def test_fire_event_failing_callback(self):
+        for expected, event, function, *args in self.cases:
+            offset = 0
+            self.codelike = _testcapi.CodeLike(1)
+            with self.subTest(function.__name__):
+                args_ = (self.codelike, offset) + tuple(args)
+                exc = OSError(42)
+                with self.assertRaises(type(exc)):
+                    self.check_event_count(event, function, args_, expected,
+                                           callback_raises=exc)
+
 
     CANNOT_DISABLE = { E.PY_THROW, E.RAISE, E.RERAISE,
                        E.EXCEPTION_HANDLED, E.PY_UNWIND }

Misc/NEWS.d/next/Core and Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst

@@ -0,0 +1 @@
+Avoid creating unnecessary :exc:`StopIteration` instances for monitoring.

Modules/_testcapi/monitoring.c

@@ -416,16 +416,17 @@ fire_event_stop_iteration(PyObject *self, PyObject *args)
 {
     PyObject *codelike;
     int offset;
-    PyObject *exception;
+    PyObject *value;
-    if (!PyArg_ParseTuple(args, "OiO", &codelike, &offset, &exception)) {
+    if (!PyArg_ParseTuple(args, "OiO", &codelike, &offset, &value)) {
         return NULL;
     }
-    NULLABLE(exception);
+    NULLABLE(value);
+    PyObject *exception = NULL;
     PyMonitoringState *state = setup_fire(codelike, offset, exception);
     if (state == NULL) {
         return NULL;
     }
-    int res = PyMonitoring_FireStopIterationEvent(state, codelike, offset);
+    int res = PyMonitoring_FireStopIterationEvent(state, codelike, offset, value);
     RETURN_INT(teardown_fire(res, state, exception));
 }
 

Python/bytecodes.c

@@ -292,11 +292,9 @@ dummy_func(
             /* Need to create a fake StopIteration error here,
              * to conform to PEP 380 */
             if (PyGen_Check(receiver)) {
-                PyErr_SetObject(PyExc_StopIteration, value);
+                if (monitor_stop_iteration(tstate, frame, this_instr, value)) {
-                if (monitor_stop_iteration(tstate, frame, this_instr)) {
                     ERROR_NO_POP();
                 }
-                PyErr_SetRaisedException(NULL);
             }
             DECREF_INPUTS();
         }
@@ -307,11 +305,9 @@ dummy_func(
 
         tier1 inst(INSTRUMENTED_END_SEND, (receiver, value -- value)) {
             if (PyGen_Check(receiver) || PyCoro_CheckExact(receiver)) {
-                PyErr_SetObject(PyExc_StopIteration, value);
+                if (monitor_stop_iteration(tstate, frame, this_instr, value)) {
-                if (monitor_stop_iteration(tstate, frame, this_instr)) {
                     ERROR_NO_POP();
                 }
-                PyErr_SetRaisedException(NULL);
             }
             Py_DECREF(receiver);
         }

Python/ceval.c

@@ -231,7 +231,8 @@ static void monitor_reraise(PyThreadState *tstate,
                  _Py_CODEUNIT *instr);
 static int monitor_stop_iteration(PyThreadState *tstate,
                  _PyInterpreterFrame *frame,
-                 _Py_CODEUNIT *instr);
+                 _Py_CODEUNIT *instr,
+                 PyObject *value);
 static void monitor_unwind(PyThreadState *tstate,
                  _PyInterpreterFrame *frame,
                  _Py_CODEUNIT *instr);
@@ -2215,12 +2216,19 @@ monitor_reraise(PyThreadState *tstate, _PyInterpreterFrame *frame,
 
 static int
 monitor_stop_iteration(PyThreadState *tstate, _PyInterpreterFrame *frame,
-                       _Py_CODEUNIT *instr)
+                       _Py_CODEUNIT *instr, PyObject *value)
 {
     if (no_tools_for_local_event(tstate, frame, PY_MONITORING_EVENT_STOP_ITERATION)) {
         return 0;
     }
-    return do_monitor_exc(tstate, frame, instr, PY_MONITORING_EVENT_STOP_ITERATION);
+    assert(!PyErr_Occurred());
+    PyErr_SetObject(PyExc_StopIteration, value);
+    int res = do_monitor_exc(tstate, frame, instr, PY_MONITORING_EVENT_STOP_ITERATION);
+    if (res < 0) {
+        return res;
+    }
+    PyErr_SetRaisedException(NULL);
+    return 0;
 }
 
 static void

Python/generated_cases.c.h

@@ -3260,11 +3260,9 @@
             /* Need to create a fake StopIteration error here,
              * to conform to PEP 380 */
             if (PyGen_Check(receiver)) {
-                PyErr_SetObject(PyExc_StopIteration, value);
+                if (monitor_stop_iteration(tstate, frame, this_instr, value)) {
-                if (monitor_stop_iteration(tstate, frame, this_instr)) {
                     goto error;
                 }
-                PyErr_SetRaisedException(NULL);
             }
             Py_DECREF(value);
             stack_pointer += -1;
@@ -3281,11 +3279,9 @@
             value = stack_pointer[-1];
             receiver = stack_pointer[-2];
             if (PyGen_Check(receiver) || PyCoro_CheckExact(receiver)) {
-                PyErr_SetObject(PyExc_StopIteration, value);
+                if (monitor_stop_iteration(tstate, frame, this_instr, value)) {
-                if (monitor_stop_iteration(tstate, frame, this_instr)) {
                     goto error;
                 }
-                PyErr_SetRaisedException(NULL);
             }
             Py_DECREF(receiver);
             stack_pointer[-2] = value;

Python/instrumentation.c

@@ -2622,7 +2622,7 @@ exception_event_teardown(int err, PyObject *exc) {
     }
     else {
         assert(PyErr_Occurred());
-        Py_DECREF(exc);
+        Py_XDECREF(exc);
     }
     return err;
 }
@@ -2712,15 +2712,17 @@ _PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, in
 }
 
 int
-_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset)
+_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value)
 {
     int event = PY_MONITORING_EVENT_STOP_ITERATION;
     assert(state->active);
+    assert(!PyErr_Occurred());
+    PyErr_SetObject(PyExc_StopIteration, value);
     PyObject *exc;
     if (exception_event_setup(&exc, event) < 0) {
         return -1;
     }
     PyObject *args[4] = { NULL, NULL, NULL, exc };
     int err = capi_call_instrumentation(state, codelike, offset, args, 3, event);
-    return exception_event_teardown(err, exc);
+    return exception_event_teardown(err, NULL);
 }