gh-153364: Limit frame, coroutine, and task-waiter chain walks

[maurycy]

The PR hardens and cleans up chain walks a bit.

It introduces a const (MAX_FRAME_CHAIN_DEPTH), uses it in the existing process_frame_chain(), which is called by get_stack_trace(), as well as adds it to parse_coro_chain(), used in both sync and async pathways, and to parse_async_frame_chain().

Also, it adds MAX_TASK_WAITER_CHAIN_DEPTH for the purpose of task-waiter limits in get_async_stack_trace().

The obvious context here is avoiding infinite loops. Truth be told, I think that in some places it unexpectedly works (ie: doesn't hang) because incomplete / torn-reads result in an exception. :-)

• Issue: _remote_debugging: No frame limit in get_async_stack_trace() #153364

[maurycy]

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]

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

cpython/Python/errors.c

Line 210 in 0c6422f

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

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

[maurycy]

Similar recursive problem here:

cpython/Modules/_remote_debugging/asyncio.c

Line 662 in a159c68

process_task_and_waiters(

[maurycy]

Wondering:

• https://github.com/python/cpython/actions/runs/29118077663/job/86446371827

0:04:10 load avg: 5.21 mem: 175.8 MiB [282/505/1] test_external_inspection worker non-zero exit code (Exit code 3221225725 (STATUS_STACK_OVERFLOW))

• https://github.com/python/cpython/actions/runs/29118077663/job/86446371829

0:04:10 load avg: 5.21 mem: 175.8 MiB [282/505/1] test_external_inspection worker non-zero exit code (Exit code 3221225725 (STATUS_STACK_OVERFLOW))

On one hand, it hints at iterative approach being the right now. On the other hand, I've just lowered the limit: 353f815 and, eventually, c9f1ebd

[maurycy]

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

[pablogsal]

This bounds how deep a single waiter chain can go, but not how many tasks we visit in total, no? awaited_by is a DAG when two tasks await the same task (and gather registers the gathering task in every child), and since we don't keep a visited set each shared waiter is re-walked once per path, appending duplicated TaskInfo entries. ~20 diamond levels of gather fan-in is already 2**20 visits without getting near the 256 limit, so the hang is still reachable. Maybe we should also cap the total number of processed tasks? I am fine doing that in a follow-up.

[pablogsal]

Unless I am missing something, parse_coro_chain recurses on the C stack via handle_yield_from_frame, so this allows 1536 nested C calls, each holding gen_object, iframe and parse_frame_object buffers. That is around 1 MB of sampler stack at the limit. You gave the task-waiter walk its own smaller constant for exactly this reason, no? Maybe the coroutine chain deserves a dedicated smaller limit too.

[pablogsal]

This can still overflow a 1 MiB stack: every recursive level keeps the 4 KiB task_obj alive, so 256 levels exhaust the stack before this check fires. We should walk the waiter graph iteratively.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.