Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4dd6f49
let me declare single limit
maurycy Jul 8, 2026
e71aaa5
use our new limit in process_frame_chain()
maurycy Jul 8, 2026
116da45
add it in parse_async_frame_chain()
maurycy Jul 8, 2026
3e12939
parse_coro_chain()
maurycy Jul 8, 2026
c46810c
NEWS
maurycy Jul 8, 2026
dd6e19a
async in the message?
maurycy Jul 8, 2026
0972e00
test
maurycy Jul 8, 2026
3c5f78b
no race
maurycy Jul 8, 2026
78c18f9
process_task_awaited_by
maurycy Jul 10, 2026
21d764e
process_task_awaited_by limit test
maurycy Jul 10, 2026
000287e
NEWS
maurycy Jul 10, 2026
353f815
MAX_TASK_WAITER_CHAIN_DEPTH
maurycy Jul 10, 2026
f66866b
TASK_WAITER_CHAIN_DEPTH in test
maurycy Jul 10, 2026
c9f1ebd
TASK_WAITER_CHAIN_DEPTH 256
maurycy Jul 10, 2026
7236ca2
prevent the drift with the comment
maurycy Jul 11, 2026
35fc6c6
better naming, better style
maurycy Jul 11, 2026
10149e9
MAX_TASK_WAITER_CHAIN_DEPTH comment
maurycy Jul 11, 2026
ef51563
task-waiter iterative bfs walk
maurycy Jul 12, 2026
b5022ba
iterative coro-walk
maurycy Jul 13, 2026
73b8674
nicer news
maurycy Jul 13, 2026
e0607dc
1 << 14
maurycy Jul 25, 2026
ed731eb
comment
maurycy Jul 25, 2026
75a8d8e
unused read_Py_ssize_t
maurycy Jul 25, 2026
c0ce737
fix tombstones
maurycy Jul 25, 2026
f3c80d4
simplify
maurycy Jul 25, 2026
4fd4027
correct msg
maurycy Jul 25, 2026
f2530ec
better test
maurycy Jul 25, 2026
0ab3342
news for tombstones
maurycy Jul 25, 2026
2a444ec
left-over from when testing buggy version
maurycy Jul 25, 2026
ed95a2e
redundant new line
maurycy Jul 25, 2026
a1221fd
Merge branch 'main' into tachyon-parse_async_frame_chain-limit
maurycy Oct 4, 2026
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
Comment thread
maurycy marked this conversation as resolved.
Outdated

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.

Maybe MAX_THREADS, MAX_TLBC_SIZE, MAX_STACK_CHUNKS, MAX_LINETABLE_SIZE or MAX_ITERATIONS are worth keeping here too?

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.

While we're at it, should MAX_TLBC_SIZE be in sync with MAX_THREADS?


#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(
Comment thread
maurycy marked this conversation as resolved.
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) {
Comment thread
maurycy marked this conversation as resolved.
Outdated
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)");

@maurycy maurycy Jul 21, 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.

Generally, there's more interplay between this issue and #154194, since we could return <truncated> instead of hard failing. Or even introduce a degree of degradation or success. I don't believe we should creep the scope here yet, but I will follow-up.

Similarly:

if (!cache_tlbc_array(unwinder, real_address, real_address + unwinder->debug_offsets.code_object.co_tlbc, unwinder->tlbc_generation)) {
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to cache TLBC array");
goto error;
}

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