diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 6b1529aa173f01c..2aef1d2c4537a57 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_CHAIN_DEPTH from _remote_debugging.h) + FRAME_CHAIN_DEPTH = 1024 + 512 + 1 + TASK_WAITER_CHAIN_DEPTH = 256 + 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_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_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..dfc05a5d428ceca --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-08-22-18-04.gh-issue-153364.JBFHEg.rst @@ -0,0 +1,3 @@ +Add limits to frame, coroutine and task-waiter walks in the Tachyon sampling +profiler. This avoids 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..8a0cca92441fcd4 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_CHAIN_DEPTH 256 /* Task waiter walk recursion cap: 256 * SIZEOF_TASK_OBJ ~= 1 MiB */ #ifndef MAX #define MAX(a, b) ((a) > (b) ? (a) : (b)) @@ -481,7 +483,8 @@ typedef int (*thread_processor_func)( typedef int (*set_entry_processor_func)( RemoteUnwinderObject *unwinder, uintptr_t key_addr, - void *context + void *context, + size_t depth ); typedef int (*interpreter_processor_func)( @@ -732,7 +735,8 @@ extern int parse_task( extern int parse_coro_chain( RemoteUnwinderObject *unwinder, uintptr_t coro_address, - PyObject *render_to + PyObject *render_to, + size_t depth ); extern int parse_async_frame_chain( @@ -747,7 +751,8 @@ extern int iterate_set_entries( RemoteUnwinderObject *unwinder, uintptr_t set_addr, set_entry_processor_func processor, - void *context + void *context, + size_t depth ); /* Task awaited_by processing */ @@ -755,7 +760,8 @@ extern int process_task_awaited_by( RemoteUnwinderObject *unwinder, uintptr_t task_address, set_entry_processor_func processor, - void *context + void *context, + size_t depth ); extern int process_single_task_node( @@ -768,7 +774,8 @@ extern int process_single_task_node( extern int process_task_and_waiters( RemoteUnwinderObject *unwinder, uintptr_t task_addr, - PyObject *result + PyObject *result, + size_t depth ); extern int find_running_task_in_thread( diff --git a/Modules/_remote_debugging/asyncio.c b/Modules/_remote_debugging/asyncio.c index 44a9a3cbce0061a..352edb8cb982d3e 100644 --- a/Modules/_remote_debugging/asyncio.c +++ b/Modules/_remote_debugging/asyncio.c @@ -121,7 +121,8 @@ iterate_set_entries( RemoteUnwinderObject *unwinder, uintptr_t set_addr, set_entry_processor_func processor, - void *context + void *context, + size_t depth ) { char set_object[SIZEOF_SET_OBJ]; if (_Py_RemoteDebug_PagedReadRemoteMemory(&unwinder->handle, set_addr, @@ -161,7 +162,7 @@ iterate_set_entries( if (ref_cnt) { // Process this valid set entry - if (processor(unwinder, key_addr, context) < 0) { + if (processor(unwinder, key_addr, context, depth) < 0) { return -1; } els++; @@ -252,7 +253,8 @@ handle_yield_from_frame( RemoteUnwinderObject *unwinder, uintptr_t gi_iframe_addr, uintptr_t gen_type_addr, - PyObject *render_to + PyObject *render_to, + size_t depth ) { // Read the entire interpreter frame at once char iframe[SIZEOF_INTERP_FRAME]; @@ -309,7 +311,8 @@ 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); + err = parse_coro_chain(unwinder, gi_await_addr, render_to, + depth + 1); if (err) { set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse coroutine chain in yield_from"); return -1; @@ -325,10 +328,19 @@ int parse_coro_chain( RemoteUnwinderObject *unwinder, uintptr_t coro_address, - PyObject *render_to + PyObject *render_to, + size_t depth ) { assert((void*)coro_address != NULL); + 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; + } + // Read the entire generator object at once char gen_object[SIZEOF_GEN_OBJ]; int err = _Py_RemoteDebug_PagedReadRemoteMemory( @@ -371,7 +383,8 @@ parse_coro_chain( Py_DECREF(name); if (frame_state == FRAME_SUSPENDED_YIELD_FROM) { - return handle_yield_from_frame(unwinder, gi_iframe_addr, gen_type_addr, render_to); + return handle_yield_from_frame(unwinder, gi_iframe_addr, gen_type_addr, + render_to, depth); } return 0; @@ -417,7 +430,7 @@ create_task_result( coro_addr = GET_MEMBER_NO_TAG(uintptr_t, task_obj, unwinder->async_debug_offsets.asyncio_task_object.task_coro); if ((void*)coro_addr != NULL) { - if (parse_coro_chain(unwinder, coro_addr, call_stack) < 0) { + if (parse_coro_chain(unwinder, coro_addr, call_stack, 0) < 0) { set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse coroutine chain"); goto error; } @@ -514,15 +527,22 @@ parse_task( * ============================================================================ */ // Forward declaration for mutual recursion -static int process_waiter_task(RemoteUnwinderObject *unwinder, uintptr_t key_addr, void *context); +static int process_waiter_task( + RemoteUnwinderObject *unwinder, + uintptr_t key_addr, + void *context, + size_t depth +); // Processor function for parsing tasks in sets static int process_task_parser( RemoteUnwinderObject *unwinder, uintptr_t key_addr, - void *context + void *context, + size_t depth ) { + (void)depth; PyObject *awaited_by = (PyObject *)context; return parse_task(unwinder, key_addr, awaited_by); } @@ -533,7 +553,8 @@ parse_task_awaited_by( uintptr_t task_address, PyObject *awaited_by ) { - return process_task_awaited_by(unwinder, task_address, process_task_parser, awaited_by); + return process_task_awaited_by( + unwinder, task_address, process_task_parser, awaited_by, 0); } int @@ -541,7 +562,8 @@ process_task_awaited_by( RemoteUnwinderObject *unwinder, uintptr_t task_address, set_entry_processor_func processor, - void *context + void *context, + size_t depth ) { // Read the entire TaskObj at once char task_obj[SIZEOF_TASK_OBJ]; @@ -560,10 +582,11 @@ 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, processor, context, depth); } else { // Single task waiting - return processor(unwinder, task_ab_addr, context); + return processor(unwinder, task_ab_addr, context, depth); } } @@ -662,15 +685,25 @@ int process_task_and_waiters( RemoteUnwinderObject *unwinder, uintptr_t task_addr, - PyObject *result + PyObject *result, + size_t depth ) { + if (depth >= MAX_TASK_WAITER_CHAIN_DEPTH) { + PyErr_SetString(PyExc_RuntimeError, + "Too many task waiters (possible infinite loop)"); + set_exception_cause(unwinder, PyExc_RuntimeError, + "Task waiter chain depth limit exceeded"); + return -1; + } + // First, add this task to the result if (process_single_task_node(unwinder, task_addr, 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); + return process_task_awaited_by( + unwinder, task_addr, process_waiter_task, result, depth + 1); } // Processor function for task waiters @@ -678,10 +711,10 @@ static int process_waiter_task( RemoteUnwinderObject *unwinder, uintptr_t key_addr, - void *context + void *context, + size_t depth ) { - PyObject *result = (PyObject *)context; - return process_task_and_waiters(unwinder, key_addr, result); + return process_task_and_waiters(unwinder, key_addr, (PyObject *)context, depth); } /* ============================================================================ @@ -776,7 +809,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 +1017,9 @@ 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_awaited_by( + unwinder, running_task_addr, process_waiter_task, result, 1) < 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) {