Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 159 additions & 34 deletions Lib/test/test_external_inspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -3804,5 +3804,130 @@ 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."""

CHAIN_DEPTH = 1024 + 512 + 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.CHAIN_DEPTH * 2})

def recurse(n):
if n <= 0:
sock.sendall(b"ready")
sock.recv(16)
return
recurse(n - 1)

recurse({self.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_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.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.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.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.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()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add limits to frame and coroutine-chain walks in the Tachyon sampling
profiler. This avoids potential hangs. Patch by Maurycy Pawłowski-Wieroński.
4 changes: 3 additions & 1 deletion Modules/_remote_debugging/_remote_debugging.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ 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 */

@maurycy maurycy Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the scope but maybe MAX_THREADS, MAX_STACK_CHUNKS, MAX_LINETABLE_ENTRIES or MAX_ITERATIONS are worth keeping here too


#ifndef MAX
#define MAX(a, b) ((a) > (b) ? (a) : (b))
Expand Down Expand Up @@ -732,7 +733,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(
Expand Down
28 changes: 23 additions & 5 deletions Modules/_remote_debugging/asyncio.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,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];
Expand Down Expand Up @@ -309,7 +310,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;
Expand All @@ -325,10 +327,19 @@ int
parse_coro_chain(

@maurycy maurycy Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it actually need to be recursive?

To put aside recursive v. iterative; CS101 strikes: if you ask me, I was thinking about a visited set here... not applicable to this fix but cycle detected would be such a nice touch. :-)

@maurycy maurycy Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized we used Floyd's algo at least once:

PyObject *slow_o = o; /* Floyd's cycle detection algo */

I, for one, like Brent's :-)

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(
Expand Down Expand Up @@ -371,7 +382,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;
Expand Down Expand Up @@ -417,7 +429,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;
}
Expand Down Expand Up @@ -776,7 +788,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(
Expand Down
6 changes: 2 additions & 4 deletions Modules/_remote_debugging/frames.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Loading