Skip to content

ref: make SENTRY_HANDLER_STACK_SIZE overridable via env var#1636

Merged
jpnurmi merged 2 commits intomasterfrom
jpnurmi/ref/handler-stack-size
Apr 13, 2026
Merged

ref: make SENTRY_HANDLER_STACK_SIZE overridable via env var#1636
jpnurmi merged 2 commits intomasterfrom
jpnurmi/ref/handler-stack-size

Conversation

@jpnurmi
Copy link
Copy Markdown
Collaborator

@jpnurmi jpnurmi commented Apr 10, 2026

Allow overriding the compile-time SENTRY_HANDLER_STACK_SIZE default at runtime via environment variable. The stack-overflow integration tests previously parametrized SENTRY_HANDLER_STACK_SIZE as a CMake option, forcing 5 extra configure+build cycles on Windows (crashpad x2, inproc x1, breakpad x2). Switching to a runtime env var lets every parametrization reuse the cached build, saving a minute or two on Windows.

Before (s) After (s)
test_crashpad_dumping_stack_overflow (default/16/32) 1.56 / 61.44 / 62.19 1.57 / 1.67 / 1.58
test_inproc_stack_overflow_stdout (default/32) 1.61 / 20.33 1.86 / 1.95
test_breakpad_stack_overflow_stdout (default/16/32) 0.12 / 21.20 / 21.57 0.13 / 0.11 / 0.12

#skip-changelog

@supervacuus
Copy link
Copy Markdown
Collaborator

Just a quick heads-up: the reason this is a build-time variable is its use inside DllMain, where we must ensure that we don't depend on anything but kernel32.dll. That it works in CI now is nice, but having both getenv() and strtod() usage (both CRT functions) in DllMain gives me nightmares, because it creates a latent instability only waiting to raise issues... and that would be again a change in the production code for a win in testing.

If we truly want to improve the test build cache usage, let's not parametrize the build-time parameter in the test; instead, supply the stack size as a CLI parameter in the example and set everything there at runtime. This eliminates multiple configs that miss the cache without destabilizing auto-init by introducing dependencies beyond kernel32.dll. Another option would be an envreader/parser that depends solely on kernel32.dll.

Allow overriding the compile-time SENTRY_HANDLER_STACK_SIZE default at
runtime via environment variable. The stack-overflow integration tests
previously parametrized SENTRY_HANDLER_STACK_SIZE as a CMake option,
forcing 5 extra configure+build cycles on Windows (crashpad x2, inproc
x1, breakpad x2). Switching to a runtime env var lets every
parametrization reuse the cached build, saving a minute or two on
Windows.

The env var is read via `GetEnvironmentVariableA` (a kernel32 export)
rather than the CRT `getenv`/`strtod`, because the lookup happens in
`sentry__set_default_thread_stack_guarantee`, which is called from
`DllMain` on `DLL_PROCESS_ATTACH` / `DLL_THREAD_ATTACH`. CRT calls
under the loader lock are a latent source of instability and must be
avoided.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jpnurmi jpnurmi force-pushed the jpnurmi/ref/handler-stack-size branch from 0bfee15 to 4845d5a Compare April 11, 2026 07:24
@jpnurmi jpnurmi marked this pull request as ready for review April 11, 2026 08:37
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4845d5a. Configure here.

@jpnurmi jpnurmi merged commit 14bc452 into master Apr 13, 2026
59 checks passed
@jpnurmi jpnurmi deleted the jpnurmi/ref/handler-stack-size branch April 13, 2026 07:57
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.

3 participants