Skip to content

Commit d625ecb

Browse files
pablogsalhetaozdh
andauthored
[3.15] gh-158539: Fix exception mode missing handlers in generators/coroutines (GH-158581) (#158852)
* [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> * [3.15] gh-158539: Use portable static assertion messages * [3.15] gh-158539: Keep layout assertions with debug-offset validation * [3.15] gh-158539: Use the platform guard for in-process inspection tests --------- Co-authored-by: LucasZhou <donghao.zhou@outlook.com>
1 parent 48998df commit d625ecb

4 files changed

Lines changed: 299 additions & 11 deletions

File tree

‎Lib/test/test_external_inspection.py‎

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

32213221

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

51+
/* Derive unexported offsets from adjacent fields to keep the debug-offset
52+
* table compatible across patch releases. */
53+
static_assert(offsetof(PyThreadState, exc_info) ==
54+
offsetof(PyThreadState, current_exception) + sizeof(uintptr_t),
55+
"exc_info must immediately follow current_exception");
56+
static_assert(offsetof(_PyErr_StackItem, previous_item) ==
57+
offsetof(_PyErr_StackItem, exc_value) + sizeof(uintptr_t),
58+
"previous_item must immediately follow exc_value");
59+
5160
/*
5261
* This logic lives in a private header because it is shared by module.c and
5362
* asyncio.c. Keep the helpers static inline so they stay local to those users
@@ -249,14 +258,15 @@ validate_fixed_field(
249258
#define PY_REMOTE_DEBUG_RUNTIME_STATE_FIELDS(APPLY, buffer_size) \
250259
APPLY(runtime_state, interpreters_head, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size)
251260

261+
/* current_exception also covers the adjacent exc_info pointer. */
252262
#define PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(APPLY, buffer_size) \
253263
APPLY(thread_state, native_thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
254264
APPLY(thread_state, interp, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
255265
APPLY(thread_state, datastack_chunk, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
256266
APPLY(thread_state, status, FIELD_SIZE(PyThreadState, _status), _Alignof(unsigned int), buffer_size); \
257267
APPLY(thread_state, holds_gil, sizeof(int), _Alignof(int), buffer_size); \
258268
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); \
269+
APPLY(thread_state, current_exception, 2 * sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
260270
APPLY(thread_state, thread_id, sizeof(unsigned long), _Alignof(long), buffer_size); \
261271
APPLY(thread_state, next, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
262272
APPLY(thread_state, current_frame, sizeof(uintptr_t), _Alignof(uintptr_t), buffer_size); \
@@ -351,10 +361,11 @@ _PyRemoteDebug_ValidateDebugOffsetsLayout(struct _Py_DebugOffsets *debug_offsets
351361
PY_REMOTE_DEBUG_THREAD_STATE_FIELDS(
352362
PY_REMOTE_DEBUG_VALIDATE_FIELD,
353363
SIZEOF_THREAD_STATE);
364+
/* exc_value also covers the adjacent previous_item pointer. */
354365
PY_REMOTE_DEBUG_VALIDATE_FIXED_FIELD(
355366
err_stackitem,
356367
exc_value,
357-
sizeof(uintptr_t),
368+
2 * sizeof(uintptr_t),
358369
_Alignof(uintptr_t),
359370
sizeof(_PyErr_StackItem));
360371
PY_REMOTE_DEBUG_VALIDATE_NESTED_FIELD(

‎Modules/_remote_debugging/threads.c‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
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+
2023
/* ============================================================================
2124
* THREAD ITERATION FUNCTIONS
2225
* ============================================================================ */
@@ -436,16 +439,49 @@ unwind_stack_for_thread(
436439
has_exception = 1;
437440
}
438441

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.
442+
// Generators and coroutines use their own exception stack items.
443+
// Follow exc_info to find the innermost handler, as sys.exception() does.
443444
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;
445+
uintptr_t exc_info = GET_MEMBER(uintptr_t, ts,
446+
unwinder->debug_offsets.thread_state.current_exception +
447+
sizeof(uintptr_t));
448+
uintptr_t exc_state_addr =
449+
*current_tstate + unwinder->debug_offsets.thread_state.exc_state;
450+
uintptr_t exc_value_offset =
451+
unwinder->debug_offsets.err_stackitem.exc_value;
452+
uintptr_t previous_item_offset =
453+
exc_value_offset + sizeof(uintptr_t);
454+
455+
for (int depth = 0; exc_info != 0 && depth < MAX_EXCEPTION_CHAIN_DEPTH;
456+
depth++)
457+
{
458+
if (exc_info == exc_state_addr) {
459+
// Bottom of the chain: the stack item embedded in the thread
460+
// state, which is already in the local thread state buffer.
461+
uintptr_t exc_value = GET_MEMBER(uintptr_t, ts,
462+
unwinder->debug_offsets.thread_state.exc_state +
463+
exc_value_offset);
464+
if (exc_value != 0) {
465+
has_exception = 1;
466+
}
467+
break;
468+
}
469+
uintptr_t exc_value = 0;
470+
if (read_ptr(unwinder, exc_info + exc_value_offset, &exc_value) < 0) {
471+
PyErr_Clear(); // Best effort: treat as no active exception
472+
break;
473+
}
474+
if (exc_value != 0) {
475+
has_exception = 1;
476+
break;
477+
}
478+
uintptr_t previous_item = 0;
479+
if (read_ptr(unwinder, exc_info + previous_item_offset,
480+
&previous_item) < 0) {
481+
PyErr_Clear();
482+
break;
483+
}
484+
exc_info = previous_item;
449485
}
450486
}
451487

0 commit comments

Comments
 (0)