From 840933b6efaa4086f947496f40e6871a68d1a056 Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Thu, 17 Sep 2026 12:41:47 +0800 Subject: [PATCH 1/6] gh-157605: Accept empty Python frame stack in sampling frame chain parser A native thread can retain a Python thread state while waiting in C code with no Python functions executing. The frame chain of that thread state holds only the base_frame sentinel, and process_frame_chain() treated the resulting NULL first frame as a broken chain, failing the entire sampling call and discarding valid stacks from all other threads. Only raise when the unparseable first frame is not the base_frame sentinel; otherwise stop the walk and accept the empty stack. Walking stops at the sentinel, so last_frame_visited still equals base_frame_addr and complete-stack validation in the frame cache is preserved. Tests: a self-contained regression test in test_external_inspection.py compiles a small C helper at runtime (skipping when no compiler is available), starts a native thread that retains a Python thread state with an empty frame stack via ctypes, and asserts that same-process sampling succeeds while other Python threads remain visible, with and without frame caching. --- Lib/test/test_external_inspection.py | 201 ++++++++++++++++++ ...-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst | 5 + Modules/_remote_debugging/frames.c | 13 +- 3 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 910fe96d5e7d81..0f5afdbfa8b276 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -13,6 +13,7 @@ from test.support import ( os_helper, SHORT_TIMEOUT, + LONG_TIMEOUT, busy_retry, requires_gil_enabled, requires_remote_subprocess_debugging, @@ -65,6 +66,74 @@ # ============================================================================ +# C source used by test_sampling_thread_with_empty_python_stack to spawn a +# native thread that keeps a Python thread state alive while no Python code +# runs on it: the frame chain of that thread state holds only the base_frame +# sentinel (gh-157605). +_NATIVE_EMPTY_STACK_HELPER_SOURCE = r""" +#include +#include + +static pthread_t helper_thread; +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static int ready = 0; +static int stopping = 0; +static int retain_state = 0; + +static void * +run(void *arg) +{ + PyGILState_STATE gstate; + PyThreadState *saved = NULL; + if (retain_state) { + gstate = PyGILState_Ensure(); + saved = PyEval_SaveThread(); + } + pthread_mutex_lock(&lock); + ready = 1; + pthread_cond_signal(&cond); + while (!stopping) { + pthread_cond_wait(&cond, &lock); + } + pthread_mutex_unlock(&lock); + if (retain_state) { + PyEval_RestoreThread(saved); + PyGILState_Release(gstate); + } + return NULL; +} + +/* Called through ctypes, which releases the calling thread's GIL. */ +int +start_native_thread(int retain) +{ + retain_state = retain; + ready = 0; + stopping = 0; + if (pthread_create(&helper_thread, NULL, run, NULL) != 0) { + return 1; + } + pthread_mutex_lock(&lock); + while (!ready) { + pthread_cond_wait(&cond, &lock); + } + pthread_mutex_unlock(&lock); + return 0; +} + +void +stop_native_thread(void) +{ + pthread_mutex_lock(&lock); + stopping = 1; + pthread_cond_signal(&cond); + pthread_mutex_unlock(&lock); + pthread_join(helper_thread, NULL); +} +""" + + def _make_test_script(script_dir, script_basename, source): to_return = make_script(script_dir, script_basename, source) importlib.invalidate_caches() @@ -488,6 +557,138 @@ def test_self_trace_with_large_linetable(self): ) +class TestEmptyPythonStackSampling(RemoteInspectionTestBase): + @skip_if_not_supported + def test_sampling_thread_with_empty_python_stack(self): + # gh-157605: a native thread can keep a Python thread state alive + # while waiting in C code with no Python frames on it: its frame + # chain holds only the base_frame sentinel. Sampling such a process + # must accept the empty stack and still report the other threads. + script = textwrap.dedent( + """\ + import collections + import ctypes + import os + import subprocess + import sys + import sysconfig + import threading + import time + import _remote_debugging + + helper_dir = sys.argv[1] + source_path = os.path.join(helper_dir, "native_helper.c") + lib_path = os.path.join(helper_dir, "native_helper.so") + with open(source_path, "w") as f: + f.write(sys.argv[2]) + + cc = sysconfig.get_config_var("CC") or "" + if not cc: + # Windows official builds have no CC config var. + print("NO-COMPILER: sysconfig CC is not set") + sys.exit(42) + includes = sorted( + {sysconfig.get_path("include"), + sysconfig.get_path("platinclude")} + ) + extra = [] + if sys.platform == "darwin": + # Delay Python symbol resolution until the .so is loaded + # into a running interpreter. + extra = ["-undefined", "dynamic_lookup"] + cmd = cc.split() + [ + "-shared", "-fPIC", "-pthread", + *[f"-I{path}" for path in includes], + *extra, + source_path, "-o", lib_path, + ] + try: + subprocess.run(cmd, check=True, capture_output=True, text=True) + except FileNotFoundError: + print("NO-COMPILER: compiler executable not found") + sys.exit(42) + except subprocess.CalledProcessError as exc: + # A real compile failure means the helper rotted: fail loud + # instead of silently skipping. + print(f"HELPER-COMPILE-FAILED: {exc.stderr}") + sys.exit(43) + + lib = ctypes.CDLL(lib_path) + lib.start_native_thread.argtypes = [ctypes.c_int] + lib.start_native_thread.restype = ctypes.c_int + lib.stop_native_thread.argtypes = [] + lib.stop_native_thread.restype = None + + started = threading.Event() + + def target(): + started.set() + time.sleep(10_000) + + def sample(pin, cache_frames): + if lib.start_native_thread(pin) != 0: + raise RuntimeError("failed to start native thread") + try: + unwinder = _remote_debugging.RemoteUnwinder( + os.getpid(), all_threads=True, + cache_frames=cache_frames, + ) + counts = collections.Counter() + for _ in range(5): + try: + trace = unwinder.get_stack_trace() + except RuntimeError: + counts["failure"] += 1 + else: + counts["success"] += 1 + frames = [ + frame + for interp in trace + for thread in interp.threads + for frame in thread.frame_info + ] + counts["target visible"] += any( + frame.funcname == "target" + for frame in frames + ) + return dict(counts) + finally: + lib.stop_native_thread() + + target_thread = threading.Thread(target=target, daemon=True) + target_thread.start() + assert started.wait(5.0), "target thread did not start" + for pin in (0, 1): + for cache_frames in (False, True): + counts = sample(pin, cache_frames) + print(f"pin={pin} cache={cache_frames} {counts}", + flush=True) + assert counts.get("success") == 5, counts + assert counts.get("target visible") == 5, counts + """ + ) + + with os_helper.temp_dir() as work_dir: + result = subprocess.run( + [sys.executable, "-c", script, work_dir, + _NATIVE_EMPTY_STACK_HELPER_SOURCE], + capture_output=True, + text=True, + timeout=LONG_TIMEOUT, + ) + if result.returncode == 42: + self.skipTest(f"C compiler not available: {result.stdout}") + if result.returncode == 43: + self.fail( + "native helper failed to compile: " + f"{result.stdout}\n{result.stderr}" + ) + self.assertEqual( + result.returncode, 0, + f"stdout: {result.stdout}\nstderr: {result.stderr}" + ) + + @requires_remote_subprocess_debugging() class TestGetStackTrace(RemoteInspectionTestBase): @skip_if_not_supported diff --git a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst new file mode 100644 index 00000000000000..c14fa29cee9314 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst @@ -0,0 +1,5 @@ +Fix a bug that caused :mod:`!_remote_debugging` sampling to fail entirely +with ``RuntimeError: Failed to parse initial frame in chain`` when a native +thread retained a Python thread state with an empty Python frame stack. The +base_frame sentinel is now accepted as a valid empty stack, and valid stacks +from other threads are reported instead of being lost. diff --git a/Modules/_remote_debugging/frames.c b/Modules/_remote_debugging/frames.c index 46968acc6ff1fe..4a4973540f188f 100644 --- a/Modules/_remote_debugging/frames.c +++ b/Modules/_remote_debugging/frames.c @@ -359,9 +359,16 @@ process_frame_chain( } if (frame == NULL && PyList_GET_SIZE(ctx->frame_info) == 0) { - const char *e = "Failed to parse initial frame in chain"; - PyErr_SetString(PyExc_RuntimeError, e); - return -1; + if (frame_addr != ctx->base_frame_addr) { + const char *e = "Failed to parse initial frame in chain"; + PyErr_SetString(PyExc_RuntimeError, e); + return -1; + } + // A native thread that released the GIL with PyEval_SaveThread() + // keeps a thread state whose frame chain holds only the + // base_frame sentinel. This is a valid empty Python stack: + // accept it and let other threads report their stacks. + break; } PyObject *extra_frame = NULL; if (unwinder->gc && frame_addr == ctx->gc_frame) { From 1a923f1bb9c40a0f7853928334b9fd290ae988b1 Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Thu, 17 Sep 2026 13:29:46 +0800 Subject: [PATCH 2/6] gh-157605: Fix regression test include dirs for out-of-tree builds sysconfig's install-style include paths do not necessarily exist for build-tree interpreters: in an out-of-tree build the test's compile step could not find Python.h (it lives in the source tree's Include directory while pyconfig.h lives in the build directory), so the helper failed to compile and the CI test run failed. Gather candidate include directories from INCLUDEPY, sysconfig paths, the executable directory and srcdir, and keep the ones that actually contain Python.h or pyconfig.h. Skip cleanly when either header cannot be found. --- Lib/test/test_external_inspection.py | 32 ++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 0f5afdbfa8b276..11b848d5a56577 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -587,10 +587,34 @@ def test_sampling_thread_with_empty_python_stack(self): # Windows official builds have no CC config var. print("NO-COMPILER: sysconfig CC is not set") sys.exit(42) - includes = sorted( - {sysconfig.get_path("include"), - sysconfig.get_path("platinclude")} - ) + # sysconfig's install-style include paths do not necessarily + # exist for build-tree interpreters (out-of-tree builds keep + # the headers in the source and build directories instead), + # so gather candidate directories and keep the ones that + # actually contain the headers. + candidates = [ + sysconfig.get_config_var("INCLUDEPY"), + sysconfig.get_path("include"), + sysconfig.get_path("platinclude"), + os.path.dirname(sys.executable), + ] + srcdir = sysconfig.get_config_var("srcdir") + if srcdir: + candidates.append(srcdir) + candidates.append(os.path.join(srcdir, "Include")) + python_h_dirs = set() + pyconfig_h_dirs = set() + for path in candidates: + if path and os.path.isdir(path): + if os.path.isfile(os.path.join(path, "Python.h")): + python_h_dirs.add(path) + if os.path.isfile(os.path.join(path, "pyconfig.h")): + pyconfig_h_dirs.add(path) + if not python_h_dirs or not pyconfig_h_dirs: + print("NO-COMPILER: no include directory with " + "Python.h and pyconfig.h was found") + sys.exit(42) + includes = sorted(python_h_dirs | pyconfig_h_dirs) extra = [] if sys.platform == "darwin": # Delay Python symbol resolution until the .so is loaded From c39b4bcb6a98d454dfa02017c05ff78abdc07e70 Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Fri, 18 Sep 2026 15:54:43 +0800 Subject: [PATCH 3/6] gh-157605: Handle a native first frame like a mid-chain native frame A first frame that parses to no Python code but has a valid continuation is now handled exactly like a mid-chain native frame: it produces the marker when native frames are requested and is skipped otherwise, instead of failing the sample with 'Failed to parse initial frame in chain'. The error remains only for a dangling first frame with no continuation, which cannot be represented in the result. Suggested in gh-157605 review. --- Modules/_remote_debugging/frames.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/Modules/_remote_debugging/frames.c b/Modules/_remote_debugging/frames.c index 4a4973540f188f..52c93ec060355a 100644 --- a/Modules/_remote_debugging/frames.c +++ b/Modules/_remote_debugging/frames.c @@ -359,16 +359,25 @@ process_frame_chain( } if (frame == NULL && PyList_GET_SIZE(ctx->frame_info) == 0) { - if (frame_addr != ctx->base_frame_addr) { + if (frame_addr == ctx->base_frame_addr) { + // A native thread that released the GIL with + // PyEval_SaveThread() keeps a thread state whose frame + // chain holds only the base_frame sentinel. This is a + // valid empty Python stack: accept it and let other + // threads report their stacks. + break; + } + if (next_frame_addr == 0) { + // A dangling undecodable first frame with no continuation + // cannot be represented in the result. const char *e = "Failed to parse initial frame in chain"; PyErr_SetString(PyExc_RuntimeError, e); return -1; } - // A native thread that released the GIL with PyEval_SaveThread() - // keeps a thread state whose frame chain holds only the - // base_frame sentinel. This is a valid empty Python stack: - // accept it and let other threads report their stacks. - break; + // A first frame without Python code but with a valid + // continuation (for example a thread sitting in a C call + // between Python frames) is handled exactly like a mid-chain + // native frame below. } PyObject *extra_frame = NULL; if (unwinder->gc && frame_addr == ctx->gc_frame) { From ddcd747a48752c80a1390bf2550c5cb28cfefeaa Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Fri, 18 Sep 2026 16:35:04 +0800 Subject: [PATCH 4/6] gh-157605: Cover native-first frames with a regression test Add test_sampling_native_first_frame: a runtime-compiled helper installs a synthetic _PyInterpreterFrame (FRAME_OWNED_BY_INTERPRETER, previous pointing at the base_frame sentinel or NULL) as the current frame of a detached thread state, the same technique as _testcapi/import.c. The test asserts that such a first frame is skipped without native frames and reported as with native frames enabled, exercising cache hits as well as misses, and that a dangling first frame with no continuation still fails the sample loudly. Also consult CONFINCLUDEPY in the include-directory candidates, check for the internal frame layout header before compiling the helper, and mention the native-first handling in the news entry. Tests skip cleanly when the headers or a compiler are unavailable. --- Lib/test/test_external_inspection.py | 249 ++++++++++++++++++ ...-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst | 6 +- 2 files changed, 253 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 11b848d5a56577..2abca2d9a52b13 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -133,6 +133,78 @@ } """ +# C source used by test_sampling_native_first_frame to install a synthetic +# undecodable first frame on a detached thread state: the chain starts at a +# frame that parses to no Python code, followed either by the base_frame +# sentinel or by nothing at all (the dangling case). +_NATIVE_FIRST_FRAME_HELPER_SOURCE = r""" +#include +#include +#define Py_BUILD_CORE 1 +#include "internal/pycore_interpframe_structs.h" + +static pthread_t helper_thread; +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static int ready = 0; +static int stopping = 0; +static int dangling = 0; +static _PyInterpreterFrame fake_frame; + +static void * +run(void *arg) +{ + PyGILState_STATE gstate = PyGILState_Ensure(); + PyThreadState *ts = PyEval_SaveThread(); + struct _PyInterpreterFrame *saved = ts->current_frame; + memset(&fake_frame, 0, sizeof(fake_frame)); + fake_frame.owner = FRAME_OWNED_BY_INTERPRETER; + fake_frame.previous = dangling ? NULL : ts->base_frame; + ts->current_frame = &fake_frame; + + pthread_mutex_lock(&lock); + ready = 1; + pthread_cond_signal(&cond); + while (!stopping) { + pthread_cond_wait(&cond, &lock); + } + pthread_mutex_unlock(&lock); + + ts->current_frame = saved; + PyEval_RestoreThread(ts); + PyGILState_Release(gstate); + return NULL; +} + +/* Called through ctypes, which releases the calling thread's GIL. */ +int +start_fake_frame_thread(int is_dangling) +{ + dangling = is_dangling; + ready = 0; + stopping = 0; + if (pthread_create(&helper_thread, NULL, run, NULL) != 0) { + return 1; + } + pthread_mutex_lock(&lock); + while (!ready) { + pthread_cond_wait(&cond, &lock); + } + pthread_mutex_unlock(&lock); + return 0; +} + +void +stop_fake_frame_thread(void) +{ + pthread_mutex_lock(&lock); + stopping = 1; + pthread_cond_signal(&cond); + pthread_mutex_unlock(&lock); + pthread_join(helper_thread, NULL); +} +""" + def _make_test_script(script_dir, script_basename, source): to_return = make_script(script_dir, script_basename, source) @@ -594,6 +666,7 @@ def test_sampling_thread_with_empty_python_stack(self): # actually contain the headers. candidates = [ sysconfig.get_config_var("INCLUDEPY"), + sysconfig.get_config_var("CONFINCLUDEPY"), sysconfig.get_path("include"), sysconfig.get_path("platinclude"), os.path.dirname(sys.executable), @@ -712,6 +785,182 @@ def sample(pin, cache_frames): f"stdout: {result.stdout}\nstderr: {result.stderr}" ) + @skip_if_not_supported + def test_sampling_native_first_frame(self): + # The first frame of a chain may parse to no Python code while a + # valid continuation follows: a thread sitting in a C call that + # owns no Python frame object. Sampling must treat it exactly + # like a mid-chain native frame, and must keep failing loudly on + # a dangling first frame that has no continuation. + script = textwrap.dedent( + """\ + import ctypes + import os + import subprocess + import sys + import sysconfig + import _remote_debugging + + helper_dir = sys.argv[1] + source_path = os.path.join(helper_dir, "native_helper.c") + lib_path = os.path.join(helper_dir, "native_helper.so") + with open(source_path, "w") as f: + f.write(sys.argv[2]) + + cc = sysconfig.get_config_var("CC") or "" + if not cc: + # Windows official builds have no CC config var. + print("NO-COMPILER: sysconfig CC is not set") + sys.exit(42) + # sysconfig's install-style include paths do not necessarily + # exist for build-tree interpreters (out-of-tree builds keep + # the headers in the source and build directories instead), + # so gather candidate directories and keep the ones that + # actually contain the headers. + candidates = [ + sysconfig.get_config_var("INCLUDEPY"), + sysconfig.get_config_var("CONFINCLUDEPY"), + sysconfig.get_path("include"), + sysconfig.get_path("platinclude"), + os.path.dirname(sys.executable), + ] + srcdir = sysconfig.get_config_var("srcdir") + if srcdir: + candidates.append(srcdir) + candidates.append(os.path.join(srcdir, "Include")) + python_h_dirs = set() + pyconfig_h_dirs = set() + for path in candidates: + if path and os.path.isdir(path): + if os.path.isfile(os.path.join(path, "Python.h")): + python_h_dirs.add(path) + if os.path.isfile(os.path.join(path, "pyconfig.h")): + pyconfig_h_dirs.add(path) + if not python_h_dirs or not pyconfig_h_dirs: + print("NO-COMPILER: no include directory with " + "Python.h and pyconfig.h was found") + sys.exit(42) + includes = sorted(python_h_dirs | pyconfig_h_dirs) + internal_header_found = any( + os.path.isfile(os.path.join( + path, "internal", "pycore_interpframe_structs.h")) + for path in includes + ) + if not internal_header_found: + # The synthetic-frame helper needs the internal frame + # layout header, which only build and full installations + # provide. + print("NO-COMPILER: internal/pycore_interpframe_structs.h " + "was not found") + sys.exit(42) + extra = [] + if sys.platform == "darwin": + # Delay Python symbol resolution until the .so is loaded + # into a running interpreter. + extra = ["-undefined", "dynamic_lookup"] + cmd = cc.split() + [ + "-shared", "-fPIC", "-pthread", + *[f"-I{path}" for path in includes], + *extra, + source_path, "-o", lib_path, + ] + try: + subprocess.run(cmd, check=True, capture_output=True, text=True) + except FileNotFoundError: + print("NO-COMPILER: compiler executable not found") + sys.exit(42) + except subprocess.CalledProcessError as exc: + # A real compile failure means the helper rotted: fail loud + # instead of silently skipping. + print(f"HELPER-COMPILE-FAILED: {exc.stderr}") + sys.exit(43) + + lib = ctypes.CDLL(lib_path) + lib.start_fake_frame_thread.argtypes = [ctypes.c_int] + lib.start_fake_frame_thread.restype = ctypes.c_int + lib.stop_fake_frame_thread.argtypes = [] + lib.stop_fake_frame_thread.restype = None + + def first_frame_lists(native, cache_frames): + unwinder = _remote_debugging.RemoteUnwinder( + os.getpid(), all_threads=True, native=native, + cache_frames=cache_frames, + ) + + def sample_once(): + trace = unwinder.get_stack_trace() + return [ + [frame.funcname for frame in thread.frame_info] + for interp in trace + for thread in interp.threads + ] + + # One initial sample stores the (degenerate) stack in the + # frame cache; later samples exercise the cache-hit path. + sample_once() + return [sample_once() for _ in range(3)] + + # The sampled thread list includes the sampling thread itself, + # whose stack is non-empty; the fake-frame thread shows up as + # an empty stack (native=False) or as a lone "" frame + # (native=True) in every sample. + def fake_frame_visible(samples, native): + expected = [""] if native else [] + return all(expected in names for names in samples) + + # A first frame without Python code followed by the sentinel + # is a native-like frame: skipped by default, reported as + # "" when native frames are enabled. + for native in (False, True): + for cache_frames in (False, True): + if lib.start_fake_frame_thread(0) != 0: + raise RuntimeError("failed to start fake-frame thread") + try: + for _ in range(3): + samples = first_frame_lists(native, cache_frames) + assert fake_frame_visible(samples, native), samples + finally: + lib.stop_fake_frame_thread() + # A dangling first frame with no continuation cannot be + # represented in the result: sampling must keep failing loudly. + for cache_frames in (False, True): + if lib.start_fake_frame_thread(1) != 0: + raise RuntimeError("failed to start fake-frame thread") + try: + for _ in range(3): + try: + first_frame_lists(False, cache_frames) + except RuntimeError as exc: + assert "Failed to parse initial frame in chain" \ + in str(exc), exc + else: + raise AssertionError( + "dangling first frame was accepted") + finally: + lib.stop_fake_frame_thread() + """ + ) + + with os_helper.temp_dir() as work_dir: + result = subprocess.run( + [sys.executable, "-c", script, work_dir, + _NATIVE_FIRST_FRAME_HELPER_SOURCE], + capture_output=True, + text=True, + timeout=LONG_TIMEOUT, + ) + if result.returncode == 42: + self.skipTest(f"C compiler not available: {result.stdout}") + if result.returncode == 43: + self.fail( + "native helper failed to compile: " + f"{result.stdout}\n{result.stderr}" + ) + self.assertEqual( + result.returncode, 0, + f"stdout: {result.stdout}\nstderr: {result.stderr}" + ) + @requires_remote_subprocess_debugging() class TestGetStackTrace(RemoteInspectionTestBase): diff --git a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst index c14fa29cee9314..22713b9de37baf 100644 --- a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst +++ b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst @@ -1,5 +1,7 @@ Fix a bug that caused :mod:`!_remote_debugging` sampling to fail entirely with ``RuntimeError: Failed to parse initial frame in chain`` when a native thread retained a Python thread state with an empty Python frame stack. The -base_frame sentinel is now accepted as a valid empty stack, and valid stacks -from other threads are reported instead of being lost. +base_frame sentinel is now accepted as a valid empty stack, and a first +frame without Python code followed by a valid continuation is handled like +a mid-chain native frame instead of failing the sample. Valid stacks from +other threads are now reported instead of being lost. From c3a001665556332fce5faf44c158a700ddb03621 Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Sat, 19 Sep 2026 20:37:09 +0800 Subject: [PATCH 5/6] gh-157605: Discriminate code-less frames by owner field A frame dropped by _PyFrame_ClearExceptCode() keeps its owner and its previous pointer while its contents are cleared, so the unwinder reads it back as a frame with no code object. Because the innermost frame is dropped on every return, this state is common rather than rare, and reporting it as and walking its stale chain produced wrong frames in the collector. is_frame_valid() now reports the owner of a code-less frame back to process_frame_chain(), which stops the walk on any code-less frame whose owner is not FRAME_OWNED_BY_INTERPRETER and reports the frames collected so far. The marker is now only emitted for interpreter-owned frames, i.e. genuine native boundaries. The owner is always read from the local frame buffer, never by dereferencing a remote address. Truncated stacks are still rejected by frame_cache_store() via its last_frame_visited check, so they are never cached. Tests: the fake-frame helper is parameterized by owner, and the test asserts that an interpreter-owned first frame is skipped without native frames and reported as with native frames enabled, while a dropped first frame produces an empty stack for that thread in both cases. The news entry now describes both behaviors. --- Lib/test/test_external_inspection.py | 100 ++++++++---------- ...-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst | 10 +- Modules/_remote_debugging/_remote_debugging.h | 12 ++- Modules/_remote_debugging/asyncio.c | 5 +- Modules/_remote_debugging/frames.c | 98 +++++++++++------ 5 files changed, 131 insertions(+), 94 deletions(-) diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 2abca2d9a52b13..39461fdf0bf68b 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -148,7 +148,7 @@ static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; static int ready = 0; static int stopping = 0; -static int dangling = 0; +static int owner_mode = FRAME_OWNED_BY_INTERPRETER; static _PyInterpreterFrame fake_frame; static void * @@ -158,8 +158,8 @@ PyThreadState *ts = PyEval_SaveThread(); struct _PyInterpreterFrame *saved = ts->current_frame; memset(&fake_frame, 0, sizeof(fake_frame)); - fake_frame.owner = FRAME_OWNED_BY_INTERPRETER; - fake_frame.previous = dangling ? NULL : ts->base_frame; + fake_frame.owner = (char)owner_mode; + fake_frame.previous = ts->base_frame; ts->current_frame = &fake_frame; pthread_mutex_lock(&lock); @@ -178,9 +178,9 @@ /* Called through ctypes, which releases the calling thread's GIL. */ int -start_fake_frame_thread(int is_dangling) +start_fake_frame_thread(int owner) { - dangling = is_dangling; + owner_mode = owner; ready = 0; stopping = 0; if (pthread_create(&helper_thread, NULL, run, NULL) != 0) { @@ -881,63 +881,57 @@ def test_sampling_native_first_frame(self): lib.stop_fake_frame_thread.argtypes = [] lib.stop_fake_frame_thread.restype = None - def first_frame_lists(native, cache_frames): + def sample_all_threads(native, cache_frames): unwinder = _remote_debugging.RemoteUnwinder( os.getpid(), all_threads=True, native=native, cache_frames=cache_frames, ) - - def sample_once(): - trace = unwinder.get_stack_trace() - return [ - [frame.funcname for frame in thread.frame_info] - for interp in trace - for thread in interp.threads - ] - - # One initial sample stores the (degenerate) stack in the - # frame cache; later samples exercise the cache-hit path. - sample_once() - return [sample_once() for _ in range(3)] - - # The sampled thread list includes the sampling thread itself, - # whose stack is non-empty; the fake-frame thread shows up as - # an empty stack (native=False) or as a lone "" frame - # (native=True) in every sample. - def fake_frame_visible(samples, native): - expected = [""] if native else [] - return all(expected in names for names in samples) - - # A first frame without Python code followed by the sentinel - # is a native-like frame: skipped by default, reported as - # "" when native frames are enabled. - for native in (False, True): - for cache_frames in (False, True): - if lib.start_fake_frame_thread(0) != 0: - raise RuntimeError("failed to start fake-frame thread") - try: - for _ in range(3): - samples = first_frame_lists(native, cache_frames) - assert fake_frame_visible(samples, native), samples - finally: - lib.stop_fake_frame_thread() - # A dangling first frame with no continuation cannot be - # represented in the result: sampling must keep failing loudly. - for cache_frames in (False, True): - if lib.start_fake_frame_thread(1) != 0: + trace = unwinder.get_stack_trace() + return [ + [frame.funcname for frame in thread.frame_info] + for interp in trace + for thread in interp.threads + ] + + def run_case(owner, native, cache_frames): + if lib.start_fake_frame_thread(owner) != 0: raise RuntimeError("failed to start fake-frame thread") try: for _ in range(3): - try: - first_frame_lists(False, cache_frames) - except RuntimeError as exc: - assert "Failed to parse initial frame in chain" \ - in str(exc), exc - else: - raise AssertionError( - "dangling first frame was accepted") + yield sample_all_threads(native, cache_frames) finally: lib.stop_fake_frame_thread() + + # Frame owner constants from _PyInterpreterFrameOwner: + # THREAD=0, GENERATOR=1, FRAME_OBJECT=2, INTERPRETER=3. + # + # owner=INTERPRETER with a continuation is a native boundary: + # skipped by default, "" with native frames enabled. + INTERPRETER_OWNER = 3 + THREAD_OWNER = 0 + for native in (False, True): + for cache_frames in (False, True): + for names in run_case(INTERPRETER_OWNER, native, + cache_frames): + expected = [""] if native else [] + assert expected in names, (native, cache_frames, names) + + # owner=FRAME_OWNED_BY_THREAD with no code object models a + # frame dropped by _PyFrame_ClearExceptCode(): the contents + # are gone but the owner and the previous pointer survive, so + # the chain is stale. The dropped frame must not be reported + # as and its stale chain must not be walked: the + # thread shows an empty stack in every sample. (The sampling + # thread itself legitimately shows a boundary when + # native frames are enabled, so only this thread's entry is + # asserted here.) Before the owner check, this configuration + # produced a marker followed by dropped frames. + for cache_frames in (False, True): + for names in run_case(THREAD_OWNER, True, cache_frames): + assert [] in names, names + for cache_frames in (False, True): + for names in run_case(THREAD_OWNER, False, cache_frames): + assert [] in names, names """ ) diff --git a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst index 22713b9de37baf..145d19e9ffc7d7 100644 --- a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst +++ b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst @@ -1,7 +1,9 @@ Fix a bug that caused :mod:`!_remote_debugging` sampling to fail entirely with ``RuntimeError: Failed to parse initial frame in chain`` when a native thread retained a Python thread state with an empty Python frame stack. The -base_frame sentinel is now accepted as a valid empty stack, and a first -frame without Python code followed by a valid continuation is handled like -a mid-chain native frame instead of failing the sample. Valid stacks from -other threads are now reported instead of being lost. +base_frame sentinel is now accepted as a valid empty stack, and a frame +without Python code (a returned-from frame dropped by +:c:func:`!_PyFrame_ClearExceptCode` or a native boundary) is no longer +reported as ```` with its stale chain walked: sampling now stops +there and reports the frames collected so far. Valid stacks from other +threads are now reported instead of being lost. diff --git a/Modules/_remote_debugging/_remote_debugging.h b/Modules/_remote_debugging/_remote_debugging.h index fa37fb7b2167ec..062b0d9bc1f6fc 100644 --- a/Modules/_remote_debugging/_remote_debugging.h +++ b/Modules/_remote_debugging/_remote_debugging.h @@ -451,7 +451,8 @@ typedef struct { Py_ssize_t num_addrs; // Count of addresses collected Py_ssize_t max_addrs; // Capacity of frame_addrs array uintptr_t last_frame_visited; // Last frame address visited - int stopped_at_cached_frame; // Whether we stopped at cached frame + int stopped_at_cached_frame; // Walk stopped at a cached frame + int stopped_at_policy_frame; // Walk stopped at a policy-accepted first frame } FrameWalkContext; /* @@ -592,7 +593,8 @@ extern int cache_tlbc_array(RemoteUnwinderObject *unwinder, uintptr_t code_addr, extern int is_frame_valid( RemoteUnwinderObject *unwinder, uintptr_t frame_addr, - uintptr_t code_object_addr + uintptr_t code_object_addr, + char *owner_out ); extern int parse_frame_object( @@ -600,7 +602,8 @@ extern int parse_frame_object( PyObject** result, uintptr_t address, uintptr_t* address_of_code_object, - uintptr_t* previous_frame + uintptr_t* previous_frame, + char *owner_out ); extern int parse_frame_from_chunks( @@ -609,7 +612,8 @@ extern int parse_frame_from_chunks( uintptr_t address, uintptr_t *previous_frame, uintptr_t *stackpointer, - StackChunkList *chunks + StackChunkList *chunks, + char *owner_out ); /* Stack chunk management */ diff --git a/Modules/_remote_debugging/asyncio.c b/Modules/_remote_debugging/asyncio.c index 44a9a3cbce0061..a80e956aa010f1 100644 --- a/Modules/_remote_debugging/asyncio.c +++ b/Modules/_remote_debugging/asyncio.c @@ -354,7 +354,7 @@ parse_coro_chain( uintptr_t prev_frame; uintptr_t gi_iframe_addr = coro_address + (uintptr_t)unwinder->debug_offsets.gen_object.gi_iframe; uintptr_t address_of_code_object = 0; - if (parse_frame_object(unwinder, &name, gi_iframe_addr, &address_of_code_object, &prev_frame) < 0) { + if (parse_frame_object(unwinder, &name, gi_iframe_addr, &address_of_code_object, &prev_frame, NULL) < 0) { set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse frame object in coro chain"); return -1; } @@ -784,7 +784,8 @@ parse_async_frame_chain( &frame_info, address_of_current_frame, &address_of_code_object, - &address_of_current_frame + &address_of_current_frame, + NULL ); if (res < 0) { diff --git a/Modules/_remote_debugging/frames.c b/Modules/_remote_debugging/frames.c index 52c93ec060355a..8213ad5bfb5eb4 100644 --- a/Modules/_remote_debugging/frames.c +++ b/Modules/_remote_debugging/frames.c @@ -167,15 +167,19 @@ int is_frame_valid( RemoteUnwinderObject *unwinder, uintptr_t frame_addr, - uintptr_t code_object_addr + uintptr_t code_object_addr, + char *owner_out ) { + void* frame = (void*)frame_addr; + + char owner = GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner); + if (owner_out != NULL) { + *owner_out = owner; + } if ((void*)code_object_addr == NULL) { return 0; } - void* frame = (void*)frame_addr; - - char owner = GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner); if (owner == FRAME_OWNED_BY_INTERPRETER) { return 0; // C frame or sentinel base frame } @@ -194,13 +198,14 @@ parse_frame_buffer( PyObject** result, const char *frame, uintptr_t* address_of_code_object, - uintptr_t* previous_frame + uintptr_t* previous_frame, + char *owner_out ) { *address_of_code_object = 0; *previous_frame = GET_MEMBER(uintptr_t, frame, unwinder->debug_offsets.interpreter_frame.previous); uintptr_t code_object = GET_MEMBER_NO_TAG(uintptr_t, frame, unwinder->debug_offsets.interpreter_frame.executable); - int frame_valid = is_frame_valid(unwinder, (uintptr_t)frame, code_object); + int frame_valid = is_frame_valid(unwinder, (uintptr_t)frame, code_object, owner_out); if (frame_valid != 1) { return frame_valid; } @@ -231,7 +236,8 @@ parse_frame_object( PyObject** result, uintptr_t address, uintptr_t* address_of_code_object, - uintptr_t* previous_frame + uintptr_t* previous_frame, + char *owner_out ) { char frame[SIZEOF_INTERP_FRAME]; Py_ssize_t bytes_read = _Py_RemoteDebug_ReadRemoteMemory( @@ -247,7 +253,7 @@ parse_frame_object( STATS_INC(unwinder, memory_reads); STATS_ADD(unwinder, memory_bytes_read, SIZEOF_INTERP_FRAME); - return parse_frame_buffer(unwinder, result, frame, address_of_code_object, previous_frame); + return parse_frame_buffer(unwinder, result, frame, address_of_code_object, previous_frame, owner_out); } int @@ -257,7 +263,8 @@ parse_frame_from_chunks( uintptr_t address, uintptr_t *previous_frame, uintptr_t *stackpointer, - StackChunkList *chunks + StackChunkList *chunks, + char *owner_out ) { void *frame_ptr = find_frame_in_chunks(chunks, address); if (!frame_ptr) { @@ -270,7 +277,7 @@ parse_frame_from_chunks( *previous_frame = GET_MEMBER(uintptr_t, frame, unwinder->debug_offsets.interpreter_frame.previous); *stackpointer = GET_MEMBER(uintptr_t, frame, unwinder->debug_offsets.interpreter_frame.stackpointer); uintptr_t code_object = GET_MEMBER_NO_TAG(uintptr_t, frame_ptr, unwinder->debug_offsets.interpreter_frame.executable); - int frame_valid = is_frame_valid(unwinder, (uintptr_t)frame, code_object); + int frame_valid = is_frame_valid(unwinder, (uintptr_t)frame, code_object, owner_out); if (frame_valid != 1) { return frame_valid; } @@ -310,12 +317,14 @@ process_frame_chain( assert(MAX_FRAMES > 0 && MAX_FRAMES < 10000); ctx->stopped_at_cached_frame = 0; + ctx->stopped_at_policy_frame = 0; ctx->last_frame_visited = 0; while ((void*)frame_addr != NULL) { PyObject *frame = NULL; uintptr_t next_frame_addr = 0; uintptr_t stackpointer = 0; + char frame_owner = -1; last_frame_addr = frame_addr; if (++frame_count > MAX_FRAMES) { @@ -326,7 +335,7 @@ process_frame_chain( assert(frame_count <= MAX_FRAMES); if (ctx->chunks && ctx->chunks->count > 0) { - if (parse_frame_from_chunks(unwinder, &frame, frame_addr, &next_frame_addr, &stackpointer, ctx->chunks) == 0) { + if (parse_frame_from_chunks(unwinder, &frame, frame_addr, &next_frame_addr, &stackpointer, ctx->chunks, &frame_owner) == 0) { goto parsed_frame; } PyErr_Clear(); @@ -337,12 +346,12 @@ process_frame_chain( if (ctx->prefetch.frame && ctx->prefetch.frame_addr == frame_addr) { parse_result = parse_frame_buffer( unwinder, &frame, ctx->prefetch.frame, - &address_of_code_object, &next_frame_addr); + &address_of_code_object, &next_frame_addr, &frame_owner); } else { parse_result = parse_frame_object( unwinder, &frame, frame_addr, - &address_of_code_object, &next_frame_addr); + &address_of_code_object, &next_frame_addr, &frame_owner); } if (parse_result < 0) { set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to parse frame object in chain"); @@ -359,36 +368,50 @@ process_frame_chain( } if (frame == NULL && PyList_GET_SIZE(ctx->frame_info) == 0) { - if (frame_addr == ctx->base_frame_addr) { - // A native thread that released the GIL with - // PyEval_SaveThread() keeps a thread state whose frame - // chain holds only the base_frame sentinel. This is a - // valid empty Python stack: accept it and let other - // threads report their stacks. + // A first frame without a code object: the base_frame + // sentinel of an empty Python stack, a frame dropped by + // _PyFrame_ClearExceptCode() (which keeps the owner and the + // previous pointer), or a genuine interpreter-owned native + // boundary. Only the owner distinguishes them. + if (frame_owner != FRAME_OWNED_BY_INTERPRETER) { + // A dropped frame: not a native boundary, and its chain + // is stale. Accept the empty stack and stop instead of + // reporting it as and walking dropped frames. + ctx->stopped_at_policy_frame = 1; break; } if (next_frame_addr == 0) { - // A dangling undecodable first frame with no continuation - // cannot be represented in the result. - const char *e = "Failed to parse initial frame in chain"; - PyErr_SetString(PyExc_RuntimeError, e); - return -1; + // The base_frame sentinel: a valid empty Python stack. + // Accept it and let other threads report their stacks. + ctx->stopped_at_policy_frame = 1; + break; } - // A first frame without Python code but with a valid - // continuation (for example a thread sitting in a C call - // between Python frames) is handled exactly like a mid-chain - // native frame below. + // An interpreter-owned frame with a continuation is a native + // boundary; handle it like a mid-chain native frame below. } PyObject *extra_frame = NULL; + if (frame == NULL && frame_owner != FRAME_OWNED_BY_INTERPRETER) { + // A frame dropped by _PyFrame_ClearExceptCode() mid-chain: + // its chain is stale, so stop here and report what was + // collected so far. + ctx->stopped_at_policy_frame = 1; + break; + } if (unwinder->gc && frame_addr == ctx->gc_frame) { _Py_DECLARE_STR(gc, ""); extra_frame = &_Py_STR(gc); } else if (unwinder->native && frame == NULL && + frame_owner == FRAME_OWNED_BY_INTERPRETER && next_frame_addr && !(unwinder->gc && next_frame_addr == ctx->gc_frame)) { + // Only an interpreter-owned frame with no code object is a + // native boundary. A frame dropped by + // _PyFrame_ClearExceptCode() keeps a THREAD/GENERATOR owner, + // so it is never reported as and its stale chain is + // not walked. _Py_DECLARE_STR(native, ""); extra_frame = &_Py_STR(native); } @@ -441,7 +464,12 @@ process_frame_chain( frame_addr = next_frame_addr; } - if (!ctx->stopped_at_cached_frame && ctx->base_frame_addr != 0 && last_frame_addr != ctx->base_frame_addr) { + // A policy stop on a dropped or interpreter-owned first frame ends + // the walk before the sentinel: the reported stack is complete as of + // the accepted truncation point. + if (!ctx->stopped_at_cached_frame && !ctx->stopped_at_policy_frame && + ctx->base_frame_addr != 0 && last_frame_addr != ctx->base_frame_addr) + { PyErr_Format(PyExc_RuntimeError, "Incomplete sample: did not reach base frame (expected 0x%lx, got 0x%lx)", ctx->base_frame_addr, last_frame_addr); @@ -563,11 +591,13 @@ try_full_cache_hit( if (ctx->prefetch.frame && ctx->prefetch.frame_addr == ctx->frame_addr) { parse_result = parse_frame_buffer(unwinder, ¤t_frame, ctx->prefetch.frame, - &code_object_addr, &previous_frame); + &code_object_addr, &previous_frame, + NULL); } else { parse_result = parse_frame_object(unwinder, ¤t_frame, ctx->frame_addr, - &code_object_addr, &previous_frame); + &code_object_addr, &previous_frame, + NULL); } if (parse_result < 0) { return -1; @@ -677,6 +707,12 @@ collect_frames_with_cache( } } + if (ctx->stopped_at_policy_frame) { + // No additional handling needed: frame_cache_store() rejects + // stacks whose walk did not end at the sentinel, so truncated + // chains are never cached. + } + if (frame_cache_store(unwinder, thread_id, ctx->frame_info, ctx->frame_addrs, ctx->num_addrs, ctx->thread_state_addr, ctx->last_profiled.seq, ctx->base_frame_addr, From 2c58f5715e89909ecc27863e5c38f083f6d95067 Mon Sep 17 00:00:00 2001 From: aidaodedjl <2623157945@qq.com> Date: Sat, 19 Sep 2026 20:55:13 +0800 Subject: [PATCH 6/6] gh-157605: Skip dropped frames and keep walking the live caller chain A frame dropped by _PyFrame_ClearExceptCode() keeps its previous pointer while its contents are cleared, and the pointer still leads to live caller frames. Rather than reporting such frames as and walking them as if they were boundaries, the chain walk now skips any code-less frame whose owner is not FRAME_OWNED_BY_INTERPRETER and continues to the live callers; only interpreter-owned frames are native boundaries and keep the marker. A dangling code-less first frame with no continuation still fails the sample loudly. The owner is always read from the local frame buffer, never by dereferencing a remote address, and the walk-stop of the previous iteration is replaced by the plain last_frame_visited check in frame_cache_store(), which already rejects stacks that did not reach the sentinel. Tests: the fake-frame helper is parameterized by owner; the test locks in empty-stack acceptance for the sentinel, the marker for interpreter-owned boundaries, and skip-past-dropped-frame behavior for dropped frames, with and without frame caching. Reviewed with liuzhijie-0614 in gh-157605. --- Lib/test/test_external_inspection.py | 18 +++--- ...-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst | 12 ++-- Modules/_remote_debugging/_remote_debugging.h | 1 - Modules/_remote_debugging/frames.c | 58 ++++++++----------- 4 files changed, 41 insertions(+), 48 deletions(-) diff --git a/Lib/test/test_external_inspection.py b/Lib/test/test_external_inspection.py index 39461fdf0bf68b..0f0c8d166035e7 100644 --- a/Lib/test/test_external_inspection.py +++ b/Lib/test/test_external_inspection.py @@ -918,14 +918,16 @@ def run_case(owner, native, cache_frames): # owner=FRAME_OWNED_BY_THREAD with no code object models a # frame dropped by _PyFrame_ClearExceptCode(): the contents - # are gone but the owner and the previous pointer survive, so - # the chain is stale. The dropped frame must not be reported - # as and its stale chain must not be walked: the - # thread shows an empty stack in every sample. (The sampling - # thread itself legitimately shows a boundary when - # native frames are enabled, so only this thread's entry is - # asserted here.) Before the owner check, this configuration - # produced a marker followed by dropped frames. + # are gone but the previous pointer still leads to live + # caller frames. The dropped frame itself must not be + # reported as , and the walk must continue past it: + # the thread shows an empty stack here because its only + # other chain entry is the sentinel. (The sampling thread + # itself legitimately shows a boundary when native + # frames are enabled, so only this thread's entry is + # asserted here.) Before the owner check, this + # configuration produced a marker for the dropped + # frame. for cache_frames in (False, True): for names in run_case(THREAD_OWNER, True, cache_frames): assert [] in names, names diff --git a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst index 145d19e9ffc7d7..21b4f8aa03b90f 100644 --- a/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst +++ b/Misc/NEWS.d/next/Library/2026-09-17-12-28-39.gh-issue-157605.nv7Qz8.rst @@ -1,9 +1,9 @@ Fix a bug that caused :mod:`!_remote_debugging` sampling to fail entirely with ``RuntimeError: Failed to parse initial frame in chain`` when a native thread retained a Python thread state with an empty Python frame stack. The -base_frame sentinel is now accepted as a valid empty stack, and a frame -without Python code (a returned-from frame dropped by -:c:func:`!_PyFrame_ClearExceptCode` or a native boundary) is no longer -reported as ```` with its stale chain walked: sampling now stops -there and reports the frames collected so far. Valid stacks from other -threads are now reported instead of being lost. +base_frame sentinel is now accepted as a valid empty stack, and code-less +frames on the chain (returned-from frames dropped by +:c:func:`!_PyFrame_ClearExceptCode`) are skipped instead of being reported +as ```` with stale caller frames. A dangling code-less first frame +still fails the sample loudly. Valid stacks from other threads are now +reported instead of being lost. diff --git a/Modules/_remote_debugging/_remote_debugging.h b/Modules/_remote_debugging/_remote_debugging.h index 062b0d9bc1f6fc..5702042e5b2a8a 100644 --- a/Modules/_remote_debugging/_remote_debugging.h +++ b/Modules/_remote_debugging/_remote_debugging.h @@ -452,7 +452,6 @@ typedef struct { Py_ssize_t max_addrs; // Capacity of frame_addrs array uintptr_t last_frame_visited; // Last frame address visited int stopped_at_cached_frame; // Walk stopped at a cached frame - int stopped_at_policy_frame; // Walk stopped at a policy-accepted first frame } FrameWalkContext; /* diff --git a/Modules/_remote_debugging/frames.c b/Modules/_remote_debugging/frames.c index 8213ad5bfb5eb4..5f4277bf608303 100644 --- a/Modules/_remote_debugging/frames.c +++ b/Modules/_remote_debugging/frames.c @@ -317,7 +317,6 @@ process_frame_chain( assert(MAX_FRAMES > 0 && MAX_FRAMES < 10000); ctx->stopped_at_cached_frame = 0; - ctx->stopped_at_policy_frame = 0; ctx->last_frame_visited = 0; while ((void*)frame_addr != NULL) { @@ -368,34 +367,38 @@ process_frame_chain( } if (frame == NULL && PyList_GET_SIZE(ctx->frame_info) == 0) { - // A first frame without a code object: the base_frame - // sentinel of an empty Python stack, a frame dropped by - // _PyFrame_ClearExceptCode() (which keeps the owner and the - // previous pointer), or a genuine interpreter-owned native - // boundary. Only the owner distinguishes them. - if (frame_owner != FRAME_OWNED_BY_INTERPRETER) { - // A dropped frame: not a native boundary, and its chain - // is stale. Accept the empty stack and stop instead of - // reporting it as and walking dropped frames. - ctx->stopped_at_policy_frame = 1; + if (frame_addr == ctx->base_frame_addr) { + // The base_frame sentinel of an empty Python stack: a + // native thread that released the GIL with + // PyEval_SaveThread() keeps a thread state whose frame + // chain holds only the sentinel. Accept the empty stack + // and let other threads report their stacks. break; } if (next_frame_addr == 0) { - // The base_frame sentinel: a valid empty Python stack. - // Accept it and let other threads report their stacks. - ctx->stopped_at_policy_frame = 1; - break; + // A dangling undecodable first frame with no + // continuation cannot be represented in the result. + // Note: an interpreter-owned frame in this state is + // indistinguishable from corruption, so the sample fails + // loudly rather than reporting a truncated stack. + const char *e = "Failed to parse initial frame in chain"; + PyErr_SetString(PyExc_RuntimeError, e); + return -1; } - // An interpreter-owned frame with a continuation is a native - // boundary; handle it like a mid-chain native frame below. + // Any other code-less first frame with a continuation is + // handled by the owner check below: an interpreter-owned + // frame is a native boundary, a dropped frame (owner + // THREAD/GENERATOR) is skipped, and the walk continues. } PyObject *extra_frame = NULL; if (frame == NULL && frame_owner != FRAME_OWNED_BY_INTERPRETER) { // A frame dropped by _PyFrame_ClearExceptCode() mid-chain: - // its chain is stale, so stop here and report what was - // collected so far. - ctx->stopped_at_policy_frame = 1; - break; + // the contents are gone but the previous pointer still leads + // to live caller frames, so skip this frame and keep + // walking. + frame_addr = next_frame_addr; + prev_frame_addr = next_frame_addr; + continue; } if (unwinder->gc && frame_addr == ctx->gc_frame) { _Py_DECLARE_STR(gc, ""); @@ -464,12 +467,7 @@ process_frame_chain( frame_addr = next_frame_addr; } - // A policy stop on a dropped or interpreter-owned first frame ends - // the walk before the sentinel: the reported stack is complete as of - // the accepted truncation point. - if (!ctx->stopped_at_cached_frame && !ctx->stopped_at_policy_frame && - ctx->base_frame_addr != 0 && last_frame_addr != ctx->base_frame_addr) - { + if (!ctx->stopped_at_cached_frame && ctx->base_frame_addr != 0 && last_frame_addr != ctx->base_frame_addr) { PyErr_Format(PyExc_RuntimeError, "Incomplete sample: did not reach base frame (expected 0x%lx, got 0x%lx)", ctx->base_frame_addr, last_frame_addr); @@ -707,12 +705,6 @@ collect_frames_with_cache( } } - if (ctx->stopped_at_policy_frame) { - // No additional handling needed: frame_cache_store() rejects - // stacks whose walk did not end at the sentinel, so truncated - // chains are never cached. - } - if (frame_cache_store(unwinder, thread_id, ctx->frame_info, ctx->frame_addrs, ctx->num_addrs, ctx->thread_state_addr, ctx->last_profiled.seq, ctx->base_frame_addr,