FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-154194: Degrade frames in Tachyon instead of failing the sample by maurycy · Pull Request #154195 · python/cpython · GitHub

Repository navigation

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Include/internal/pycore_global_strings.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ struct _Py_global_strings {
STRUCT_FOR_STR(native, "<native>")
STRUCT_FOR_STR(str_replace_inf, "1e309")
STRUCT_FOR_STR(type_params, ".type_params")
STRUCT_FOR_STR(unknown_file, "<unknown file>")
STRUCT_FOR_STR(unknown_function, "<unknown function>")
STRUCT_FOR_STR(unreadable_frame, "<unreadable frame>")
STRUCT_FOR_STR(utf_8, "utf-8")
} literals;

Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_runtime_init_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Include/internal/pycore_unicodeobject_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions Lib/asyncio/tools.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ def __init__(
# ─── indexing helpers ───────────────────────────────────────────
def _format_stack_entry(elem: str|FrameInfo) -> str:
if not isinstance(elem, str):
Comment on lines 28 to 29

maurycy Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I believe that isinstance(elem, str) is dead since #135436. Perhaps we should clean this defensive code everywhere.

if elem.location is None:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Regular collectors rely on these helpers:

if location is None:
return 0

if location is None:
return DEFAULT_LOCATION

if elem.filename in ("", "~"):
return f"{elem.funcname}"
return f"{elem.funcname} {elem.filename}"
if elem.location.lineno == 0 and elem.filename == "":
return f"{elem.funcname}"
else:
Expand Down Expand Up @@ -190,8 +194,7 @@ def build_task_table(result):
# Build coroutine stack string
frames = [frame for coro in task_info.coroutine_stack
for frame in coro.call_stack]
coro_stack = " -> ".join(_format_stack_entry(x).split(" ")[0]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This does not work for the <unknown function> sentinel.

for x in frames)
coro_stack = " -> ".join(x.funcname for x in frames)

# Handle tasks with no awaiters
if not task_info.awaited_by:
Expand All @@ -202,8 +205,7 @@ def build_task_table(result):
# Handle tasks with awaiters
for coro_info in task_info.awaited_by:
parent_id = coro_info.task_name
awaiter_frames = [_format_stack_entry(x).split(" ")[0]
for x in coro_info.call_stack]
awaiter_frames = [x.funcname for x in coro_info.call_stack]
awaiter_chain = " -> ".join(awaiter_frames)
awaiter_name = id2name.get(parent_id, "Unknown")
parent_id_str = (hex(parent_id) if isinstance(parent_id, int)
Expand Down
76 changes: 76 additions & 0 deletions Lib/test/test_asyncio/test_tools.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,82 @@ def test_table_output_format(self):

class TestAsyncioToolsEdgeCases(unittest.TestCase):

def test_frames_without_location_tree(self):
"""Frames the unwinder could not fully read - should not crash."""
input_ = [
AwaitedInfo(
thread_id=1,
awaited_by=[
TaskInfo(
task_id=1,
task_name="Task-A",
coroutine_stack=[
CoroInfo(
call_stack=[
FrameInfo("<unreadable frame>", "~", None),
FrameInfo("<unknown function>", "app.py", None),
FrameInfo("big", "big.py", None),
],
task_name=1
)
],
awaited_by=[]
)
]
)
]
self.assertEqual(
tools.build_async_tree(input_),
[[
"└── (T) Task-A",
" └── big big.py",
" └── <unknown function> app.py",
" └── <unreadable frame>",
]],
)

def test_frames_without_location_table(self):
"""Frame names are not truncated at the first space."""
input_ = [
AwaitedInfo(
thread_id=1,
awaited_by=[
TaskInfo(
task_id=1,
task_name="Task-A",
coroutine_stack=[
CoroInfo(
call_stack=[
FrameInfo("<unreadable frame>", "~", None)
],
task_name=1
)
],
awaited_by=[
CoroInfo(
call_stack=[
FrameInfo("<unknown function>", "app.py", None)
],
task_name=2
)
]
)
]
)
]
self.assertEqual(
tools.build_task_table(input_),
[[
1,
"0x1",
"Task-A",
"<unreadable frame>",
"<unknown function>",
"Unknown",
"0x2",
]],
)

def test_task_awaits_self(self):
"""A task directly awaits itself - should raise a cycle."""
input_ = [
Expand Down
87 changes: 87 additions & 0 deletions Lib/test/test_external_inspection.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -4132,6 +4132,93 @@ def test_get_stats_disabled_raises(self):
client_socket.sendall(b"done")


@requires_remote_subprocess_debugging()
@skip_if_not_supported
@unittest.skipIf(
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
"Test only runs on Linux with process_vm_readv support",
)
class TestMetadataDegradation(RemoteInspectionTestBase):
"""Tests for graceful degradation of oversized code-object metadata."""

def test_long_qualname_truncated_not_dropped(self):
"""A qualname longer than 1024 chars is truncated instead of
failing the whole sample."""
name = "f" * 1100
src = f"def {name}(sample):\n return sample()\n"
ns = {}
exec(src, ns)

trace = ns[name](RemoteUnwinder(os.getpid()).get_stack_trace)
frame = self._find_frame_in_trace(
trace, lambda f: f.funcname.startswith("fff")
)
self.assertIsNotNone(frame)
self.assertEqual(frame.funcname, "f" * 1024)

def test_long_filename_truncated(self):
"""A filename longer than 1024 chars is truncated instead of
failing the whole sample."""
src = "def g(sample):\n return sample()\n"
ns = {}
exec(compile(src, "x" * 1500 + ".py", "exec"), ns)

trace = ns["g"](RemoteUnwinder(os.getpid()).get_stack_trace)
frame = self._find_frame_in_trace(trace, lambda f: f.funcname == "g")
self.assertIsNotNone(frame)
self.assertEqual(frame.filename, "x" * 1024)

def test_oversized_linetable_degrades_to_no_location(self):
"""A linetable over MAX_LINETABLE_SIZE degrades to a frame without
location instead of failing the whole sample."""
src = (
"def big(sample):\n"
+ " x = 1\n" * 20_000
+ " return sample()\n"
)
ns = {}
exec(compile(src, "big_linetable.py", "exec"), ns)
big = ns["big"]
self.assertGreater(len(big.__code__.co_linetable), 64 * 1024)

trace = big(RemoteUnwinder(os.getpid()).get_stack_trace)
frame = self._find_frame_in_trace(
trace, lambda f: f.funcname == "big"
)
self.assertIsNone(frame.location)
self.assertEqual(frame.filename, "big_linetable.py")

@unittest.skipIf(
sys.platform == "win32",
"Process death maps to ProcessLookupError only on POSIX platforms",
)
def test_dead_process_raises_not_degrades(self):
"""Death of the target raises ProcessLookupError instead of
degrading to synthetic frames."""
script_body = """\
import time
sock.sendall(b"ready")
time.sleep(10_000)
"""
with self._target_process(script_body) as (p, client_socket, make_unwinder):
_wait_for_signal(client_socket, b"ready")
unwinder = make_unwinder()
_get_stack_trace_with_retry(unwinder)

p.kill()
p.wait()

for _ in busy_retry(SHORT_TIMEOUT, error=False):
try:
unwinder.get_stack_trace()
except ProcessLookupError:
break
except RuntimeError:
continue
else:
self.fail("ProcessLookupError never raised for dead process")


@requires_remote_subprocess_debugging()
class TestFrameChainLimits(RemoteInspectionTestBase):
"""Frame chain walks abort instead of looping/overflowing on deep chains."""
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,26 @@ def test_same_line_different_columns(self):
collector, count = self.roundtrip(samples)
self.assertEqual(count, 3)

def test_synthetic_frames_roundtrip(self):
"""Degraded/sentinel frames (location=None) survive the binary format."""
frames = [
FrameInfo(("~", None, name, None))
for name in (
"<GC>",
"<native>",
"<unknown function>",
"<unknown file>",
"<unreadable frame>",
Comment on lines +673 to +677

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Perhaps we could add something like is_synthetic to FrameInfo?

)
]
frames.append(FrameInfo(("app.py", None, "<unknown function>", None)))
frames.append(FrameInfo(("<unknown file>", None, "real_func", None)))
samples = [[make_interpreter(0, [make_thread(1, frames)])]]

collector, count = self.roundtrip(samples)
self.assertEqual(count, 1)
self.assert_samples_equal(samples, collector)


class TestBinaryEdgeCases(BinaryFormatTestBase):
"""Tests for edge cases in binary format."""
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix the sampling profiler dropping entire samples when a non-fatal read fails;
frames now keep any readable metadata, and long funcnames and filenames are
truncated instead. Patch by Maurycy Pawłowski-Wieroński.
2 changes: 1 addition & 1 deletion Modules/_remote_debugging/_remote_debugging.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ typedef enum _WIN32_THREADSTATE {
#define set_exception_cause(unwinder, exc_type, message) \
do { \
assert(PyErr_Occurred() && "function returned -1 without setting exception"); \
if (unwinder->debug && !_Py_RemoteDebug_HasPermissionError()) { \
if (unwinder->debug && !_Py_RemoteDebug_IsFatalReadError()) { \
_set_debug_exception_cause(exc_type, message); \
} \
} while (0)
Expand Down
53 changes: 45 additions & 8 deletions Modules/_remote_debugging/code_objects.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,16 @@ parse_code_object(RemoteUnwinderObject *unwinder,
PyObject **result,
const CodeObjectContext *ctx)
{
_Py_DECLARE_STR(unknown_function, "<unknown function>");
_Py_DECLARE_STR(unknown_file, "<unknown file>");
_Py_DECLARE_STR(unreadable_frame, "<unreadable frame>");

void *key = (void *)ctx->code_addr;
CachedCodeMetadata *meta = NULL;
PyObject *func = NULL;
PyObject *file = NULL;
PyObject *linetable = NULL;
int code_metadata_incomplete = 0;

maurycy Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Perhaps parse_code_object() should return this information, and we should not store incomplete objects in the frame_cache?

Perhaps we should collect degradation stats? Important caveat is that we will likely have more types of degradation.


#ifdef Py_GIL_DISABLED
// In free threading builds, code object addresses might have the low bit set
Expand All @@ -369,30 +374,50 @@ parse_code_object(RemoteUnwinderObject *unwinder,
if (_Py_RemoteDebug_PagedReadRemoteMemory(
&unwinder->handle, real_address, SIZEOF_CODE_OBJ, code_object) < 0)
{
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read code object");
goto error;
if (_Py_RemoteDebug_IsFatalReadError()) {
goto error;
}
PyErr_Clear();
func = Py_NewRef(&_Py_STR(unreadable_frame));
file = Py_NewRef(_Py_LATIN1_CHR('~'));
goto degraded;
}

func = read_py_str(unwinder,
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.qualname), 1024);
if (!func) {
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read function name from code object");
goto error;
if (_Py_RemoteDebug_IsFatalReadError()) {
goto error;
}
PyErr_Clear();
func = Py_NewRef(&_Py_STR(unknown_function));
code_metadata_incomplete = 1;
}

file = read_py_str(unwinder,
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.filename), 1024);
if (!file) {
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read filename from code object");
goto error;
if (_Py_RemoteDebug_IsFatalReadError()) {
goto error;
}
PyErr_Clear();
file = Py_NewRef(&_Py_STR(unknown_file));
code_metadata_incomplete = 1;
}

if (code_metadata_incomplete) {
goto degraded;
}

linetable = read_py_bytes(unwinder,
GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.linetable),
MAX_LINETABLE_SIZE);
if (!linetable) {
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read linetable from code object");
goto error;
if (_Py_RemoteDebug_IsFatalReadError()) {
goto error;
}
PyErr_Clear();
goto degraded;
}

meta = PyMem_RawMalloc(sizeof(CachedCodeMetadata));
Expand Down Expand Up @@ -561,6 +586,18 @@ parse_code_object(RemoteUnwinderObject *unwinder,
*result = tuple;
return 0;

degraded: {
PyObject *degraded_tuple = make_frame_info(unwinder, file, Py_None,
func, Py_None);
Py_CLEAR(func);
Py_CLEAR(file);
if (!degraded_tuple) {
return -1;
}
*result = degraded_tuple;
return 0;
}

error:
Py_XDECREF(func);
Py_XDECREF(file);
Expand Down
Loading
Loading

Back | FazBrowse Home | New Git URL