-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
gh-138122: Validate base frame before caching in remote debugging frame cache #142852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d3ffd62 to
224dede
Compare
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 224dede 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142852%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Lib/test/test_profiling/test_sampling_profiler/test_integration.py
Outdated
Show resolved
Hide resolved
Lib/test/test_profiling/test_sampling_profiler/test_integration.py
Outdated
Show resolved
Hide resolved
Lib/test/test_profiling/test_sampling_profiler/test_integration.py
Outdated
Show resolved
Hide resolved
Lib/test/test_profiling/test_sampling_profiler/test_integration.py
Outdated
Show resolved
Hide resolved
2da04f7 to
f31e71d
Compare
…ng frame cache The frame cache in the remote debugging module was storing frame chains without validating that they reached the base frame. This could happen when a frame chain was interrupted or when the process state changed during reading, resulting in incomplete stacks being cached. Subsequent samples that hit the cache would then produce flamegraphs that didn't reach the bottom of the call stack. The fix passes base_frame_addr through to process_frame_chain() which already has validation logic to ensure the frame walk terminates at the expected sentinel frame. By enabling this validation in the caching code path and tracking whether we've confirmed reaching the base frame, we now only store complete frame chains in the cache. When extending from cached data, we trust that the cached frames were already validated at storage time, maintaining the invariant that cached stacks are always complete. An integration test using deeply nested generators that oscillate the stack depth is added to verify that all sampled stacks contain the entry point function. This catches regressions where incomplete stacks might be cached and returned.
f31e71d to
2cd7686
Compare
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 2cd7686 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142852%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
The frame cache in the remote debugging module was storing frame chains
without validating that they reached the base frame. This could happen
when a frame chain was interrupted or when the process state changed
during reading, resulting in incomplete stacks being cached. Subsequent
samples that hit the cache would then produce flamegraphs that didn't
reach the bottom of the call stack.
The fix passes base_frame_addr through to process_frame_chain() which
already has validation logic to ensure the frame walk terminates at the
expected sentinel frame. By enabling this validation in the caching code
path and tracking whether we've confirmed reaching the base frame, we
now only store complete frame chains in the cache. When extending from
cached data, we trust that the cached frames were already validated at
storage time, maintaining the invariant that cached stacks are always
complete.
An integration test using deeply nested generators that oscillate the
stack depth is added to verify that all sampled stacks contain the entry
point function. This catches regressions where incomplete stacks might
be cached and returned.