Skip to content

feat: Support optional stack capture size limit - #172

Open
cerisier wants to merge 3 commits into
getsentry:getsentryfrom
cerisier:cerisier/linux-stack-capture-limit
Open

cerisier wants to merge 3 commits into
getsentry:getsentryfrom
cerisier:cerisier/linux-stack-capture-limit

Conversation

@cerisier

@cerisier cerisier commented Sep 25, 2026 •

Copy link
Copy Markdown

Follow-up to #165 with optional behavior that preserves the crashing thread's full stack.

This is motivated because Zig programs place a 256 KiB alternate signal stack in static TLS for every thread. On Linux/glibc, Crashpad includes that TLS in each worker's stack capture leading to the dump limit being exhausted quite fast with growing number of threads.

In the case of http://github.com/zml serving stack (LLMd) written in Zig, we hit this limitation almost instantly.

It kept the same general idea of #165 while making the limit optional and applying it only to non-crashing threads. This follows the precedent of #137 and getsentry/sentry-native#1427 where opt-in settings seem to be a favored approach.

Tracking parent PR: getsentry/sentry-native#2137

@cerisier
cerisier marked this pull request as ready for review September 25, 2026 17:30
Comment on lines +171 to 175
}

// If non-default values have been found for all options, the loop can end
// early.
if (local_options.crashpad_handler_behavior != TriState::kUnset &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The loop's early termination condition incorrectly requires max_stack_capture_size to be non-zero, even though 0 is a valid default value, causing a performance regression.
Severity: MEDIUM

Suggested Fix

Remove the max_stack_capture_size != 0 check from the loop's early termination condition. The loop should break once the three TriState options (crashpad_handler_behavior, system_crash_reporter_forwarding, and gather_indirectly_referenced_memory) are resolved, consistent with the existing pattern for other optional numeric values.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: snapshot/linux/process_snapshot_linux.cc#L171-L175

Potential issue: The early termination condition for the module iteration loop at lines
175-178 is logically incorrect. It now requires `max_stack_capture_size != 0` to break
the loop. However, `max_stack_capture_size = 0` is a valid default value indicating no
stack capture limit. This change forces the loop to iterate through all loaded modules
even when all other configuration options have been resolved, causing a performance
regression. This happens in the common case where a process has multiple modules and
does not explicitly set a `max_stack_capture_size`. The existing pattern for
`indirectly_referenced_memory_cap` correctly avoids including the numeric value in the
termination condition.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indirectly_referenced_memory_cap has a companion TriState that tells us whether the value was resolved. This option currently doesn’t. I could use the same TriState pattern if preferred, but simply removing the check could skip a value set by a later module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant