From d6e5e4a9d39a6dab5ae83e0aaabd98963d5a17a7 Mon Sep 17 00:00:00 2001 From: Matt Wozniski Date: Tue, 6 Oct 2026 00:13:44 -0400 Subject: [PATCH] Ensure legacy tracing ignores PyMonitoring_Fire* `sys.setprofile` and `sys.settrace` callbacks are no longer called for `sys.monitoring` events that extension modules fire for their own code objects through the monitoring C API. Until now, the `sys.setprofile` or `sys.settrace` function would get run, but it would be passed the top frame on the Python stack rather than anything related to the code object passed to the monitoring C API, and this would manifest to the legacy tracing function as nonsensical duplicate events rather than anything that could be attributed to the code object passed to the monitoring API. --- Lib/test/test_monitoring.py | 162 ++++++++++++------ ...-10-06-00-24-20.gh-issue-158711.6W3Rgg.rst | 3 + Python/legacy_tracing.c | 76 ++++---- 3 files changed, 157 insertions(+), 84 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-06-00-24-20.gh-issue-158711.6W3Rgg.rst diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index 5c2d69934b02ea..ab39e25dbfec96 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -2605,46 +2605,59 @@ def setUp(self): self.cases = [ # (Event, function, *args) - ( 1, E.PY_START, capi.fire_event_py_start), - ( 1, E.PY_RESUME, capi.fire_event_py_resume), - ( 1, E.PY_YIELD, capi.fire_event_py_yield, 10), - ( 1, E.PY_RETURN, capi.fire_event_py_return, 20), - ( 2, E.CALL, capi.fire_event_call, callable, 40), - ( 1, E.JUMP, capi.fire_event_jump, 60), - ( 1, E.BRANCH_RIGHT, capi.fire_event_branch_right, 70), - ( 1, E.BRANCH_LEFT, capi.fire_event_branch_left, 80), - ( 1, E.PY_THROW, capi.fire_event_py_throw, ValueError(1)), - ( 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, 7), - ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, StopIteration(8)), + (E.PY_START, capi.fire_event_py_start), + (E.PY_RESUME, capi.fire_event_py_resume), + (E.PY_YIELD, capi.fire_event_py_yield, 10), + (E.PY_RETURN, capi.fire_event_py_return, 20), + (E.CALL, capi.fire_event_call, callable, 40), + (E.LINE, capi.fire_event_line, 50), + (E.JUMP, capi.fire_event_jump, 60), + (E.BRANCH_RIGHT, capi.fire_event_branch_right, 70), + (E.BRANCH_LEFT, capi.fire_event_branch_left, 80), + (E.C_RETURN, capi.fire_event_c_return, 90), + (E.PY_THROW, capi.fire_event_py_throw, ValueError(1)), + (E.RAISE, capi.fire_event_raise, ValueError(2)), + (E.RERAISE, capi.fire_event_reraise, ValueError(3)), + (E.C_RAISE, capi.fire_event_c_raise, ValueError(4)), + (E.EXCEPTION_HANDLED, capi.fire_event_exception_handled, ValueError(5)), + (E.PY_UNWIND, capi.fire_event_py_unwind, ValueError(6)), + (E.STOP_ITERATION, capi.fire_event_stop_iteration, 7), + (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, callback_raises=None): - class Counter: - 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 + self.EXPECT_RAISED_EXCEPTION = [ + E.PY_THROW, E.RAISE, E.EXCEPTION_HANDLED, E.PY_UNWIND, E.RERAISE, E.C_RAISE + ] + class Counter: + """Count events fired only for the given CodeLike.""" + def __init__(self, codelike, callback_raises=None): + self.codelike = codelike + self.callback_raises = callback_raises + self.disable = False + self.count = 0 + + def __call__(self, code, *args): + if code is not self.codelike: + return + self.count += 1 + if self.callback_raises: + exc = self.callback_raises + self.callback_raises = None + raise exc + if self.disable: + return sys.monitoring.DISABLE + + def check_event_count(self, event, func, args, expected=None, callback_raises=None): try: - counter = Counter(callback_raises) + counter = self.Counter(self.codelike, 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) + event_value = int(math.log2(E.CALL)) else: sys.monitoring.set_events(TEST_TOOL, event) - event_value = int(math.log2(event)) + event_value = int(math.log2(event)) with self.Scope(self.codelike, event_value): counter.count = 0 try: @@ -2654,7 +2667,8 @@ def __call__(self, *args): self.assertEqual(str(e), str(expected)) return else: - self.assertEqual(counter.count, expected) + self.assertIsNone(expected) + self.assertEqual(counter.count, 1) prev = sys.monitoring.register_callback(TEST_TOOL, event, None) with self.Scope(self.codelike, event_value): @@ -2666,15 +2680,15 @@ def __call__(self, *args): sys.monitoring.set_events(TEST_TOOL, 0) def test_fire_event(self): - for expected, event, function, *args in self.cases: + for event, function, *args in self.cases: offset = 0 self.codelike = self._testcapi.CodeLike(1) with self.subTest(function.__name__): args_ = (self.codelike, offset) + tuple(args) - self.check_event_count(event, function, args_, expected) + self.check_event_count(event, function, args_) def test_missing_exception(self): - for _, event, function, *args in self.cases: + for event, function, *args in self.cases: if event not in self.EXPECT_RAISED_EXCEPTION: continue assert args and isinstance(args[-1], BaseException) @@ -2687,33 +2701,34 @@ def test_missing_exception(self): self.check_event_count(event, function, args_, expected) def test_fire_event_failing_callback(self): - for expected, event, function, *args in self.cases: + for event, function, *args in self.cases: offset = 0 self.codelike = self._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, + self.check_event_count(event, function, args_, callback_raises=exc) CANNOT_DISABLE = { E.PY_THROW, E.RAISE, E.RERAISE, - E.EXCEPTION_HANDLED, E.PY_UNWIND } + E.EXCEPTION_HANDLED, E.PY_UNWIND, E.C_RETURN, E.C_RAISE } - def check_disable(self, event, func, args, expected): + def check_disable(self, event, func, args): try: - counter = CounterWithDisable() + counter = self.Counter(self.codelike) 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) + event_value = int(math.log2(E.CALL)) else: sys.monitoring.set_events(TEST_TOOL, event) - event_value = int(math.log2(event)) + event_value = int(math.log2(event)) with self.Scope(self.codelike, event_value): counter.count = 0 func(*args) - self.assertEqual(counter.count, expected) + self.assertEqual(counter.count, 1) counter.disable = True if event in self.CANNOT_DISABLE: # use try-except rather then assertRaises to avoid @@ -2721,28 +2736,27 @@ def check_disable(self, event, func, args, expected): try: counter.count = 0 func(*args) - self.assertEqual(counter.count, expected) + self.assertEqual(counter.count, 1) except ValueError: pass else: - self.Error("Expected a ValueError") + self.fail("Expected a ValueError") else: counter.count = 0 func(*args) - self.assertEqual(counter.count, expected) + self.assertEqual(counter.count, 1) counter.count = 0 func(*args) - self.assertEqual(counter.count, expected - 1) + self.assertEqual(counter.count, 0) finally: sys.monitoring.set_events(TEST_TOOL, 0) def test_disable_event(self): - for expected, event, function, *args in self.cases: - offset = 0 + for event, function, *args in self.cases: self.codelike = self._testcapi.CodeLike(2) with self.subTest(function.__name__): args_ = (self.codelike, 0) + tuple(args) - self.check_disable(event, function, args_, expected) + self.check_disable(event, function, args_) def test_enter_scope_two_events(self): _testcapi = self._testcapi @@ -2781,3 +2795,53 @@ def test_enter_scope_two_events(self): finally: sys.monitoring.set_events(TEST_TOOL, 0) + + def check_legacy_tracing_ignores_pymonitoring_fire(self, setter, expected): + # gh-158711: When a PyMonitoring_Fire* function is passed a code object + # not being run by the top frame, it must not trigger a call to the + # legacy trace or profile func. + def fire(function, args): + function(*args) + + for event, function, *args in self.cases: + with self.subTest(function.__name__): + events = [] + def tracer(frame, what, _arg): + if frame.f_code is fire.__code__: + events.append(what) + return tracer + + self.codelike = self._testcapi.CodeLike(1) + args_ = (self.codelike, 0) + tuple(args) + if event == E.C_RETURN or event == E.C_RAISE: + event_value = int(math.log2(E.CALL)) + else: + event_value = int(math.log2(event)) + # Also watch the event with a sys.monitoring tool, so that a + # correctly ignored event can be told apart from one that was + # never dispatched at all (e.g. if Scope stopped activating). + counter = self.Counter(self.codelike) + try: + sys.monitoring.register_callback(TEST_TOOL, event, counter) + sys.monitoring.set_events(TEST_TOOL, 1 << event_value) + setter(tracer) + with self.Scope(self.codelike, event_value): + fire(function, args_) + finally: + setter(None) + sys.monitoring.set_events(TEST_TOOL, 0) + sys.monitoring.register_callback(TEST_TOOL, event, None) + self.assertEqual(events, expected) + self.assertEqual(counter.count, 1) + + def test_setprofile_ignores_pymonitoring_fire(self): + self.check_legacy_tracing_ignores_pymonitoring_fire( + sys.setprofile, + ['call', 'c_call', 'c_return', 'return'], + ) + + def test_settrace_ignores_pymonitoring_fire(self): + self.check_legacy_tracing_ignores_pymonitoring_fire( + sys.settrace, + ['call', 'line', 'return'], + ) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-06-00-24-20.gh-issue-158711.6W3Rgg.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-06-00-24-20.gh-issue-158711.6W3Rgg.rst new file mode 100644 index 00000000000000..cb9adcf51eceac --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-06-00-24-20.gh-issue-158711.6W3Rgg.rst @@ -0,0 +1,3 @@ +:func:`sys.setprofile` and :func:`sys.settrace` callbacks are no longer called +for :mod:`sys.monitoring` events that extension modules fire for their own code +objects through the :ref:`monitoring C API `. diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index bf65a904de4d21..486f274948ab20 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -27,18 +27,27 @@ typedef struct _PyLegacyEventHandler { * arg: The arg (a PyObject *) */ +/* Return the top frame (borrowed) if it's running `code`, else NULL. */ +static PyFrameObject * +get_top_frame_if_running_code(PyObject *code) +{ + PyFrameObject *frame = PyEval_GetFrame(); + if (frame == NULL || (PyObject *)_PyFrame_GetCode(frame->f_frame) != code) { + return NULL; + } + return frame; +} + static PyObject * -call_profile_func(_PyLegacyEventHandler *self, PyObject *arg) +call_profile_func(_PyLegacyEventHandler *self, PyObject *code, PyObject *arg) { PyThreadState *tstate = _PyThreadState_GET(); if (tstate->c_profilefunc == NULL) { Py_RETURN_NONE; } - PyFrameObject *frame = PyEval_GetFrame(); + PyFrameObject *frame = get_top_frame_if_running_code(code); if (frame == NULL) { - PyErr_SetString(PyExc_SystemError, - "Missing frame when calling profile function."); - return NULL; + Py_RETURN_NONE; } Py_INCREF(frame); int err = tstate->c_profilefunc(tstate->c_profileobj, frame, self->event, arg); @@ -57,7 +66,7 @@ sys_profile_start( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 2); - return call_profile_func(self, Py_None); + return call_profile_func(self, args[0], Py_None); } static PyObject * @@ -68,7 +77,7 @@ sys_profile_throw( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_profile_func(self, Py_None); + return call_profile_func(self, args[0], Py_None); } static PyObject * @@ -79,7 +88,7 @@ sys_profile_return( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_profile_func(self, args[2]); + return call_profile_func(self, args[0], args[2]); } static PyObject * @@ -90,7 +99,7 @@ sys_profile_unwind( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_profile_func(self, NULL); + return call_profile_func(self, args[0], NULL); } static PyObject * @@ -100,10 +109,15 @@ sys_profile_call_or_return( ) { _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(op); assert(kwnames == NULL); - assert(PyVectorcall_NARGS(nargsf) == 4); + if (PyVectorcall_NARGS(nargsf) != 4) { + // PyMonitoring_FireCReturnEvent and PyMonitoring_FireCRaiseEvent + // don't provide the callable. + assert(PyVectorcall_NARGS(nargsf) == 3); + Py_RETURN_NONE; + } PyObject *callable = args[2]; if (PyCFunction_Check(callable)) { - return call_profile_func(self, callable); + return call_profile_func(self, args[0], callable); } if (Py_TYPE(callable) == &PyMethodDescr_Type) { PyObject *self_arg = args[3]; @@ -119,7 +133,7 @@ sys_profile_call_or_return( if (meth == NULL) { return NULL; } - PyObject *res = call_profile_func(self, meth); + PyObject *res = call_profile_func(self, args[0], meth); Py_DECREF(meth); return res; } @@ -175,17 +189,15 @@ _PyEval_SetOpcodeTrace(PyFrameObject *frame, bool enable) } static PyObject * -call_trace_func(_PyLegacyEventHandler *self, PyObject *arg) +call_trace_func(_PyLegacyEventHandler *self, PyObject *code, PyObject *arg) { PyThreadState *tstate = _PyThreadState_GET(); if (tstate->c_tracefunc == NULL) { Py_RETURN_NONE; } - PyFrameObject *frame = PyEval_GetFrame(); + PyFrameObject *frame = get_top_frame_if_running_code(code); if (frame == NULL) { - PyErr_SetString(PyExc_SystemError, - "Missing frame when calling trace function."); - return NULL; + Py_RETURN_NONE; } if (frame->f_trace_opcodes) { if (_PyEval_SetOpcodeTrace(frame, true) != 0) { @@ -223,7 +235,7 @@ sys_trace_exception_func( if (tuple == NULL) { return NULL; } - PyObject *res = call_trace_func(self, tuple); + PyObject *res = call_trace_func(self, args[0], tuple); Py_DECREF(tuple); return res; } @@ -236,7 +248,7 @@ sys_trace_start( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 2); - return call_trace_func(self, Py_None); + return call_trace_func(self, args[0], Py_None); } static PyObject * @@ -247,7 +259,7 @@ sys_trace_throw( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_trace_func(self, Py_None); + return call_trace_func(self, args[0], Py_None); } static PyObject * @@ -258,7 +270,7 @@ sys_trace_unwind( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_trace_func(self, NULL); + return call_trace_func(self, args[0], NULL); } static PyObject * @@ -270,9 +282,8 @@ sys_trace_return( assert(!PyErr_Occurred()); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - assert(PyCode_Check(args[0])); PyObject *val = args[2]; - PyObject *res = call_trace_func(self, val); + PyObject *res = call_trace_func(self, args[0], val); return res; } @@ -284,7 +295,7 @@ sys_trace_yield( _PyLegacyEventHandler *self = _PyLegacyEventHandler_CAST(callable); assert(kwnames == NULL); assert(PyVectorcall_NARGS(nargsf) == 3); - return call_trace_func(self, args[2]); + return call_trace_func(self, args[0], args[2]); } static PyObject * @@ -354,13 +365,10 @@ sys_trace_line_func( assert(PyVectorcall_NARGS(nargsf) == 2); int line = PyLong_AsInt(args[1]); assert(line >= 0); - PyFrameObject *frame = PyEval_GetFrame(); + PyFrameObject *frame = get_top_frame_if_running_code(args[0]); if (frame == NULL) { - PyErr_SetString(PyExc_SystemError, - "Missing frame when calling trace function."); - return NULL; + Py_RETURN_NONE; } - assert(args[0] == (PyObject *)_PyFrame_GetCode(frame->f_frame)); return trace_line(tstate, self, frame, line); } @@ -379,6 +387,10 @@ sys_trace_jump_func( Py_RETURN_NONE; } assert(PyVectorcall_NARGS(nargsf) == 3); + PyFrameObject *frame = get_top_frame_if_running_code(args[0]); + if (frame == NULL) { + Py_RETURN_NONE; + } int from = PyLong_AsInt(args[1])/sizeof(_Py_CODEUNIT); assert(from >= 0); int to = PyLong_AsInt(args[2])/sizeof(_Py_CODEUNIT); @@ -397,12 +409,6 @@ sys_trace_jump_func( /* Will be handled by target INSTRUMENTED_LINE */ return &_PyInstrumentation_DISABLE; } - PyFrameObject *frame = PyEval_GetFrame(); - if (frame == NULL) { - PyErr_SetString(PyExc_SystemError, - "Missing frame when calling trace function."); - return NULL; - } if (!frame->f_trace_lines) { Py_RETURN_NONE; }