diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 6b1529aa173f01c..6439bc5c29539bc 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -339,6 +339,40 @@ def _run_script_and_get_trace( finally: _cleanup_sockets(client_socket, server_socket) + @contextmanager + def _target_process(self, script_body): + """Context manager for running a target process with socket sync.""" + port = find_unused_port() + script = f"""\ +import socket +sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +sock.connect(('localhost', {port})) +{textwrap.dedent(script_body)} +""" + + with os_helper.temp_dir() as work_dir: + script_dir = os.path.join(work_dir, "script_pkg") + os.mkdir(script_dir) + + server_socket = _create_server_socket(port) + script_name = _make_test_script(script_dir, "script", script) + client_socket = None + + try: + with _managed_subprocess([sys.executable, script_name]) as p: + client_socket, _ = server_socket.accept() + server_socket.close() + server_socket = None + + def make_unwinder(cache_frames=True): + return RemoteUnwinder( + p.pid, all_threads=True, cache_frames=cache_frames + ) + + yield p, client_socket, make_unwinder + finally: + _cleanup_sockets(client_socket, server_socket) + def _find_frame_in_trace(self, stack_trace, predicate): """ Find a frame matching predicate in stack trace. @@ -2927,40 +2961,6 @@ class TestFrameCaching(RemoteInspectionTestBase): All tests verify cache reuse via object identity checks (assertIs). """ - @contextmanager - def _target_process(self, script_body): - """Context manager for running a target process with socket sync.""" - port = find_unused_port() - script = f"""\ -import socket -sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) -sock.connect(('localhost', {port})) -{textwrap.dedent(script_body)} -""" - - with os_helper.temp_dir() as work_dir: - script_dir = os.path.join(work_dir, "script_pkg") - os.mkdir(script_dir) - - server_socket = _create_server_socket(port) - script_name = _make_test_script(script_dir, "script", script) - client_socket = None - - try: - with _managed_subprocess([sys.executable, script_name]) as p: - client_socket, _ = server_socket.accept() - server_socket.close() - server_socket = None - - def make_unwinder(cache_frames=True): - return RemoteUnwinder( - p.pid, all_threads=True, cache_frames=cache_frames - ) - - yield p, client_socket, make_unwinder - finally: - _cleanup_sockets(client_socket, server_socket) - def _get_frames_with_retry(self, unwinder, required_funcs): """Get frames containing required_funcs, with retry for transient errors.""" for _ in range(MAX_TRIES): @@ -3804,5 +3804,163 @@ def test_get_stats_disabled_raises(self): client_socket.sendall(b"done") +@requires_remote_subprocess_debugging() +class TestFrameChainLimits(RemoteInspectionTestBase): + """Frame chain walks abort instead of looping/overflowing on deep chains.""" + + # Limits plus one, to exceed them (must match MAX_FRAME_CHAIN_DEPTH / + # MAX_TASK_WAITER_WALK_TASKS from _remote_debugging.h) + FRAME_CHAIN_DEPTH = 1024 + 512 + 1 + TASK_WAITER_WALK_TASKS = 2**16 + 1 + + def _assert_unwinder_limit_error(self, unwind, expected_substring): + """Call unwind() until it raises the frame chain limit error. + + unwind must construct the RemoteUnwinder and call it, so that + transient RuntimeErrors from either step are retried; a successful + call means the limit never triggered and fails immediately. + """ + last_error = None + for _ in busy_retry(SHORT_TIMEOUT, error=False): + try: + unwind() + except TRANSIENT_ERRORS as e: + if expected_substring in str(e): + return + last_error = e + continue + self.fail( + "frame chain limit did not trigger; call returned a result" + ) + self.fail( + f"frame chain limit never raised; last transient error: " + f"{last_error!r}" + ) + + @skip_if_not_supported + @unittest.skipIf( + sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED, + "Test only runs on Linux with process_vm_readv support", + ) + def test_get_stack_trace_deep_frame_chain_aborts(self): + """Test that a frame chain deeper than the limit aborts the + synchronous stack walk instead of walking it indefinitely.""" + script_body = f"""\ + import sys + sys.setrecursionlimit({self.FRAME_CHAIN_DEPTH * 2}) + + def recurse(n): + if n <= 0: + sock.sendall(b"ready") + sock.recv(16) + return + recurse(n - 1) + + recurse({self.FRAME_CHAIN_DEPTH}) + """ + with self._target_process(script_body) as (p, client_socket, _): + _wait_for_signal(client_socket, b"ready") + self._assert_unwinder_limit_error( + lambda: RemoteUnwinder(p.pid).get_stack_trace(), + "Too many stack frames", + ) + client_socket.sendall(b"done") + + @skip_if_not_supported + @unittest.skipIf( + sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED, + "Test only runs on Linux with process_vm_readv support", + ) + def test_get_async_stack_trace_deep_task_waiter_chain_aborts(self): + """Test that a task waiter chain deeper than the limit aborts + the walk instead of overflowing the C stack.""" + script_body = f"""\ + import asyncio + + async def chain(n): + if n <= 0: + sock.sendall(b"ready") + sock.recv(16) + return + + task = asyncio.create_task(chain(n - 1)) + await task + + asyncio.run(chain({self.TASK_WAITER_WALK_TASKS})) + """ + with self._target_process(script_body) as (p, client_socket, _): + _wait_for_signal(client_socket, b"ready") + self._assert_unwinder_limit_error( + lambda: RemoteUnwinder(p.pid).get_async_stack_trace(), + "Too many task waiters", + ) + client_socket.sendall(b"done") + + @skip_if_not_supported + @unittest.skipIf( + sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED, + "Test only runs on Linux with process_vm_readv support", + ) + def test_get_async_stack_trace_deep_frame_chain_aborts(self): + """Test that a frame chain deeper than the limit aborts the async + stack walk instead of walking it indefinitely.""" + script_body = f"""\ + import sys, asyncio + sys.setrecursionlimit({self.FRAME_CHAIN_DEPTH * 2}) + + def recurse(n): + if n <= 0: + sock.sendall(b"ready") + sock.recv(16) + return + recurse(n - 1) + + async def deep(): + recurse({self.FRAME_CHAIN_DEPTH}) + + asyncio.run(deep()) + """ + with self._target_process(script_body) as (p, client_socket, _): + _wait_for_signal(client_socket, b"ready") + self._assert_unwinder_limit_error( + lambda: RemoteUnwinder(p.pid).get_async_stack_trace(), + "Too many async stack frames", + ) + client_socket.sendall(b"done") + + @skip_if_not_supported + @unittest.skipIf( + sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED, + "Test only runs on Linux with process_vm_readv support", + ) + def test_get_all_awaited_by_deep_coro_chain_aborts(self): + """Test that a coroutine await chain deeper than the limit aborts + the walk instead of overflowing the C stack.""" + script_body = f"""\ + import sys, asyncio + sys.setrecursionlimit({self.FRAME_CHAIN_DEPTH * 2}) + + async def chain(n): + if n <= 0: + await asyncio.sleep(10_000) + return + await chain(n - 1) + + async def main(): + task = asyncio.create_task(chain({self.FRAME_CHAIN_DEPTH})) + await asyncio.sleep(0) + sock.sendall(b"ready") + await task + + asyncio.run(main()) + """ + with self._target_process(script_body) as (p, client_socket, _): + _wait_for_signal(client_socket, b"ready") + self._assert_unwinder_limit_error( + lambda: RemoteUnwinder(p.pid).get_all_awaited_by(), + "Too many coroutine frames", + ) + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-07-08-22-18-04.gh-issue-153364.JBFHEg.rst b/Misc/NEWS.d/next/Library/2026-07-08-22-18-04.gh-issue-153364.JBFHEg.rst new file mode 100644 index 000000000000000..546cd9e1c884dee --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-08-22-18-04.gh-issue-153364.JBFHEg.rst @@ -0,0 +1,2 @@ +Make frame, coroutine and task-waiter walks iterative and bounded, avoiding +potential hangs and stack overflows. Patch by Maurycy Pawłowski-Wieroński. diff --git a/Modules/_remote_debugging/_remote_debugging.h b/Modules/_remote_debugging/_remote_debugging.h index fa37fb7b2167ecf..115de983854c0af 100644 --- a/Modules/_remote_debugging/_remote_debugging.h +++ b/Modules/_remote_debugging/_remote_debugging.h @@ -147,6 +147,8 @@ typedef enum _WIN32_THREADSTATE { #define MAX_STACK_CHUNK_SIZE (16 * 1024 * 1024) /* 16 MB max for stack chunks */ #define MAX_LONG_DIGITS 64 /* Allows values up to ~2^1920 */ #define MAX_SET_TABLE_SIZE (1 << 20) /* 1 million entries max for set iteration */ +#define MAX_FRAME_CHAIN_DEPTH (1024 + 512) /* Iteration bound for frame chain walks */ +#define MAX_TASK_WAITER_WALK_TASKS (1 << 16) /* Total-task bound for waiter walks */ #ifndef MAX #define MAX(a, b) ((a) > (b) ? (a) : (b)) @@ -478,12 +480,6 @@ typedef int (*thread_processor_func)( void *context ); -typedef int (*set_entry_processor_func)( - RemoteUnwinderObject *unwinder, - uintptr_t key_addr, - void *context -); - typedef int (*interpreter_processor_func)( RuntimeOffsets *offsets, uintptr_t interpreter_state_addr, @@ -729,12 +725,6 @@ extern int parse_task( PyObject *render_to ); -extern int parse_coro_chain( - RemoteUnwinderObject *unwinder, - uintptr_t coro_address, - PyObject *render_to -); - extern int parse_async_frame_chain( RemoteUnwinderObject *unwinder, PyObject *calls, @@ -742,22 +732,6 @@ extern int parse_async_frame_chain( uintptr_t running_task_code_obj ); -/* Set iteration */ -extern int iterate_set_entries( - RemoteUnwinderObject *unwinder, - uintptr_t set_addr, - set_entry_processor_func processor, - void *context -); - -/* Task awaited_by processing */ -extern int process_task_awaited_by( - RemoteUnwinderObject *unwinder, - uintptr_t task_address, - set_entry_processor_func processor, - void *context -); - extern int process_single_task_node( RemoteUnwinderObject *unwinder, uintptr_t task_addr, @@ -765,12 +739,6 @@ extern int process_single_task_node( PyObject *result ); -extern int process_task_and_waiters( - RemoteUnwinderObject *unwinder, - uintptr_t task_addr, - PyObject *result -); - extern int find_running_task_in_thread( RemoteUnwinderObject *unwinder, uintptr_t thread_state_addr, diff --git a/Modules/_remote_debugging/asyncio.c b/Modules/_remote_debugging/asyncio.c index 44a9a3cbce0061a..63c53fe58775569 100644 --- a/Modules/_remote_debugging/asyncio.c +++ b/Modules/_remote_debugging/asyncio.c @@ -116,12 +116,11 @@ ensure_async_debug_offsets(RemoteUnwinderObject *unwinder) * SET ITERATION FUNCTIONS * ============================================================================ */ -int +static int iterate_set_entries( RemoteUnwinderObject *unwinder, uintptr_t set_addr, - set_entry_processor_func processor, - void *context + PyObject *awaited_by ) { char set_object[SIZEOF_SET_OBJ]; if (_Py_RemoteDebug_PagedReadRemoteMemory(&unwinder->handle, set_addr, @@ -153,19 +152,10 @@ iterate_set_entries( } if ((void*)key_addr != NULL) { - Py_ssize_t ref_cnt; - if (read_Py_ssize_t(unwinder, table_ptr, &ref_cnt) < 0) { - set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read set entry ref count"); + if (parse_task(unwinder, key_addr, awaited_by) < 0) { return -1; } - - if (ref_cnt) { - // Process this valid set entry - if (processor(unwinder, key_addr, context) < 0) { - return -1; - } - els++; - } + els++; } table_ptr += sizeof(void*) * 2; i++; @@ -248,12 +238,14 @@ parse_task_name( * ============================================================================ */ static int -handle_yield_from_frame( +get_awaited_coro_address( RemoteUnwinderObject *unwinder, uintptr_t gi_iframe_addr, uintptr_t gen_type_addr, - PyObject *render_to + uintptr_t *next_coro ) { + *next_coro = 0; + // Read the entire interpreter frame at once char iframe[SIZEOF_INTERP_FRAME]; int err = _Py_RemoteDebug_PagedReadRemoteMemory( @@ -309,11 +301,7 @@ handle_yield_from_frame( doesn't match the type of whatever it points to in its cr_await. */ - err = parse_coro_chain(unwinder, gi_await_addr, render_to); - if (err) { - set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse coroutine chain in yield_from"); - return -1; - } + *next_coro = gi_await_addr; } } } @@ -321,7 +309,7 @@ handle_yield_from_frame( return 0; } -int +static int parse_coro_chain( RemoteUnwinderObject *unwinder, uintptr_t coro_address, @@ -329,49 +317,64 @@ parse_coro_chain( ) { assert((void*)coro_address != NULL); - // Read the entire generator object at once - char gen_object[SIZEOF_GEN_OBJ]; - int err = _Py_RemoteDebug_PagedReadRemoteMemory( - &unwinder->handle, - coro_address, - SIZEOF_GEN_OBJ, - gen_object); - if (err < 0) { - set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read generator object in coro chain"); - return -1; - } + for (size_t depth = 0; (void*)coro_address != NULL; depth++) { + if (depth >= MAX_FRAME_CHAIN_DEPTH) { + PyErr_SetString(PyExc_RuntimeError, + "Too many coroutine frames (possible infinite loop)"); + set_exception_cause(unwinder, PyExc_RuntimeError, + "Coroutine chain depth limit exceeded"); + return -1; + } - int8_t frame_state = GET_MEMBER(int8_t, gen_object, unwinder->debug_offsets.gen_object.gi_frame_state); - if (frame_state == FRAME_CLEARED) { - return 0; - } + // Read the entire generator object at once + char gen_object[SIZEOF_GEN_OBJ]; + int err = _Py_RemoteDebug_PagedReadRemoteMemory( + &unwinder->handle, + coro_address, + SIZEOF_GEN_OBJ, + gen_object); + if (err < 0) { + set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read generator object in coro chain"); + return -1; + } - uintptr_t gen_type_addr = GET_MEMBER(uintptr_t, gen_object, unwinder->debug_offsets.pyobject.ob_type); + int8_t frame_state = GET_MEMBER(int8_t, gen_object, unwinder->debug_offsets.gen_object.gi_frame_state); + if (frame_state == FRAME_CLEARED) { + return 0; + } - PyObject* name = NULL; + uintptr_t gen_type_addr = GET_MEMBER(uintptr_t, gen_object, unwinder->debug_offsets.pyobject.ob_type); - // Parse the previous frame using the gi_iframe from local copy - uintptr_t prev_frame; - uintptr_t gi_iframe_addr = coro_address + (uintptr_t)unwinder->debug_offsets.gen_object.gi_iframe; - uintptr_t address_of_code_object = 0; - if (parse_frame_object(unwinder, &name, gi_iframe_addr, &address_of_code_object, &prev_frame) < 0) { - set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse frame object in coro chain"); - return -1; - } + PyObject* name = NULL; - if (!name) { - return 0; - } + // Parse the previous frame using the gi_iframe from local copy + uintptr_t prev_frame; + uintptr_t gi_iframe_addr = coro_address + (uintptr_t)unwinder->debug_offsets.gen_object.gi_iframe; + uintptr_t address_of_code_object = 0; + if (parse_frame_object(unwinder, &name, gi_iframe_addr, &address_of_code_object, &prev_frame) < 0) { + set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse frame object in coro chain"); + return -1; + } - if (PyList_Append(render_to, name)) { + if (!name) { + return 0; + } + + if (PyList_Append(render_to, name)) { + Py_DECREF(name); + set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to append frame to coro chain"); + return -1; + } Py_DECREF(name); - set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to append frame to coro chain"); - return -1; - } - Py_DECREF(name); - if (frame_state == FRAME_SUSPENDED_YIELD_FROM) { - return handle_yield_from_frame(unwinder, gi_iframe_addr, gen_type_addr, render_to); + if (frame_state != FRAME_SUSPENDED_YIELD_FROM) { + return 0; + } + + if (get_awaited_coro_address(unwinder, gi_iframe_addr, gen_type_addr, + &coro_address) < 0) { + return -1; + } } return 0; @@ -513,35 +516,11 @@ parse_task( * TASK AWAITED_BY PROCESSING * ============================================================================ */ -// Forward declaration for mutual recursion -static int process_waiter_task(RemoteUnwinderObject *unwinder, uintptr_t key_addr, void *context); - -// Processor function for parsing tasks in sets -static int -process_task_parser( - RemoteUnwinderObject *unwinder, - uintptr_t key_addr, - void *context -) { - PyObject *awaited_by = (PyObject *)context; - return parse_task(unwinder, key_addr, awaited_by); -} - static int parse_task_awaited_by( RemoteUnwinderObject *unwinder, uintptr_t task_address, PyObject *awaited_by -) { - return process_task_awaited_by(unwinder, task_address, process_task_parser, awaited_by); -} - -int -process_task_awaited_by( - RemoteUnwinderObject *unwinder, - uintptr_t task_address, - set_entry_processor_func processor, - void *context ) { // Read the entire TaskObj at once char task_obj[SIZEOF_TASK_OBJ]; @@ -560,10 +539,10 @@ process_task_awaited_by( char awaited_by_is_a_set = GET_MEMBER(char, task_obj, unwinder->async_debug_offsets.asyncio_task_object.task_awaited_by_is_set); if (awaited_by_is_a_set) { - return iterate_set_entries(unwinder, task_ab_addr, processor, context); + return iterate_set_entries(unwinder, task_ab_addr, awaited_by); } else { // Single task waiting - return processor(unwinder, task_ab_addr, context); + return parse_task(unwinder, task_ab_addr, awaited_by); } } @@ -658,30 +637,39 @@ process_single_task_node( return -1; } -int -process_task_and_waiters( +static int +process_task_waiters( RemoteUnwinderObject *unwinder, - uintptr_t task_addr, PyObject *result ) { - // First, add this task to the result - if (process_single_task_node(unwinder, task_addr, NULL, result) < 0) { - return -1; + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(result); i++) { + PyObject *task_info = PyList_GET_ITEM(result, i); + PyObject *waiters = PyStructSequence_GET_ITEM(task_info, 3); + for (Py_ssize_t j = 0; j < PyList_GET_SIZE(waiters); j++) { + if (PyList_GET_SIZE(result) >= MAX_TASK_WAITER_WALK_TASKS) { + PyErr_SetString(PyExc_RuntimeError, + "Too many task waiters (possible infinite loop)"); + set_exception_cause(unwinder, PyExc_RuntimeError, + "Task waiter walk size limit exceeded"); + return -1; + } + PyObject *waiter = PyList_GET_ITEM(waiters, j); + PyObject *task_id = PyStructSequence_GET_ITEM(waiter, 1); + void *task_ptr = PyLong_AsVoidPtr(task_id); + if (task_ptr == NULL && PyErr_Occurred()) { + set_exception_cause(unwinder, PyExc_RuntimeError, + "Failed to parse waiter task ID"); + return -1; + } + if (process_single_task_node( + unwinder, (uintptr_t)task_ptr, NULL, result) < 0) + { + return -1; + } + } } - // Now find all tasks that are waiting for this task and process them - return process_task_awaited_by(unwinder, task_addr, process_waiter_task, result); -} - -// Processor function for task waiters -static int -process_waiter_task( - RemoteUnwinderObject *unwinder, - uintptr_t key_addr, - void *context -) { - PyObject *result = (PyObject *)context; - return process_task_and_waiters(unwinder, key_addr, result); + return 0; } /* ============================================================================ @@ -776,7 +764,13 @@ parse_async_frame_chain( return -1; } + size_t frame_count = 0; while ((void*)address_of_current_frame != NULL) { + if (++frame_count > MAX_FRAME_CHAIN_DEPTH) { + PyErr_SetString(PyExc_RuntimeError, "Too many async stack frames (possible infinite loop)"); + set_exception_cause(unwinder, PyExc_RuntimeError, "Async frame chain iteration limit exceeded"); + return -1; + } PyObject* frame_info = NULL; uintptr_t address_of_code_object; int res = parse_frame_object( @@ -978,7 +972,7 @@ process_running_task_chain( } // Now find all tasks that are waiting for this task and process them - if (process_task_awaited_by(unwinder, running_task_addr, process_waiter_task, result) < 0) { + if (process_task_waiters(unwinder, result) < 0) { return -1; } diff --git a/Modules/_remote_debugging/frames.c b/Modules/_remote_debugging/frames.c index 46968acc6ff1feb..fde3474a0539190 100644 --- a/Modules/_remote_debugging/frames.c +++ b/Modules/_remote_debugging/frames.c @@ -305,9 +305,7 @@ process_frame_chain( uintptr_t frame_addr = ctx->frame_addr; uintptr_t prev_frame_addr = 0; uintptr_t last_frame_addr = 0; - const size_t MAX_FRAMES = 1024 + 512; size_t frame_count = 0; - assert(MAX_FRAMES > 0 && MAX_FRAMES < 10000); ctx->stopped_at_cached_frame = 0; ctx->last_frame_visited = 0; @@ -318,12 +316,12 @@ process_frame_chain( uintptr_t stackpointer = 0; last_frame_addr = frame_addr; - if (++frame_count > MAX_FRAMES) { + if (++frame_count > MAX_FRAME_CHAIN_DEPTH) { PyErr_SetString(PyExc_RuntimeError, "Too many stack frames (possible infinite loop)"); set_exception_cause(unwinder, PyExc_RuntimeError, "Frame chain iteration limit exceeded"); return -1; } - assert(frame_count <= MAX_FRAMES); + assert(frame_count <= MAX_FRAME_CHAIN_DEPTH); if (ctx->chunks && ctx->chunks->count > 0) { if (parse_frame_from_chunks(unwinder, &frame, frame_addr, &next_frame_addr, &stackpointer, ctx->chunks) == 0) {