Bound the SIGPROF frame walk to prevent an infinite loop (4.2.2) - #1
Closed
lingfeng-guan-glean wants to merge 1 commit into
Closed
Bound the SIGPROF frame walk to prevent an infinite loop (4.2.2)#1lingfeng-guan-glean wants to merge 1 commit into
lingfeng-guan-glean wants to merge 1 commit into
Conversation
PopulateFrames walked the interpreter frame chain bounded only by num_frames < kMaxFramesToCapture, but num_frames advances only for complete frames. A SIGPROF landing mid frame push/pop can leave a torn/stale `previous` link forming a cycle of incomplete frames; the walk then spins forever inside the signal handler holding the GIL, wedging the process (liveness probe timeout -> restart). Add an absolute step cap (kMaxWalkIterations = 4 * kMaxFramesToCapture) plus a self-loop fast-path to the 3.11/3.12/3.13 branches. The pre-3.11 branch increments unconditionally and is unaffected. Bump 4.2.1 -> 4.2.2.
Owner
Author
|
Superseded: reopened against the Glean fork (rahul-roy-glean) stacked on the merged SafeCopy PR rahul-roy-glean#2. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PopulateFrames(googlecloudprofiler/src/populate_frames.cc) walks the interpreter frame chain with the loop bounded only bynum_frames < kMaxFramesToCapture. Butnum_framesadvances only for complete frames (fr.f_code != nullptr && !FrameIsIncomplete(&fr)). Incomplete frames — a frame caught mid-push in its prologue,FRAME_OWNED_BY_CSTACKshims, null/unreadable code — are correctly skipped (so real stacks aren't truncated), but they don't advance the counter.A SIGPROF can land while CPython is pushing/popping a frame. CPython's
_PyFrame_Initializedeliberately leavespreviousunwritten (linked later) and a mid-push frame is incomplete by construction; reused datastack memory supplies a staleprevious. That can form a cycle of incomplete frames. Because none of them advancenum_frames, the existing cap never trips and the walk spins forever inside the SIGPROF handler — on the GIL-holding thread. The process wedges (~1 core, GIL held); in k8s the liveness probe times out and the pod is restarted.The existing
num_framescap only bounds cycles that contain at least one complete frame; an all-incomplete cycle is unbounded.Fix
kMaxWalkIterations = 4 * kMaxFramesToCapture(512) — guarantees termination regardless of frame completeness.if (fr.previous == faddr) break;self-loop fast-path for the trivial 1-cycle.num_framesunconditionally and is unaffected.SafeCopy-failurebreak.Skipping incomplete frames is preserved (breaking on them would truncate legitimate stacks at shim/prologue frames); the only change is that the walk can never exceed 512 total steps.
Version
4.2.1 -> 4.2.2 (backward-compatible bug fix).
Notes
Stacked on
lg/fix-frame-walk-copy(the deployed 4.2.1 line) so the diff is isolated to this fix. Distinct from the SafeCopy crash-fix (that prevents SIGSEGV on a bad pointer; this prevents an infinite loop on a cyclic chain).