From 2dbcf91f5edcb216fbc0f2cd2b6dfc35fee4c4e4 Mon Sep 17 00:00:00 2001 From: LucasZhou Date: Thu, 1 Oct 2026 16:38:52 -0500 Subject: [PATCH] gh-158539: Fix exception mode missing handlers in generators/coroutines The sampling profiler's exception mode decided whether a thread was handling an exception by reading the embedded PyThreadState.exc_state. Generators, coroutines and async generators repoint tstate->exc_info at their own _PyErr_StackItem while they run, so an except block running in one of them (or in a function they call) stored the exception in that item instead, and was never sampled. Follow tstate->exc_info and its previous_item chain, mirroring _PyErr_GetTopmostException(), and export the two debug offsets needed to walk the chain from remote memory. The common case where exc_info points at the embedded exc_state keeps the existing zero-extra-read fast path. --- Include/internal/pycore_debug_offsets.h | 4 + Lib/test/test_external_inspection.py | 395 ++++++++++++++++++ ...-10-01-14-00-00.gh-issue-158539.q1w2e3.rst | 5 + .../debug_offsets_validation.h | 9 +- Modules/_remote_debugging/threads.c | 63 ++- 5 files changed, 466 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-10-01-14-00-00.gh-issue-158539.q1w2e3.rst diff --git a/Include/internal/pycore_debug_offsets.h b/Include/internal/pycore_debug_offsets.h index 6e1eb573c8c2c83..93f0efd882cdc7e 100644 --- a/Include/internal/pycore_debug_offsets.h +++ b/Include/internal/pycore_debug_offsets.h @@ -113,11 +113,13 @@ typedef struct _Py_DebugOffsets { uint64_t gil_requested; uint64_t current_exception; uint64_t exc_state; + uint64_t exc_info; } thread_state; // Exception stack item offset struct { uint64_t exc_value; + uint64_t previous_item; } err_stackitem; // InterpreterFrame offset; @@ -304,9 +306,11 @@ typedef struct _Py_DebugOffsets { .gil_requested = offsetof(PyThreadState, gil_requested), \ .current_exception = offsetof(PyThreadState, current_exception), \ .exc_state = offsetof(PyThreadState, exc_state), \ + .exc_info = offsetof(PyThreadState, exc_info), \ }, \ .err_stackitem = { \ .exc_value = offsetof(_PyErr_StackItem, exc_value), \ + .previous_item = offsetof(_PyErr_StackItem, previous_item), \ }, \ .interpreter_frame = { \ .size = sizeof(_PyInterpreterFrame), \ diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index c83d2cb2abeac81..5921a66a71ef7cf 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -2935,6 +2935,23 @@ class TestExceptionDetectionScenarios(RemoteInspectionTestBase): 4. finally_no_exception: Finally block with no exception raised -> Should NOT have HAS_EXCEPTION (no exception state) + + 5. except_block_in_generator: Thread inside an except block running in a + generator + -> SHOULD have HAS_EXCEPTION (exc_info points at the generator's + _PyErr_StackItem, not the thread's embedded exc_state) + + 6. except_block_in_genexpr_callee: Except block in a function called from a + generator expression + -> SHOULD have HAS_EXCEPTION (same reason as 5) + + 7. except_block_in_coroutine: Thread inside an except block running in a + coroutine + -> SHOULD have HAS_EXCEPTION (same reason as 5) + + 8. except_block_in_callee_from_coroutine: Except block in a function called + from a coroutine + -> SHOULD have HAS_EXCEPTION (same reason as 5) """ def _make_single_scenario_script(self, port, scenario): @@ -3018,6 +3035,105 @@ def target_thread(): while True: time.sleep(0.01) +t = threading.Thread(target=target_thread) +t.start() +t.join() +""", + "except_block_in_generator": f"""\ +import socket +import threading +import time + +def target_thread(): + '''Inside except block that runs in a generator''' + conn = socket.create_connection(("localhost", {port})) + conn.sendall(b"ready:" + str(threading.get_native_id()).encode()) + + def gen(): + try: + raise ValueError("test") + except ValueError: + while True: + time.sleep(0.01) + yield + + for _ in gen(): + pass + +t = threading.Thread(target=target_thread) +t.start() +t.join() +""", + "except_block_in_genexpr_callee": f"""\ +import socket +import threading +import time + +def target_thread(): + '''Inside except block in a function called from a generator expression''' + conn = socket.create_connection(("localhost", {port})) + conn.sendall(b"ready:" + str(threading.get_native_id()).encode()) + + def callee(): + try: + raise ValueError("test") + except ValueError: + while True: + time.sleep(0.01) + + list(callee() for _ in range(1)) + +t = threading.Thread(target=target_thread) +t.start() +t.join() +""", + "except_block_in_coroutine": f"""\ +import asyncio +import socket +import threading +import time + +def target_thread(): + '''Inside except block that runs in a coroutine''' + conn = socket.create_connection(("localhost", {port})) + conn.sendall(b"ready:" + str(threading.get_native_id()).encode()) + + async def coro(): + try: + raise ValueError("test") + except ValueError: + while True: + time.sleep(0.01) + + asyncio.run(coro()) + +t = threading.Thread(target=target_thread) +t.start() +t.join() +""", + "except_block_in_callee_from_coroutine": f"""\ +import asyncio +import socket +import threading +import time + +def target_thread(): + '''Inside except block in a function called from a coroutine''' + conn = socket.create_connection(("localhost", {port})) + conn.sendall(b"ready:" + str(threading.get_native_id()).encode()) + + def callee(): + try: + raise ValueError("test") + except ValueError: + while True: + time.sleep(0.01) + + async def coro(): + callee() + + asyncio.run(coro()) + t = threading.Thread(target=target_thread) t.start() t.join() @@ -3181,6 +3297,285 @@ def test_finally_no_exception_no_flag(self): self.assertIsNotNone(thread_tid, "Thread ID not received") self._check_exception_status(p, thread_tid, expect_exception=False) + @unittest.skipIf( + sys.platform not in ("linux", "darwin", "win32"), + "Test only runs on supported platforms (Linux, macOS, or Windows)", + ) + @unittest.skipIf( + sys.platform == "android", "Android raises Linux-specific exception" + ) + def test_except_block_in_generator_has_exception(self): + """gh-158539: a handler running in a generator has HAS_EXCEPTION. + + Generators repoint ``tstate->exc_info`` at their own + ``_PyErr_StackItem``, so the embedded ``exc_state`` stays empty and the + profiler must follow ``exc_info`` to see the handled exception. + """ + with self._run_scenario_process("except_block_in_generator") as (p, thread_tid): + self.assertIsNotNone(thread_tid, "Thread ID not received") + self._check_exception_status(p, thread_tid, expect_exception=True) + + @unittest.skipIf( + sys.platform not in ("linux", "darwin", "win32"), + "Test only runs on supported platforms (Linux, macOS, or Windows)", + ) + @unittest.skipIf( + sys.platform == "android", "Android raises Linux-specific exception" + ) + def test_except_block_in_genexpr_callee_has_exception(self): + """gh-158539: a handler in a function called from a generator expression. + + The handler itself lives in an ordinary function, but the generator + expression on the stack means ``exc_info`` does not point at the + thread's embedded ``exc_state``. + """ + with self._run_scenario_process( + "except_block_in_genexpr_callee" + ) as (p, thread_tid): + self.assertIsNotNone(thread_tid, "Thread ID not received") + self._check_exception_status(p, thread_tid, expect_exception=True) + + @unittest.skipIf( + sys.platform not in ("linux", "darwin", "win32"), + "Test only runs on supported platforms (Linux, macOS, or Windows)", + ) + @unittest.skipIf( + sys.platform == "android", "Android raises Linux-specific exception" + ) + def test_except_block_in_coroutine_has_exception(self): + """gh-158539: a handler running in a coroutine has HAS_EXCEPTION.""" + with self._run_scenario_process("except_block_in_coroutine") as (p, thread_tid): + self.assertIsNotNone(thread_tid, "Thread ID not received") + self._check_exception_status(p, thread_tid, expect_exception=True) + + @unittest.skipIf( + sys.platform not in ("linux", "darwin", "win32"), + "Test only runs on supported platforms (Linux, macOS, or Windows)", + ) + @unittest.skipIf( + sys.platform == "android", "Android raises Linux-specific exception" + ) + def test_except_block_in_callee_from_coroutine_has_exception(self): + """gh-158539: a handler in a function called from a coroutine.""" + with self._run_scenario_process( + "except_block_in_callee_from_coroutine" + ) as (p, thread_tid): + self.assertIsNotNone(thread_tid, "Thread ID not received") + self._check_exception_status(p, thread_tid, expect_exception=True) + + +class TestExceptionDetectionInProcess(RemoteInspectionTestBase): + """gh-158539: HAS_EXCEPTION for handlers running in generators/coroutines. + + ``TestExceptionDetectionScenarios`` samples a child process and therefore + needs subprocess debugging permissions. These tests inspect the current + process with ``RemoteUnwinder`` and only need self-inspection, so they also + run on macOS without special entitlements. + """ + + @classmethod + def setUpClass(cls): + try: + RemoteUnwinder(os.getpid(), all_threads=True).get_stack_trace() + except Exception as exc: + raise unittest.SkipTest(f"self-inspection is unavailable: {exc}") + + def _check_running_handler( + self, target, expect_exception, *, mode=PROFILING_MODE_ALL, + skip_non_matching_threads=False, + ): + """Run *target* in a thread and check its HAS_EXCEPTION flag. + + *target* receives ``(ready, stop)`` events and must signal ``ready`` + only once it is executing inside the code region under test, then keep + running until ``stop`` is set. + """ + stop = threading.Event() + ready = threading.Event() + failure = [] + + def runner(): + try: + target(ready, stop) + except BaseException as exc: + failure.append(exc) + ready.set() + + thread = threading.Thread(target=runner, daemon=True) + thread.start() + try: + self.assertTrue(ready.wait(SHORT_TIMEOUT), "handler never started") + self.assertFalse(failure, f"handler raised {failure!r}") + + unwinder = RemoteUnwinder( + os.getpid(), + all_threads=True, + mode=mode, + skip_non_matching_threads=skip_non_matching_threads, + ) + observed = [] + for _ in busy_retry(SHORT_TIMEOUT): + with contextlib.suppress(*TRANSIENT_ERRORS): + statuses = self._get_thread_statuses(unwinder.get_stack_trace()) + status = statuses.get(thread.native_id) + if status is None: + continue + has_exception = bool(status & THREAD_STATUS_HAS_EXCEPTION) + observed.append(has_exception) + if has_exception == expect_exception: + break + self.assertTrue( + observed, "target thread status was never observed" + ) + self.assertIn( + expect_exception, + observed, + f"HAS_EXCEPTION was never {expect_exception} while the " + f"handler was running (observed {observed})", + ) + finally: + stop.set() + thread.join(SHORT_TIMEOUT) + + def _busy_until_stopped(self, ready, stop): + ready.set() + while not stop.is_set(): + time.sleep(0.001) + + def test_handler_in_function(self): + def target(ready, stop): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + + self._check_running_handler(target, expect_exception=True) + + def test_handler_in_generator(self): + def target(ready, stop): + def gen(): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + yield + + for _ in gen(): + pass + + self._check_running_handler(target, expect_exception=True) + + def test_handler_in_genexpr_callee(self): + def target(ready, stop): + def callee(): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + + list(callee() for _ in range(1)) + + self._check_running_handler(target, expect_exception=True) + + def test_handler_in_coroutine(self): + async def coro(ready, stop): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + + def target(ready, stop): + asyncio.run(coro(ready, stop)) + + self._check_running_handler(target, expect_exception=True) + + def test_handler_in_callee_from_coroutine(self): + def callee(ready, stop): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + + async def coro(ready, stop): + callee(ready, stop) + + def target(ready, stop): + asyncio.run(coro(ready, stop)) + + self._check_running_handler(target, expect_exception=True) + + def test_outer_handler_while_generator_runs(self): + """A generator with no handler of its own must not hide the outer one. + + ``exc_info`` points at the generator's empty ``_PyErr_StackItem`` whose + ``previous_item`` is the thread's ``exc_state``, so the profiler has to + walk the chain to find the exception ``sys.exception()`` reports. + """ + def target(ready, stop): + def gen(): + self._busy_until_stopped(ready, stop) + yield + + try: + raise ValueError("outer") + except ValueError: + for _ in gen(): + pass + + self._check_running_handler(target, expect_exception=True) + + def test_generator_without_exception(self): + def target(ready, stop): + def gen(): + self._busy_until_stopped(ready, stop) + yield + + for _ in gen(): + pass + + self._check_running_handler(target, expect_exception=False) + + def test_generator_finally_after_except(self): + """The handled exception is cleared before the generator's finally.""" + def target(ready, stop): + def gen(): + try: + raise ValueError("test") + except ValueError: + pass + finally: + self._busy_until_stopped(ready, stop) + yield + + for _ in gen(): + pass + + self._check_running_handler(target, expect_exception=False) + + def test_exception_mode_filter_keeps_generator_handler(self): + """The exception-mode thread filter must not drop a generator handler. + + This mirrors what ``--mode=exception`` actually does: threads without + HAS_EXCEPTION are skipped before their stack is unwound. + """ + def target(ready, stop): + def gen(): + try: + raise ValueError("test") + except ValueError: + self._busy_until_stopped(ready, stop) + yield + + for _ in gen(): + pass + + self._check_running_handler( + target, + expect_exception=True, + mode=PROFILING_MODE_EXCEPTION, + skip_non_matching_threads=True, + ) + @requires_remote_subprocess_debugging() class TestFrameCaching(RemoteInspectionTestBase): diff --git a/Misc/NEWS.d/next/Library/2026-10-01-14-00-00.gh-issue-158539.q1w2e3.rst b/Misc/NEWS.d/next/Library/2026-10-01-14-00-00.gh-issue-158539.q1w2e3.rst new file mode 100644 index 000000000000000..b1ab9c8e017fb26 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-01-14-00-00.gh-issue-158539.q1w2e3.rst @@ -0,0 +1,5 @@ +Fix :mod:`profiling.sampling` exception mode discarding samples for code +running inside an ``except`` block in a generator or coroutine. The remote +debugger now follows ``tstate->exc_info`` and its ``previous_item`` chain +instead of only reading the embedded ``exc_state``, matching the exception +that :func:`sys.exception` reports. diff --git a/Modules/_remote_debugging/debug_offsets_validation.h b/Modules/_remote_debugging/debug_offsets_validation.h index c0c01a0a639e196..5c1b3b18e2496a2 100644 --- a/Modules/_remote_debugging/debug_offsets_validation.h +++ b/Modules/_remote_debugging/debug_offsets_validation.h @@ -31,7 +31,7 @@ #define FIELD_SIZE(type, member) sizeof(((type *)0)->member) enum { - PY_REMOTE_DEBUG_OFFSETS_TOTAL_SIZE = 888, + PY_REMOTE_DEBUG_OFFSETS_TOTAL_SIZE = 904, PY_REMOTE_ASYNC_DEBUG_OFFSETS_TOTAL_SIZE = 104, }; @@ -257,6 +257,7 @@ validate_fixed_field( APPLY(thread_state, holds_gil, sizeof(int), _Alignof(int), buffer_size); \ APPLY(thread_state, gil_requested, sizeof(int), _Alignof(int), buffer_size); \ APPLY(thread_state, current_exception, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \ + APPLY(thread_state, exc_info, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \ APPLY(thread_state, thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \ APPLY(thread_state, next, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \ APPLY(thread_state, current_frame, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \ @@ -357,6 +358,12 @@ _PyRemoteDebug_ValidateDebugOffsetsLayout(struct _Py_DebugOffsets *debug_offsets sizeof(uintptr_t), _Alignof(uintptr_t), sizeof(_PyErr_StackItem)); + PY_REMOTE_DEBUG_VALIDATE_FIXED_FIELD( + err_stackitem, + previous_item, + sizeof(uintptr_t), + _Alignof(uintptr_t), + sizeof(_PyErr_StackItem)); PY_REMOTE_DEBUG_VALIDATE_NESTED_FIELD( thread_state, exc_state, diff --git a/Modules/_remote_debugging/threads.c b/Modules/_remote_debugging/threads.c index 04c70cc96d6bd1e..52f7b0767bab25c 100644 --- a/Modules/_remote_debugging/threads.c +++ b/Modules/_remote_debugging/threads.c @@ -17,6 +17,11 @@ #include #endif +/* Upper bound on how far the handled-exception chain (exc_info->previous_item) + * is followed in remote memory. The chain is normally at most a couple of + * entries deep; the bound only guards against corrupted memory. */ +#define MAX_EXCEPTION_CHAIN_DEPTH 16 + /* ============================================================================ * THREAD ITERATION FUNCTIONS * ============================================================================ */ @@ -436,16 +441,56 @@ unwind_stack_for_thread( has_exception = 1; } - // Check exc_state.exc_value (exception being handled in except block) - // exc_state is embedded in PyThreadState, so we read it directly from - // the thread state buffer. This catches most cases; nested exception - // handlers where exc_info points elsewhere are rare. + // Check the exception currently being handled by an except block. + // + // The active _PyErr_StackItem is normally exc_state, embedded in the + // thread state, but generators, coroutines and async generators repoint + // tstate->exc_info at their own _PyErr_StackItem while they run. Reading + // only the embedded exc_state therefore misses every handler that runs in + // a generator or coroutine, or in a function one of them calls. Follow + // exc_info and walk previous_item like _PyErr_GetTopmostException() so + // that an outer handler is still found while a generator without a handler + // of its own is running. if (!has_exception) { - uintptr_t exc_value = GET_MEMBER(uintptr_t, ts, - unwinder->debug_offsets.thread_state.exc_state + - unwinder->debug_offsets.err_stackitem.exc_value); - if (exc_value != 0) { - has_exception = 1; + uintptr_t exc_info = GET_MEMBER(uintptr_t, ts, + unwinder->debug_offsets.thread_state.exc_info); + uintptr_t exc_state_addr = + *current_tstate + unwinder->debug_offsets.thread_state.exc_state; + uintptr_t exc_value_offset = + unwinder->debug_offsets.err_stackitem.exc_value; + uintptr_t previous_item_offset = + unwinder->debug_offsets.err_stackitem.previous_item; + + for (int depth = 0; exc_info != 0 && depth < MAX_EXCEPTION_CHAIN_DEPTH; + depth++) + { + if (exc_info == exc_state_addr) { + // Bottom of the chain: the stack item embedded in the thread + // state, which is already in the local thread state buffer. + uintptr_t exc_value = GET_MEMBER(uintptr_t, ts, + unwinder->debug_offsets.thread_state.exc_state + + exc_value_offset); + if (exc_value != 0) { + has_exception = 1; + } + break; + } + uintptr_t exc_value = 0; + if (read_ptr(unwinder, exc_info + exc_value_offset, &exc_value) < 0) { + PyErr_Clear(); // Best effort: treat as no active exception + break; + } + if (exc_value != 0) { + has_exception = 1; + break; + } + uintptr_t previous_item = 0; + if (read_ptr(unwinder, exc_info + previous_item_offset, + &previous_item) < 0) { + PyErr_Clear(); + break; + } + exc_info = previous_item; } }