Defer frame acquisition in sys.monitoring callbacks to reduce object retention

In _return_event: move _getframe(1) call after step_cmd == -1 early return
check, avoiding unnecessary frame creation when not stepping.

In _start_method_event: pass depth=1 to _get_func_code_info instead of frame
object, deferring frame creation to after always_skip_code check.

In _unwind_event and _raise_event: wrap frame/arg usage in try/finally to
explicitly clear references after use, preventing exception tracebacks from
keeping frame objects alive.

Regenerated _pydevd_sys_monitoring_cython.pyx from updated Python source.

Co-authored-by: rchiodo <19672699+rchiodo@users.noreply.github.com>

Author: Copilot

src/debugpy/_vendored/pydevd/_pydevd_sys_monitoring/_pydevd_sys_monitoring.py

@@ -895,36 +895,43 @@ def _unwind_event(code, instruction, exc):
 
     # print('_unwind_event', code, exc)
     frame = _getframe(1)
-    arg = (type(exc), exc, exc.__traceback__)
-
-    has_caught_exception_breakpoint_in_pydb = (
-        py_db.break_on_caught_exceptions or py_db.break_on_user_uncaught_exceptions or py_db.has_plugin_exception_breaks
-    )
+    try:
+        arg = (type(exc), exc, exc.__traceback__)
 
