Skip to content

Commit d44c119

Browse files
maurycypablogsal
andcommitted
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)
1 parent 28864d1 commit d44c119

12 files changed

Lines changed: 269 additions & 14 deletions

File tree

‎Include/internal/pycore_global_objects_fini_generated.h‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_global_strings.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ struct _Py_global_strings {
5454
STRUCT_FOR_STR(native, "<native>")
5555
STRUCT_FOR_STR(str_replace_inf, "1e309")
5656
STRUCT_FOR_STR(type_params, ".type_params")
57+
STRUCT_FOR_STR(unknown_file, "<unknown file>")
58+
STRUCT_FOR_STR(unknown_function, "<unknown function>")
59+
STRUCT_FOR_STR(unreadable_frame, "<unreadable frame>")
5760
STRUCT_FOR_STR(utf_8, "utf-8")
5861
} literals;
5962

‎Include/internal/pycore_runtime_init_generated.h‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_unicodeobject_generated.h‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎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
@@ -665,6 +665,26 @@ def test_same_line_different_columns(self):
665665
collector, count = self.roundtrip(samples)
666666
self.assertEqual(count, 3)
667667

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

669689
class TestBinaryEdgeCases(BinaryFormatTestBase):
670690
"""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)

0 commit comments

Comments
 (0)