Skip to content

Commit 35144d8

Browse files
pablogsalhetaozdh
andcommitted
[3.15] gh-158539: Fix exception mode missing handlers in generators/coroutines (GH-158581)
Backport of GH-158581. Co-authored-by: LucasZhou <donghao.zhou@outlook.com>
1 parent fce28da commit 35144d8

4 files changed

Lines changed: 296 additions & 11 deletions

File tree

‎Lib/test/test_external_inspection.py‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3219,6 +3219,241 @@ def test_finally_no_exception_no_flag(self):
32193219
self._check_exception_status(p, thread_tid, expect_exception=False)
32203220

32213221

3222+
class TestExceptionDetectionInProcess(RemoteInspectionTestBase):
3223+
"""gh-158539: HAS_EXCEPTION for handlers running in generators/coroutines.
3224+
3225+
``TestExceptionDetectionScenarios`` samples a child process and therefore
3226+
needs subprocess debugging permissions. These tests inspect the current
3227+
process with ``RemoteUnwinder`` and only need self-inspection, so they also
3228+
run on macOS without special entitlements.
3229+
"""
3230+
3231+
@classmethod
3232+
def setUpClass(cls):
3233+
try:
3234+
RemoteUnwinder(os.getpid(), all_threads=True).get_stack_trace()
3235+
except PermissionError as exc:
3236+
raise unittest.SkipTest(f"self-inspection is unavailable: {exc}")
3237+
3238+
def _check_running_handler(
3239+
self, target, expect_exception, *, mode=PROFILING_MODE_ALL,
3240+
skip_non_matching_threads=False,
3241+
):
3242+
"""Run *target* in a thread and check its HAS_EXCEPTION flag.
3243+
3244+
*target* receives ``(ready, stop)`` events and must signal ``ready``
3245+
only once it is executing inside the code region under test, then keep
3246+
running until ``stop`` is set.
3247+
"""
3248+
stop = threading.Event()
3249+
ready = threading.Event()
3250+
failure = []
3251+
3252+
def runner():
3253+
try:
3254+
target(ready, stop)
3255+
except BaseException as exc:
3256+
failure.append(exc)
3257+
ready.set()
3258+
3259+
thread = threading.Thread(target=runner, daemon=True)
3260+
thread.start()
3261+
try:
3262+
self.assertTrue(ready.wait(SHORT_TIMEOUT), "handler never started")
3263+
self.assertFalse(failure, f"handler raised {failure!r}")
3264+
3265+
unwinder = RemoteUnwinder(
3266+
os.getpid(),
3267+
all_threads=True,
3268+
mode=mode,
3269+
skip_non_matching_threads=skip_non_matching_threads,
3270+
)
3271+
observed = []
3272+
for _ in busy_retry(SHORT_TIMEOUT):
3273+
with contextlib.suppress(*TRANSIENT_ERRORS):
3274+
statuses = self._get_thread_statuses(unwinder.get_stack_trace())
3275+
status = statuses.get(thread.native_id)
3276+
if status is None:
3277+
continue
3278+
has_exception = bool(status & THREAD_STATUS_HAS_EXCEPTION)
3279+
observed.append(has_exception)
3280+
if has_exception == expect_exception:
3281+
break
3282+
self.assertTrue(
3283+
observed, "target thread status was never observed"
3284+
)
3285+
self.assertIn(
3286+
expect_exception,
3287+
observed,
3288+
f"HAS_EXCEPTION was never {expect_exception} while the "
3289+
f"handler was running (observed {observed})",
3290+
)
3291+
finally:
3292+
stop.set()
3293+
thread.join(SHORT_TIMEOUT)
3294+
3295+
def _busy_until_stopped(self, ready, stop):
3296+
ready.set()
3297+
while not stop.is_set():
3298+
time.sleep(0.001)
3299+
3300+
def test_handler_in_function(self):
3301+
def target(ready, stop):
3302+
try:
3303+
raise ValueError("test")
3304+
except ValueError:
3305+
self._busy_until_stopped(ready, stop)
3306+
3307+
self._check_running_handler(target, expect_exception=True)
3308+
3309+
def test_handler_in_generator(self):
3310+
def target(ready, stop):
3311+
def gen():
3312+
try:
3313+
raise ValueError("test")
3314+
except ValueError:
3315+
self._busy_until_stopped(ready, stop)
3316+
yield
3317+
3318+
for _ in gen():
3319+
pass
3320+
3321+
self._check_running_handler(target, expect_exception=True)
3322+
3323+
def test_handler_in_genexpr_callee(self):
3324+
def target(ready, stop):
3325+
def callee():
3326+
try:
3327+
raise ValueError("test")
3328+
except ValueError:
3329+
self._busy_until_stopped(ready, stop)
3330+
3331+
list(callee() for _ in range(1))
3332+
3333+
self._check_running_handler(target, expect_exception=True)
3334+
3335+
def test_handler_in_coroutine(self):
3336+
async def coro(ready, stop):
3337+
try:
3338+
raise ValueError("test")
3339+
except ValueError:
3340+
self._busy_until_stopped(ready, stop)
3341+
3342+
def target(ready, stop):
3343+
asyncio.run(coro(ready, stop))
3344+
3345+
self._check_running_handler(target, expect_exception=True)
3346+
3347+
def test_handler_in_callee_from_coroutine(self):
3348+
def callee(ready, stop):
3349+
try:
3350+
raise ValueError("test")
3351+
except ValueError:
3352+
self._busy_until_stopped(ready, stop)
3353+
3354+
async def coro(ready, stop):
3355+
callee(ready, stop)
3356+
3357+
def target(ready, stop):
3358+
asyncio.run(coro(ready, stop))
3359+
3360+
self._check_running_handler(target, expect_exception=True)
3361+
3362+
def test_outer_handler_while_generator_runs(self):
3363+
"""A generator with no handler of its own must not hide the outer one.
3364+
3365+
``exc_info`` points at the generator's empty ``_PyErr_StackItem`` whose
3366+
``previous_item`` is the thread's ``exc_state``, so the profiler has to
3367+
walk the chain to find the exception ``sys.exception()`` reports.
3368+
"""
3369+
def target(ready, stop):
3370+
def gen():
3371+
self._busy_until_stopped(ready, stop)
3372+
yield
3373+
3374+
try:
3375+
raise ValueError("outer")
3376+
except ValueError:
3377+
for _ in gen():
3378+
pass
3379+
3380+
self._check_running_handler(target, expect_exception=True)
3381+
3382+
def test_generator_without_exception(self):
3383+
def target(ready, stop):
3384+
def gen():
3385+
self._busy_until_stopped(ready, stop)
3386+
yield
3387+
3388+
for _ in gen():
3389+
pass
3390+
3391+
self._check_running_handler(target, expect_exception=False)
3392+
3393+
def test_outer_handler_while_nested_generators_run(self):
3394+
def target(ready, stop):
3395+
def gen(depth):
3396+
if depth:
3397+
yield from gen(depth - 1)
3398+
else:
3399+
self._busy_until_stopped(ready, stop)
3400+
yield
3401+
3402+
try:
3403+
raise ValueError("outer")
3404+
except ValueError:
3405+
for _ in gen(32):
3406+
pass
3407+
3408+
self._check_running_handler(
3409+
target,
3410+
expect_exception=True,
3411+
mode=PROFILING_MODE_EXCEPTION,
3412+
skip_non_matching_threads=True,
3413+
)
3414+
3415+
def test_generator_finally_after_except(self):
3416+
"""The handled exception is cleared before the generator's finally."""
3417+
def target(ready, stop):
3418+
def gen():
3419+
try:
3420+
raise ValueError("test")
3421+
except ValueError:
3422+
pass
3423+
finally:
3424+
self._busy_until_stopped(ready, stop)
3425+
yield
3426+
3427+
for _ in gen():
3428+
pass
3429+
3430+
self._check_running_handler(target, expect_exception=False)
3431+
3432+
def test_exception_mode_filter_keeps_generator_handler(self):
3433+
"""The exception-mode thread filter must not drop a generator handler.
3434+
3435+
This mirrors what ``--mode=exception`` actually does: threads without
3436+
HAS_EXCEPTION are skipped before their stack is unwound.
3437+
"""
3438+
def target(ready, stop):
3439+
def gen():
3440+
try:
3441+
raise ValueError("test")
3442+
except ValueError:
3443+
self._busy_until_stopped(ready, stop)
3444+
yield
3445+
3446+
for _ in gen():
3447+
pass
3448+
3449+
self._check_running_handler(
3450+
target,
3451+
expect_exception=True,
3452+
mode=PROFILING_MODE_EXCEPTION,
3453+
skip_non_matching_threads=True,
3454+
)
3455+
3456+
32223457
@requires_remote_subprocess_debugging()
32233458
class TestFrameCaching(RemoteInspectionTestBase):
32243459
"""Test that frame caching produces correct results.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix :mod:`profiling.sampling` exception mode discarding samples for code
2+
running inside an ``except`` block in a generator or coroutine. The remote
3+
debugger now follows ``tstate->exc_info`` and its ``previous_item`` chain
4+
instead of only reading the embedded ``exc_state``, matching the exception
5+
that :func:`sys.exception` reports.

‎Modules/_remote_debugging/debug_offsets_validation.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,15 @@ validate_fixed_field(
249249
#define PY_REMOTE_DEBUG_RUNTIME_STATE_FIELDS(APPLY, buffer_size) \
250250
APPLY(runtime_state, interpreters_head, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size)
251251

252+
/* current_exception also covers the adjacent exc_info pointer. */
252253
#define PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(APPLY, buffer_size) \
253254
APPLY(thread_state, native_thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
254255
APPLY(thread_state, interp, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
255256
APPLY(thread_state, datastack_chunk, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
256257
APPLY(thread_state, status, FIELD_SIZE(PyThreadState, _status), _Alignof(unsigned int), buffer_size); \
257258
APPLY(thread_state, holds_gil, sizeof(int), _Alignof(int), buffer_size); \
258259
APPLY(thread_state, gil_requested, sizeof(int), _Alignof(int), buffer_size); \
259-
APPLY(thread_state, current_exception, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
260+
APPLY(thread_state, current_exception, 2 * sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
260261
APPLY(thread_state, thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
261262
APPLY(thread_state, next, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
262263
APPLY(thread_state, current_frame, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
@@ -351,10 +352,11 @@ _PyRemoteDebug_ValidateDebugOffsetsLayout(struct _Py_DebugOffsets *debug_offsets
351352
PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(
352353
PY_REMOTE_DEBUG_VALIDATE_FIELD,
353354
SIZEOF_THREAD_STATE);
355+
/* exc_value also covers the adjacent previous_item pointer. */
354356
PY_REMOTE_DEBUG_VALIDATE_FIXED_FIELD(
355357
err_stackitem,
356358
exc_value,
357-
sizeof(uintptr_t),
359+
2 * sizeof(uintptr_t),
358360
_Alignof(uintptr_t),
359361
sizeof(_PyErr_StackItem));
360362
PY_REMOTE_DEBUG_VALIDATE_NESTED_FIELD(

‎Modules/_remote_debugging/threads.c‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@
1717
#include <sys/wait.h>
1818
#endif
1919

20+
/* Bound traversal of corrupted remote exception chains. */
21+
#define MAX_EXCEPTION_CHAIN_DEPTH (2 << 15)
22+
23+
/* Derive unexported offsets from adjacent fields to keep the debug-offset
24+
* table compatible across patch releases. */
25+
static_assert(offsetof(PyThreadState, exc_info) ==
26+
offsetof(PyThreadState, current_exception) + sizeof(uintptr_t));
27+
static_assert(offsetof(_PyErr_StackItem, previous_item) ==
28+
offsetof(_PyErr_StackItem, exc_value) + sizeof(uintptr_t));
29+
2030
/* ============================================================================
2131
* THREAD ITERATION FUNCTIONS
2232
* ============================================================================ */
@@ -436,16 +446,49 @@ unwind_stack_for_thread(
436446
has_exception = 1;
437447
}
438448

439-
// Check exc_state.exc_value (exception being handled in except block)
440-
// exc_state is embedded in PyThreadState, so we read it directly from
441-
// the thread state buffer. This catches most cases; nested exception
442-
// handlers where exc_info points elsewhere are rare.
449+
// Generators and coroutines use their own exception stack items.
450+
// Follow exc_info to find the innermost handler, as sys.exception() does.
443451
if (!has_exception) {
444-
uintptr_t exc_value = GET_MEMBER(uintptr_t, ts,
445-
unwinder->debug_offsets.thread_state.exc_state +
446-
unwinder->debug_offsets.err_stackitem.exc_value);
447-
if (exc_value != 0) {
448-
has_exception = 1;
452+
uintptr_t exc_info = GET_MEMBER(uintptr_t, ts,
453+
unwinder->debug_offsets.thread_state.current_exception +
454+
sizeof(uintptr_t));
455+
uintptr_t exc_state_addr =
456+
*current_tstate + unwinder->debug_offsets.thread_state.exc_state;
457+
uintptr_t exc_value_offset =
458+
unwinder->debug_offsets.err_stackitem.exc_value;
459+
uintptr_t previous_item_offset =
460+
exc_value_offset + sizeof(uintptr_t);
461+
462+
for (int depth = 0; exc_info != 0 && depth < MAX_EXCEPTION_CHAIN_DEPTH;
463+
depth++)
464+
{
465+
if (exc_info == exc_state_addr) {
466+
// Bottom of the chain: the stack item embedded in the thread
467+
// state, which is already in the local thread state buffer.
468+
uintptr_t exc_value = GET_MEMBER(uintptr_t, ts,
469+
unwinder->debug_offsets.thread_state.exc_state +
470+
exc_value_offset);
471+
if (exc_value != 0) {
472+
has_exception = 1;
473+
}
474+
break;
475+
}
476+
uintptr_t exc_value = 0;
477+
if (read_ptr(unwinder, exc_info + exc_value_offset, &exc_value) < 0) {
478+
PyErr_Clear(); // Best effort: treat as no active exception
479+
break;
480+
}
481+
if (exc_value != 0) {
482+
has_exception = 1;
483+
break;
484+
}
485+
uintptr_t previous_item = 0;
486+
if (read_ptr(unwinder, exc_info + previous_item_offset,
487+
&previous_item) < 0) {
488+
PyErr_Clear();
489+
break;
490+
}
491+
exc_info = previous_item;
449492
}
450493
}
451494

0 commit comments

Comments
 (0)