Skip to content

Commit 80f9aa0

Browse files
authored
gh-157605: Accept empty native-thread stacks when sampling
Accept empty native-thread stacks when sampling instead of raising an error. Distinguish interpreter-owned C frames from cleared Python frames so frame cleanup cannot produce a false <native> marker.
1 parent d3663ef commit 80f9aa0

3 files changed

Lines changed: 89 additions & 9 deletions

File tree

‎Lib/test/test_external_inspection.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,6 +1684,80 @@ def test_self_trace(self):
16841684
self.assertEqual(this_thread_stack[1].funcname, "TestGetStackTrace.test_self_trace")
16851685
self.assertTrue(this_thread_stack[1].filename.endswith("test_external_inspection.py"))
16861686

1687+
@skip_if_not_supported
1688+
@unittest.skipIf(
1689+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
1690+
"Test only runs on Linux with process_vm_readv support",
1691+
)
1692+
def test_empty_native_thread_stack(self):
1693+
_testcapi = import_module("_testcapi")
1694+
lock = threading.Lock()
1695+
lock.acquire()
1696+
# A built-in callback leaves the C thread's Python stack empty.
1697+
_testcapi.call_in_temporary_c_thread(lock.acquire, False)
1698+
try:
1699+
for cache_frames, native in ((False, False), (False, True),
1700+
(True, False), (True, True)):
1701+
with self.subTest(cache_frames=cache_frames, native=native):
1702+
unwinder = RemoteUnwinder(
1703+
os.getpid(), all_threads=True, cache_frames=cache_frames,
1704+
native=native,
1705+
)
1706+
_get_stack_trace_with_retry(
1707+
unwinder, condition=lambda trace: len(trace[0].threads) == 2,
1708+
)
1709+
threads = unwinder.get_stack_trace()[0].threads
1710+
native_stack, python_stack = sorted(
1711+
(thread.frame_info for thread in threads), key=len,
1712+
)
1713+
self.assertEqual(native_stack, [])
1714+
self.assertEqual(
1715+
python_stack[0].funcname,
1716+
"TestGetStackTrace.test_empty_native_thread_stack",
1717+
)
1718+
finally:
1719+
lock.release()
1720+
_testcapi.join_temporary_c_thread()
1721+
1722+
@skip_if_not_supported
1723+
@unittest.skipIf(
1724+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
1725+
"Test only runs on Linux with process_vm_readv support",
1726+
)
1727+
def test_popping_python_frame_is_not_native(self):
1728+
script = """\
1729+
def leaf(depth):
1730+
if depth:
1731+
leaf(depth - 1)
1732+
1733+
while True:
1734+
leaf(300)
1735+
"""
1736+
with _managed_subprocess([sys.executable, "-c", script]) as process:
1737+
for _ in busy_retry(SHORT_TIMEOUT):
1738+
try:
1739+
unwinder = RemoteUnwinder(
1740+
process.pid, native=True, gc=False, cache_frames=False,
1741+
)
1742+
except RuntimeError:
1743+
continue
1744+
break
1745+
samples = 0
1746+
for _ in range(10_000):
1747+
try:
1748+
threads = unwinder.get_stack_trace()[0].threads
1749+
except TRANSIENT_ERRORS:
1750+
continue
1751+
if not threads:
1752+
continue
1753+
frames = threads[0].frame_info
1754+
names = [frame.funcname for frame in frames]
1755+
if "leaf" not in names:
1756+
continue
1757+
samples += 1
1758+
self.assertNotIn(("leaf", "<native>"), zip(names, names[1:]))
1759+
self.assertGreater(samples, 1000)
1760+
16871761
@skip_if_not_supported
16881762
@unittest.skipIf(
16891763
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix :mod:`profiling.sampling` failing when a native thread has an empty Python
2+
stack.

‎Modules/_remote_debugging/frames.c‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,21 +163,23 @@ find_frame_in_chunks(StackChunkList *chunks, uintptr_t remote_ptr)
163163
* FRAME PARSING FUNCTIONS
164164
* ============================================================================ */
165165

166+
enum { FRAME_PARSE_INTERPRETER = 2 };
167+
166168
int
167169
is_frame_valid(
168170
RemoteUnwinderObject *unwinder,
169171
uintptr_t frame_addr,
170172
uintptr_t code_object_addr
171173
) {
172174
if ((void*)code_object_addr == NULL) {
173-
return 0;
175+
return 0; // Frame being cleared
174176
}
175177

176178
void* frame = (void*)frame_addr;
177179

178180
char owner = GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner);
179181
if (owner == FRAME_OWNED_BY_INTERPRETER) {
180-
return 0; // C frame or sentinel base frame
182+
return FRAME_PARSE_INTERPRETER; // C frame or sentinel base frame
181183
}
182184

183185
if (owner != FRAME_OWNED_BY_GENERATOR && owner != FRAME_OWNED_BY_THREAD) {
@@ -313,6 +315,7 @@ process_frame_chain(
313315
ctx->last_frame_visited = 0;
314316

315317
while ((void*)frame_addr != NULL) {
318+
int parse_result = 0;
316319
PyObject *frame = NULL;
317320
uintptr_t next_frame_addr = 0;
318321
uintptr_t stackpointer = 0;
@@ -326,14 +329,15 @@ process_frame_chain(
326329
assert(frame_count <= MAX_FRAMES);
327330

328331
if (ctx->chunks && ctx->chunks->count > 0) {
329-
if (parse_frame_from_chunks(unwinder, &frame, frame_addr, &next_frame_addr, &stackpointer, ctx->chunks) == 0) {
332+
parse_result = parse_frame_from_chunks(
333+
unwinder, &frame, frame_addr, &next_frame_addr, &stackpointer, ctx->chunks);
334+
if (parse_result == 0) {
330335
goto parsed_frame;
331336
}
332337
PyErr_Clear();
333338
}
334339
{
335340
uintptr_t address_of_code_object = 0;
336-
int parse_result;
337341
if (ctx->prefetch.frame && ctx->prefetch.frame_addr == frame_addr) {
338342
parse_result = parse_frame_buffer(
339343
unwinder, &frame, ctx->prefetch.frame,
@@ -358,19 +362,19 @@ process_frame_chain(
358362
continue;
359363
}
360364

361-
if (frame == NULL && PyList_GET_SIZE(ctx->frame_info) == 0) {
362-
const char *e = "Failed to parse initial frame in chain";
363-
PyErr_SetString(PyExc_RuntimeError, e);
364-
return -1;
365-
}
366365
PyObject *extra_frame = NULL;
367366
if (unwinder->gc && frame_addr == ctx->gc_frame) {
368367
_Py_DECLARE_STR(gc, "<GC>");
369368
extra_frame = &_Py_STR(gc);
370369
}
370+
// A leading frame without Python code marks no transition between
371+
// Python frames: it is a frame being popped or C code the thread is
372+
// returning into.
371373
else if (unwinder->native &&
372374
frame == NULL &&
375+
parse_result == FRAME_PARSE_INTERPRETER &&
373376
next_frame_addr &&
377+
PyList_GET_SIZE(ctx->frame_info) > 0 &&
374378
!(unwinder->gc && next_frame_addr == ctx->gc_frame))
375379
{
376380
_Py_DECLARE_STR(native, "<native>");

0 commit comments

Comments
 (0)