-    if has_caught_exception_breakpoint_in_pydb:
-        _should_stop, frame, user_uncaught_exc_info = should_stop_on_exception(
-            py_db, thread_info.additional_info, frame, thread_info.thread, arg, None, is_unwind=True
+        has_caught_exception_breakpoint_in_pydb = (
+            py_db.break_on_caught_exceptions or py_db.break_on_user_uncaught_exceptions or py_db.has_plugin_exception_breaks
         )
-        if user_uncaught_exc_info:
-            # TODO: Check: this may no longer be needed as in the unwind we know it's
-            # an exception bubbling up (wait for all tests to pass to check it).
-            if func_code_info.try_except_container_obj is None:
-                container_obj = _TryExceptContainerObj(py_db.collect_try_except_info(frame.f_code))
-                func_code_info.try_except_container_obj = container_obj
-
-            is_unhandled = is_unhandled_exception(
-                func_code_info.try_except_container_obj, py_db, frame, user_uncaught_exc_info[1], user_uncaught_exc_info[2]
+
+        if has_caught_exception_breakpoint_in_pydb:
+            _should_stop, frame, user_uncaught_exc_info = should_stop_on_exception(
+                py_db, thread_info.additional_info, frame, thread_info.thread, arg, None, is_unwind=True
             )
+            if user_uncaught_exc_info:
+                # TODO: Check: this may no longer be needed as in the unwind we know it's
+                # an exception bubbling up (wait for all tests to pass to check it).
+                if func_code_info.try_except_container_obj is None:
+                    container_obj = _TryExceptContainerObj(py_db.collect_try_except_info(frame.f_code))
+                    func_code_info.try_except_container_obj = container_obj
+
+                is_unhandled = is_unhandled_exception(
+                    func_code_info.try_except_container_obj, py_db, frame, user_uncaught_exc_info[1], user_uncaught_exc_info[2]
+                )
+
+                if is_unhandled:
+                    handle_exception(py_db, thread_info.thread, frame, user_uncaught_exc_info[0], EXCEPTION_TYPE_USER_UNHANDLED)
+                    return
 
-            if is_unhandled:
-                handle_exception(py_db, thread_info.thread, frame, user_uncaught_exc_info[0], EXCEPTION_TYPE_USER_UNHANDLED)
+        break_on_uncaught_exceptions = py_db.break_on_uncaught_exceptions
+        if break_on_uncaught_exceptions:
+            if frame is _get_unhandled_exception_frame(exc, 1):
+                stop_on_unhandled_exception(py_db, thread_info.thread, thread_info.additional_info, arg)
                 return
-
-    break_on_uncaught_exceptions = py_db.break_on_uncaught_exceptions
-    if break_on_uncaught_exceptions:
-        if frame is _get_unhandled_exception_frame(exc, 1):
-            stop_on_unhandled_exception(py_db, thread_info.thread, thread_info.additional_info, arg)
-            return
+    finally:
+        # Clear frame and arg references to avoid preventing garbage collection
+        # of objects referenced from the frame's locals. The arg tuple also
+        # contains the traceback which may hold frame references.
+        del frame
+        del arg
 
 
 # fmt: off
@@ -967,21 +974,28 @@ def _raise_event(code, instruction, exc):
         return
 
     frame = _getframe(1)
-    arg = (type(exc), exc, exc.__traceback__)
+    try:
+        arg = (type(exc), exc, exc.__traceback__)
 
-    # Compute the previous exception info (if any). We use it to check if the exception
-    # should be stopped
-    prev_exc_info = _thread_local_info._user_uncaught_exc_info if hasattr(_thread_local_info, "_user_uncaught_exc_info") else None
-    should_stop, frame, _user_uncaught_exc_info = should_stop_on_exception(
-        py_db, thread_info.additional_info, frame, thread_info.thread, arg, prev_exc_info
-    )
+        # Compute the previous exception info (if any). We use it to check if the exception
+        # should be stopped
+        prev_exc_info = _thread_local_info._user_uncaught_exc_info if hasattr(_thread_local_info, "_user_uncaught_exc_info") else None
+        should_stop, frame, _user_uncaught_exc_info = should_stop_on_exception(
+            py_db, thread_info.additional_info, frame, thread_info.thread, arg, prev_exc_info
+        )
 
-    # Save the current exception info for the next raise event.
-    _thread_local_info._user_uncaught_exc_info = _user_uncaught_exc_info
+        # Save the current exception info for the next raise event.
+        _thread_local_info._user_uncaught_exc_info = _user_uncaught_exc_info
 
-    # print('!!!! should_stop (in raise)', should_stop)
-    if should_stop:
-        handle_exception(py_db, thread_info.thread, frame, arg, EXCEPTION_TYPE_HANDLED)
+        # print('!!!! should_stop (in raise)', should_stop)
+        if should_stop:
+            handle_exception(py_db, thread_info.thread, frame, arg, EXCEPTION_TYPE_HANDLED)
+    finally:
+        # Clear frame and arg references to avoid preventing garbage collection
+        # of objects referenced from the frame's locals. The arg tuple also
+        # contains the traceback which may hold frame references.
+        del frame
+        del arg
 
 
 # fmt: off
@@ -1083,13 +1097,16 @@ def _return_event(code, instruction, retval):
 
     info = thread_info.additional_info
 
-    # We know the frame depth.
-    frame = _getframe(1)
-
     step_cmd = info.pydev_step_cmd
     if step_cmd == -1:
         return
 
+    # We know the frame depth.
+    # Note: getting the frame lazily (only when we need it) is important
+    # because obtaining a frame creates a reference that will keep all
+    # objects in that frame alive until this callback returns.
+    frame = _getframe(1)
+
     if info.suspend_type != PYTHON_SUSPEND:
         # Plugin stepping
         if func_code_info.plugin_return_stepping:
@@ -1703,13 +1720,20 @@ def _start_method_event(code, instruction_offset):
         # threads may still want it...
         return
 
-    frame = _getframe(1)
-    func_code_info = _get_func_code_info(code, frame)
+    # Note: passing depth=1 instead of the frame avoids creating a frame reference
+    # unnecessarily in the fast path (cache hit). The frame will only be obtained
+    # inside _get_func_code_info if needed (cache miss).
+    func_code_info = _get_func_code_info(code, 1)
     if func_code_info.always_skip_code:
         # if DEBUG:
         #     print('disable (always skip)')
         return monitor.DISABLE
 
+    # Only get the frame when we actually need it (after the always_skip_code check).
+    # Obtaining a frame creates a reference that keeps all objects in that frame
+    # alive until this callback returns.
+    frame = _getframe(1)
+
     keep_enabled: bool = _enable_code_tracing(py_db, thread_info.additional_info, func_code_info, code, frame, True)
 
     if func_code_info.function_breakpoint_found:

src/debugpy/_vendored/pydevd/_pydevd_sys_monitoring/_pydevd_sys_monitoring_cython.pyx

@@ -901,36 +901,43 @@ cdef _unwind_event(code, instruction, exc):
 
     # print('_unwind_event', code, exc)
     frame = _getframe(1)
-    arg = (type(exc), exc, exc.__traceback__)
-
-    has_caught_exception_breakpoint_in_pydb = (
-        py_db.break_on_caught_exceptions or py_db.break_on_user_uncaught_exceptions or py_db.has_plugin_exception_breaks
-    )
+    try:
+        arg = (type(exc), exc, exc.__traceback__)
 
-    if has_caught_exception_breakpoint_in_pydb:
-        _should_stop, frame, user_uncaught_exc_info = should_stop_on_exception(
-            py_db, thread_info.additional_info, frame, thread_info.thread, arg, None, is_unwind=True
+        has_caught_exception_breakpoint_in_pydb = (
+            py_db.break_on_caught_exceptions or py_db.break_on_user_uncaught_exceptions or py_db.has_plugin_exception_breaks
         )
-        if user_uncaught_exc_info:
-            # TODO: Check: this may no longer be needed as in the unwind we know it's
-            # an exception bubbling up (wait for all tests to pass to check it).
-            if func_code_info.try_except_container_obj is None:
-                container_obj = _TryExceptContainerObj(py_db.collect_try_except_info(frame.f_code))
-                func_code_info.try_except_container_obj = container_obj
-
-            is_unhandled = is_unhandled_exception(
-                func_code_info.try_except_container_obj, py_db, frame, user_uncaught_exc_info[1], user_uncaught_exc_info[2]
+
+        if has_caught_exception_breakpoint_in_pydb:
+            _should_stop, frame, user_uncaught_exc_info = should_stop_on_exception(
+                py_db, thread_info.additional_info, frame, thread_info.thread, arg, None, is_unwind=True
             )
+            if user_uncaught_exc_info:
+                # TODO: Check: this may no longer be needed as in the unwind we know it's
+                # an exception bubbling up (wait for all tests to pass to check it).
+                if func_code_info.try_except_container_obj is None:
+                    container_obj = _TryExceptContainerObj(py_db.collect_try_except_info(frame.f_code))
+                    func_code_info.try_except_container_obj = container_obj
+
+                is_unhandled = is_unhandled_exception(
+                    func_code_info.try_except_container_obj, py_db, frame, user_uncaught_exc_info[1], user_uncaught_exc_info[2]
+                )
+
+                if is_unhandled:
+                    handle_exception(py_db, thread_info.thread, frame, user_uncaught_exc_info[0], EXCEPTION_TYPE_USER_UNHANDLED)
+                    return
 
-            if is_unhandled:
-                handle_exception(py_db, thread_info.thread, frame, user_uncaught_exc_info[0], EXCEPTION_TYPE_USER_UNHANDLED)
+        break_on_uncaught_exceptions = py_db.break_on_uncaught_exceptions
+        if break_on_uncaught_exceptions:
+            if frame is _get_unhandled_exception_frame(exc, 1):
+                stop_on_unhandled_exception(py_db, thread_info.thread, thread_info.additional_info, arg)
                 return
-
-    break_on_uncaught_exceptions = py_db.break_on_uncaught_exceptions
-    if break_on_uncaught_exceptions:
-        if frame is _get_unhandled_exception_frame(exc, 1):
-            stop_on_unhandled_exception(py_db, thread_info.thread, thread_info.additional_info, arg)
-            return
+    finally:
+        # Clear frame and arg references to avoid preventing garbage collection
+        # of objects referenced from the frame's locals. The arg tuple also
+        # contains the traceback which may hold frame references.
+        del frame
+        del arg
 
 
 # fmt: off
@@ -973,21 +980,28 @@ cdef _raise_event(code, instruction, exc):
         return
 
     frame = _getframe(1)
-    arg = (type(exc), exc, exc.__traceback__)
+    try:
+        arg = (type(exc), exc, exc.__traceback__)
 
-    # Compute the previous exception info (if any). We use it to check if the exception
-    # should be stopped
-    prev_exc_info = _thread_local_info._user_uncaught_exc_info if hasattr(_thread_local_info, "_user_uncaught_exc_info") else None
-    should_stop, frame, _user_uncaught_exc_info = should_stop_on_exception(
-        py_db, thread_info.additional_info, frame, thread_info.thread, arg, prev_exc_info
-    )
+        # Compute the previous exception info (if any). We use it to check if the exception
+        # should be stopped
+        prev_exc_info = _thread_local_info._user_uncaught_exc_info if hasattr(_thread_local_info, "_user_uncaught_exc_info") else None
+        should_stop, frame, _user_uncaught_exc_info = should_stop_on_exception(
+            py_db, thread_info.additional_info, frame, thread_info.thread, arg, prev_exc_info
+        )
 
-    # Save the current exception info for the next raise event.
-    _thread_local_info._user_uncaught_exc_info = _user_uncaught_exc_info
+        # Save the current exception info for the next raise event.
+        _thread_local_info._user_uncaught_exc_info = _user_uncaught_exc_info
 
-    # print('!!!! should_stop (in raise)', should_stop)
-    if should_stop:
-        handle_exception(py_db, thread_info.thread, frame, arg, EXCEPTION_TYPE_HANDLED)
+        # print('!!!! should_stop (in raise)', should_stop)
+        if should_stop:
+            handle_exception(py_db, thread_info.thread, frame, arg, EXCEPTION_TYPE_HANDLED)
+    finally:
+        # Clear frame and arg references to avoid preventing garbage collection
+        # of objects referenced from the frame's locals. The arg tuple also
+        # contains the traceback which may hold frame references.
+        del frame
+        del arg
 
 
 # fmt: off
@@ -1089,13 +1103,16 @@ cdef _return_event(code, instruction, retval):
 
     info = thread_info.additional_info
 
-    # We know the frame depth.
-    frame = _getframe(1)
-
     step_cmd = info.pydev_step_cmd
     if step_cmd == -1:
         return
 
+    # We know the frame depth.
+    # Note: getting the frame lazily (only when we need it) is important
+    # because obtaining a frame creates a reference that will keep all
+    # objects in that frame alive until this callback returns.
+    frame = _getframe(1)
+
     if info.suspend_type != PYTHON_SUSPEND:
         # Plugin stepping
         if func_code_info.plugin_return_stepping:
@@ -1709,13 +1726,20 @@ cdef _start_method_event(code, instruction_offset):
         # threads may still want it...
         return
 
-    frame = _getframe(1)
-    func_code_info = _get_func_code_info(code, frame)
+    # Note: passing depth=1 instead of the frame avoids creating a frame reference
+    # unnecessarily in the fast path (cache hit). The frame will only be obtained
+    # inside _get_func_code_info if needed (cache miss).
+    func_code_info = _get_func_code_info(code, 1)
     if func_code_info.always_skip_code:
         # if DEBUG:
         #     print('disable (always skip)')
         return monitor.DISABLE
 
+    # Only get the frame when we actually need it (after the always_skip_code check).
+    # Obtaining a frame creates a reference that keeps all objects in that frame
+    # alive until this callback returns.
+    frame = _getframe(1)
+
     keep_enabled: bool = _enable_code_tracing(py_db, thread_info.additional_info, func_code_info, code, frame, True)
 
     if func_code_info.function_breakpoint_found:

src/debugpy/_vendored/pydevd/tests_python/test_sys_monitoring.py

@@ -292,3 +292,61 @@ def b():  #  line 4
 
     method()
     monitor.set_events(DEBUGGER_ID, 0)
+
+
+def test_frame_references_not_held(with_monitoring):
+    """
+    Test that sys.monitoring callbacks don't keep frame references alive
+    longer than necessary, which would prevent garbage collection of objects
+    referenced by locals in those frames.
+
+    See: https://github.com/microsoft/debugpy/issues/1813
+    """
+    import gc
+    import weakref
+
+    class _TestObj:
+        """Object that should be promptly garbage collected."""
+
+        def __init__(self, name):
+            self.name = name
+
+    refs = []
+
+    def _start_method(code, offset):
+        # Gets frame only if needed (simulating the improved pattern).
+        frame = sys._getframe(1)
+        if "test_sys_monitoring" in code.co_filename:
+            monitor.set_local_events(DEBUGGER_ID, code, monitor.events.PY_RETURN)
+
+    def _return_method(code, offset, retval):
+        # In the fixed code, the frame is only obtained when step_cmd != -1.
+        # Here we simulate the common path where no stepping is happening.
+        pass  # No frame obtained when not needed
+
+    monitor.set_events(DEBUGGER_ID, monitor.events.PY_START)
+    monitor.register_callback(DEBUGGER_ID, monitor.events.PY_START, _start_method)
+    monitor.register_callback(DEBUGGER_ID, monitor.events.PY_RETURN, _return_method)
+
+    def create_obj(name):
+        obj = _TestObj(name)
+        refs.append(weakref.ref(obj))
+        return 42  # obj should be freed when this function returns
+
+    # Create several objects
+    for i in range(10):
+        create_obj(f"obj_{i}")
+
+    # Force garbage collection
+    gc.collect()
+    gc.collect()
+    gc.collect()
+
+    # All objects should have been freed
+    alive = sum(1 for ref in refs if ref() is not None)
+    assert alive == 0, (
+        f"Expected 0 objects alive, but {alive}/{len(refs)} are still alive. "
+        f"Frame references may be keeping objects alive."
+    )
+
+    monitor.set_events(DEBUGGER_ID, 0)