Skip to content

Commit c27f494

Browse files
pablogsalmaurycy
andauthored
[3.15] gh-154194: Degrade frames in Tachyon instead of failing the sample (GH-154195) (#158831)
* gh-154194: Degrade frames in Tachyon instead of failing the sample (#154195) * degrade gracefully * news * better NEWS wording * do not raise on MAX_REMOTE_STR_READ * bye MAX_REMOTE_STR_READ * fix -m asyncio ps|pstree * test truncation and linetable sentinel * simpler * simpler * redundant now * respect #157790 in the news --------- Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com> (cherry picked from commit 7d25916) * Preserve the stable ABI when creating fallback frame names --------- Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
1 parent 4f7af46 commit c27f494

8 files changed

Lines changed: 253 additions & 14 deletions

File tree

‎Lib/asyncio/tools.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ def __init__(
2727
# ─── indexing helpers ───────────────────────────────────────────
2828
def _format_stack_entry(elem: str|FrameInfo) -> str:
2929
if not isinstance(elem, str):
30+
if elem.location is None:
31+
if elem.filename in ("", "~"):
32+
return f"{elem.funcname}"
33+
return f"{elem.funcname} {elem.filename}"
3034
if elem.location.lineno == 0 and elem.filename == "":
3135
return f"{elem.funcname}"
3236
else:
@@ -190,8 +194,7 @@ def build_task_table(result):
190194
# Build coroutine stack string
191195
frames = [frame for coro in task_info.coroutine_stack
192196
for frame in coro.call_stack]
193-
coro_stack = " -> ".join(_format_stack_entry(x).split(" ")[0]
194-
for x in frames)
197+
coro_stack = " -> ".join(x.funcname for x in frames)
195198

196199
# Handle tasks with no awaiters
197200
if not task_info.awaited_by:
@@ -202,8 +205,7 @@ def build_task_table(result):
202205
# Handle tasks with awaiters
203206
for coro_info in task_info.awaited_by:
204207
parent_id = coro_info.task_name
205-
awaiter_frames = [_format_stack_entry(x).split(" ")[0]
206-
for x in coro_info.call_stack]
208+
awaiter_frames = [x.funcname for x in coro_info.call_stack]
207209
awaiter_chain = " -> ".join(awaiter_frames)
208210
awaiter_name = id2name.get(parent_id, "Unknown")
209211
parent_id_str = (hex(parent_id) if isinstance(parent_id, int)

‎Lib/test/test_asyncio/test_tools.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,82 @@ def test_table_output_format(self):
15581558

15591559
class TestAsyncioToolsEdgeCases(unittest.TestCase):
15601560

1561+
def test_frames_without_location_tree(self):
1562+
"""Frames the unwinder could not fully read - should not crash."""
1563+
input_ = [
1564+
AwaitedInfo(
1565+
thread_id=1,
1566+
awaited_by=[
1567+
TaskInfo(
1568+
task_id=1,
1569+
task_name="Task-A",
1570+
coroutine_stack=[
1571+
CoroInfo(
1572+
call_stack=[
1573+
FrameInfo("<unreadable frame>", "~", None),
1574+
FrameInfo("<unknown function>", "app.py", None),
1575+
FrameInfo("big", "big.py", None),
1576+
],
1577+
task_name=1
1578+
)
1579+
],
1580+
awaited_by=[]
1581+
)
1582+
]
1583+
)
1584+
]
1585+
self.assertEqual(
1586+
tools.build_async_tree(input_),
1587+
[[
1588+
"└── (T) Task-A",
1589+
" └── big big.py",
1590+
" └── <unknown function> app.py",
1591+
" └── <unreadable frame>",
1592+
]],
1593+
)
1594+
1595+
def test_frames_without_location_table(self):
1596+
"""Frame names are not truncated at the first space."""
1597+
input_ = [
1598+
AwaitedInfo(
1599+
thread_id=1,
1600+
awaited_by=[
1601+
TaskInfo(
1602+
task_id=1,
1603+
task_name="Task-A",
1604+
coroutine_stack=[
1605+
CoroInfo(
1606+
call_stack=[
1607+
FrameInfo("<unreadable frame>", "~", None)
1608+
],
1609+
task_name=1
1610+
)
1611+
],
1612+
awaited_by=[
1613+
CoroInfo(
1614+
call_stack=[
1615+
FrameInfo("<unknown function>", "app.py", None)
1616+
],
1617+
task_name=2
1618+
)
1619+
]
1620+
)
1621+
]
1622+
)
1623+
]
1624+
self.assertEqual(
1625+
tools.build_task_table(input_),
1626+
[[
1627+
1,
1628+
"0x1",
1629+
"Task-A",
1630+
"<unreadable frame>",
1631+
"<unknown function>",
1632+
"Unknown",
1633+
"0x2",
1634+
]],
1635+
)
1636+
15611637
def test_task_awaits_self(self):
15621638
"""A task directly awaits itself - should raise a cycle."""
15631639
input_ = [

‎Lib/test/test_external_inspection.py‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4070,6 +4070,93 @@ def test_get_stats_disabled_raises(self):
40704070
client_socket.sendall(b"done")
40714071

40724072

4073+
@requires_remote_subprocess_debugging()
4074+
@skip_if_not_supported
4075+
@unittest.skipIf(
4076+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
4077+
"Test only runs on Linux with process_vm_readv support",
4078+
)
4079+
class TestMetadataDegradation(RemoteInspectionTestBase):
4080+
"""Tests for graceful degradation of oversized code-object metadata."""
4081+
4082+
def test_long_qualname_truncated_not_dropped(self):
4083+
"""A qualname longer than 1024 chars is truncated instead of
4084+
failing the whole sample."""
4085+
name = "f" * 1100
4086+
src = f"def {name}(sample):\n return sample()\n"
4087+
ns = {}
4088+
exec(src, ns)
4089+
4090+
trace = ns[name](RemoteUnwinder(os.getpid()).get_stack_trace)
4091+
frame = self._find_frame_in_trace(
4092+
trace, lambda f: f.funcname.startswith("fff")
4093+
)
4094+
self.assertIsNotNone(frame)
4095+
self.assertEqual(frame.funcname, "f" * 1024)
4096+
4097+
def test_long_filename_truncated(self):
4098+
"""A filename longer than 1024 chars is truncated instead of
4099+
failing the whole sample."""
4100+
src = "def g(sample):\n return sample()\n"
4101+
ns = {}
4102+
exec(compile(src, "x" * 1500 + ".py", "exec"), ns)
4103+
4104+
trace = ns["g"](RemoteUnwinder(os.getpid()).get_stack_trace)
4105+
frame = self._find_frame_in_trace(trace, lambda f: f.funcname == "g")
4106+
self.assertIsNotNone(frame)
4107+
self.assertEqual(frame.filename, "x" * 1024)
4108+
4109+
def test_oversized_linetable_degrades_to_no_location(self):
4110+
"""A linetable over MAX_LINETABLE_SIZE degrades to a frame without
4111+
location instead of failing the whole sample."""
4112+
src = (
4113+
"def big(sample):\n"
4114+
+ " x = 1\n" * 20_000
4115+
+ " return sample()\n"
4116+
)
4117+
ns = {}
4118+
exec(compile(src, "big_linetable.py", "exec"), ns)
4119+
big = ns["big"]
4120+
self.assertGreater(len(big.__code__.co_linetable), 64 * 1024)
4121+
4122+
trace = big(RemoteUnwinder(os.getpid()).get_stack_trace)
4123+
frame = self._find_frame_in_trace(
4124+
trace, lambda f: f.funcname == "big"
4125+
)
4126+
self.assertIsNone(frame.location)
4127+
self.assertEqual(frame.filename, "big_linetable.py")
4128+
4129+
@unittest.skipIf(
4130+
sys.platform == "win32",
4131+
"Process death maps to ProcessLookupError only on POSIX platforms",
4132+
)
4133+
def test_dead_process_raises_not_degrades(self):
4134+
"""Death of the target raises ProcessLookupError instead of
4135+
degrading to synthetic frames."""
4136+
script_body = """\
4137+
import time
4138+
sock.sendall(b"ready")
4139+
time.sleep(10_000)
4140+
"""
4141+
with self._target_process(script_body) as (p, client_socket, make_unwinder):
4142+
_wait_for_signal(client_socket, b"ready")
4143+
unwinder = make_unwinder()
4144+
_get_stack_trace_with_retry(unwinder)
4145+
4146+
p.kill()
4147+
p.wait()
4148+
4149+
for _ in busy_retry(SHORT_TIMEOUT, error=False):
4150+
try:
4151+
unwinder.get_stack_trace()
4152+
except ProcessLookupError:
4153+
break
4154+
except RuntimeError:
4155+
continue
4156+
else:
4157+
self.fail("ProcessLookupError never raised for dead process")
4158+
4159+
40734160
@requires_remote_subprocess_debugging()
40744161
class TestFrameChainLimits(RemoteInspectionTestBase):
40754162
"""Frame chain walks abort instead of looping/overflowing on deep chains."""

‎Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,26 @@ def test_same_line_different_columns(self):
667667
collector, count = self.roundtrip(samples)
668668
self.assertEqual(count, 3)
669669

670+
def test_synthetic_frames_roundtrip(self):
671+
"""Degraded/sentinel frames (location=None) survive the binary format."""
672+
frames = [
673+
FrameInfo(("~", None, name, None))
674+
for name in (
675+
"<GC>",
676+
"<native>",
677+
"<unknown function>",
678+
"<unknown file>",
679+
"<unreadable frame>",
680+
)
681+
]
682+
frames.append(FrameInfo(("app.py", None, "<unknown function>", None)))
683+
frames.append(FrameInfo(("<unknown file>", None, "real_func", None)))
684+
samples = [[make_interpreter(0, [make_thread(1, frames)])]]
685+
686+
collector, count = self.roundtrip(samples)
687+
self.assertEqual(count, 1)
688+
self.assert_samples_equal(samples, collector)
689+
670690

671691
class TestBinaryEdgeCases(BinaryFormatTestBase):
672692
"""Tests for edge cases in binary format."""
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix the sampling profiler dropping entire samples when a non-fatal read fails;
2+
frames now keep any readable metadata, and long funcnames and filenames are
3+
truncated instead. Patch by Maurycy Pawłowski-Wieroński.

‎Modules/_remote_debugging/_remote_debugging.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ typedef enum _WIN32_THREADSTATE {
182182
#define set_exception_cause(unwinder, exc_type, message) \
183183
do { \
184184
assert(PyErr_Occurred() && "function returned -1 without setting exception"); \
185-
if (unwinder->debug && !_Py_RemoteDebug_HasPermissionError()) { \
185+
if (unwinder->debug && !_Py_RemoteDebug_IsFatalReadError()) { \
186186
_set_debug_exception_cause(exc_type, message); \
187187
} \
188188
} while (0)

‎Modules/_remote_debugging/code_objects.c‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ parse_code_object(RemoteUnwinderObject *unwinder,
346346
PyObject *func = NULL;
347347
PyObject *file = NULL;
348348
PyObject *linetable = NULL;
349+
int code_metadata_incomplete = 0;
349350

350351
#ifdef Py_GIL_DISABLED
351352
// In free threading builds, code object addresses might have the low bit set
@@ -369,30 +370,59 @@ parse_code_object(RemoteUnwinderObject *unwinder,
369370
if (_Py_RemoteDebug_PagedReadRemoteMemory(
370371
&unwinder->handle, real_address, SIZEOF_CODE_OBJ, code_object) < 0)
371372
{
372-
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read code object");
373-
goto error;
373+
if (_Py_RemoteDebug_IsFatalReadError()) {
374+
goto error;
375+
}
376+
PyErr_Clear();
377+
func = PyUnicode_FromString("<unreadable frame>");
378+
if (!func) {
379+
goto error;
380+
}
381+
file = Py_NewRef(_Py_LATIN1_CHR('~'));
382+
goto degraded;
374383
}
375384

376385
func = read_py_str(unwinder,
377386
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.qualname), 1024);
378387
if (!func) {
379-
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read function name from code object");
380-
goto error;
388+
if (_Py_RemoteDebug_IsFatalReadError()) {
389+
goto error;
390+
}
391+
PyErr_Clear();
392+
func = PyUnicode_FromString("<unknown function>");
393+
if (!func) {
394+
goto error;
395+
}
396+
code_metadata_incomplete = 1;
381397
}
382398

383399
file = read_py_str(unwinder,
384400
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.filename), 1024);
385401
if (!file) {
386-
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read filename from code object");
387-
goto error;
402+
if (_Py_RemoteDebug_IsFatalReadError()) {
403+
goto error;
404+
}
405+
PyErr_Clear();
406+
file = PyUnicode_FromString("<unknown file>");
407+
if (!file) {
408+
goto error;
409+
}
410+
code_metadata_incomplete = 1;
411+
}
412+
413+
if (code_metadata_incomplete) {
414+
goto degraded;
388415
}
389416

390417
linetable = read_py_bytes(unwinder,
391418
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.linetable),
392419
MAX_LINETABLE_SIZE);
393420
if (!linetable) {
394-
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read linetable from code object");
395-
goto error;
421+
if (_Py_RemoteDebug_IsFatalReadError()) {
422+
goto error;
423+
}
424+
PyErr_Clear();
425+
goto degraded;
396426
}
397427

398428
meta = PyMem_RawMalloc(sizeof(CachedCodeMetadata));
@@ -561,6 +591,18 @@ parse_code_object(RemoteUnwinderObject *unwinder,
561591
*result = tuple;
562592
return 0;
563593

594+
degraded: {
595+
PyObject *degraded_tuple = make_frame_info(unwinder, file, Py_None,
596+
func, Py_None);
597+
Py_CLEAR(func);
598+
Py_CLEAR(file);
599+
if (!degraded_tuple) {
600+
return -1;
601+
}
602+
*result = degraded_tuple;
603+
return 0;
604+
}
605+
564606
error:
565607
Py_XDECREF(func);
566608
Py_XDECREF(file);

‎Python/remote_debug.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,18 @@ _Py_RemoteDebug_HasPermissionError(void)
107107
&& PyErr_ExceptionMatches(PyExc_PermissionError);
108108
}
109109

110+
static inline int
111+
_Py_RemoteDebug_IsFatalReadError(void)
112+
{
113+
return _Py_RemoteDebug_HasPermissionError()
114+
|| PyErr_ExceptionMatches(PyExc_MemoryError)
115+
|| PyErr_ExceptionMatches(PyExc_ProcessLookupError)
116+
|| (PyErr_Occurred() && !PyErr_ExceptionMatches(PyExc_Exception));
117+
}
118+
110119
#define _set_debug_exception_cause(exception, format, ...) \
111120
do { \
112-
if (!_Py_RemoteDebug_HasPermissionError()) { \
121+
if (!_Py_RemoteDebug_IsFatalReadError()) { \
113122
PyThreadState *tstate = _PyThreadState_GET(); \
114123
if (!_PyErr_Occurred(tstate)) { \
115124
_PyErr_Format(tstate, exception, format, ##__VA_ARGS__); \

0 commit comments

Comments
 (0)