From 84e462182990d2be30f245a195f6338f6707af6a Mon Sep 17 00:00:00 2001
From: lingfeng-guan-glean <lingfeng.guan@glean.com>
Date: Thu, 16 Jul 2026 20:22:58 -0700
Subject: [PATCH] Bound the SIGPROF frame walk to prevent an infinite loop

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.
---
 googlecloudprofiler/__version__.py         |  7 +++---
 googlecloudprofiler/src/populate_frames.cc | 29 +++++++++++++++++++---
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/googlecloudprofiler/__version__.py b/googlecloudprofiler/__version__.py
index 02d34d4..22285ae 100644
--- a/googlecloudprofiler/__version__.py
+++ b/googlecloudprofiler/__version__.py
@@ -17,6 +17,7 @@
 
 # setup.py reads the version information from here to set package version
 # Glean fork: upstream 4.1.0 + 3.12/3.13 support (vendored as 4.2.0) + the
-# SIGPROF frame-walk crash fix (copy-the-chain). Patch bump over the synthetic
-# 4.2.0 -- the fix is a backward-compatible bug fix. Upstream has no 4.2.x.
-__version__ = '4.2.1'
+# SIGPROF frame-walk crash fix (copy-the-chain, 4.2.1) + a bound on the frame
+# walk to prevent an infinite loop on a torn/cyclic `previous` chain (4.2.2).
+# Backward-compatible bug fixes; upstream has no 4.2.x.
+__version__ = '4.2.2'
diff --git a/googlecloudprofiler/src/populate_frames.cc b/googlecloudprofiler/src/populate_frames.cc
index 47f492d..24a40b9 100644
--- a/googlecloudprofiler/src/populate_frames.cc
+++ b/googlecloudprofiler/src/populate_frames.cc
@@ -32,6 +32,14 @@ bool SafeCopy(void *dst, const void *src, size_t n) {
   return got == static_cast<long>(n);
 }
 
+// Absolute cap on frames walked per SIGPROF sample. num_frames only advances for
+// complete frames, so a cycle of incomplete/stale frames -- e.g. a torn
+// `previous` link left by a SIGPROF landing mid frame push/pop -- would otherwise
+// spin this walk forever inside the signal handler (GIL held), wedging the
+// process. 4x the capture cap leaves headroom for legitimately skipped frames
+// (C-stack shims, prologue frames).
+static const int kMaxWalkIterations = 4 * kMaxFramesToCapture;
+
 #if PY_VERSION_HEX >= PY_313
 
 /**
@@ -92,7 +100,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
 
   _PyInterpreterFrame *faddr = ts->current_frame;
   int num_frames = 0;
-  while (faddr != nullptr && num_frames < kMaxFramesToCapture) {
+  int steps = 0;
+  while (faddr != nullptr && num_frames < kMaxFramesToCapture &&
+         ++steps <= kMaxWalkIterations) {
     _PyInterpreterFrame fr;
     if (!SafeCopy(&fr, faddr, sizeof(fr))) {
       break;  // unreadable frame: stop, keep the frames gathered so far
@@ -108,6 +118,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
           (fr.instr_ptr - _PyCode_CODE(code)) * sizeof(_Py_CODEUNIT));
       num_frames++;
     }
+    if (fr.previous == faddr) {
+      break;  // self-referential link: stop instead of spinning
+    }
     faddr = fr.previous;
   }
   return num_frames;
@@ -174,7 +187,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
   }
 
   int num_frames = 0;
-  while (faddr != nullptr && num_frames < kMaxFramesToCapture) {
+  int steps = 0;
+  while (faddr != nullptr && num_frames < kMaxFramesToCapture &&
+         ++steps <= kMaxWalkIterations) {
     _PyInterpreterFrame fr;
     if (!SafeCopy(&fr, faddr, sizeof(fr))) {
       break;  // unreadable frame: stop, keep the frames gathered so far
@@ -188,6 +203,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
           (fr.prev_instr - _PyCode_CODE(fr.f_code)) * sizeof(_Py_CODEUNIT));
       num_frames++;
     }
+    if (fr.previous == faddr) {
+      break;  // self-referential link: stop instead of spinning
+    }
     faddr = fr.previous;
   }
   return num_frames;
@@ -248,7 +266,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
   }
 
   int num_frames = 0;
-  while (faddr != nullptr && num_frames < kMaxFramesToCapture) {
+  int steps = 0;
+  while (faddr != nullptr && num_frames < kMaxFramesToCapture &&
+         ++steps <= kMaxWalkIterations) {
     _PyInterpreterFrame fr;
     if (!SafeCopy(&fr, faddr, sizeof(fr))) {
       break;  // unreadable frame: stop, keep the frames gathered so far
@@ -262,6 +282,9 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) {
           (fr.prev_instr - _PyCode_CODE(fr.f_code)) * sizeof(_Py_CODEUNIT));
       num_frames++;
     }
+    if (fr.previous == faddr) {
+      break;  // self-referential link: stop instead of spinning
+    }
     faddr = fr.previous;
   }
   return num_frames;