fix(profiling): avoid crashing stack sampler with active foreign SIGSEGV handlers (PROF-15342) by vlad-scherbich · Pull Request #18798 · DataDog/dd-trace-py

vlad-scherbich · 2026-06-29T21:03:10Z

Description

safe_memcpy's fault recovery only works while the profiler owns the SIGSEGV/SIGBUS
handlers. Libraries like PyTorch/CUDA (and abseil via vLLM/gRPC) install their own
handler during startup; once a foreign handler owns those signals, a fault on a stale
read is no longer recovered and the process crashes (PROF-14568).

This PR makes the default-on state safe and removes the need for the _DD_PROFILING_STACK_FAST_COPY=0
workaround. It handles the handlers we cannot wrap (torch/CUDA/abseil) via detect-and-fallback.

Changes

Sampler starts on the safe syscall copy (process_vm_readv / mach_vm_read_overwrite)
for a short warmup, so a fault during crash-prone startup can't crash the process (these
syscalls return an error instead of faulting).
After warmup it upgrades to safe_memcpy only if we still own both SIGSEGV and
SIGBUS (segv_handler_installed()).
It re-checks ownership every cycle and permanently falls back to the syscall copy if
a handler is taken over later (e.g. lazy CUDA init). If no safe fallback exists
(process_vm_readv blocked), it stops sampling — we degrade to dropped samples,
never a crash.
Warmup is a fixed 15s internal constant (not a user knob). Policy is auto-fallback, not
reinstall-and-chain, so the foreign handler stays authoritative; init_segv_catcher
stays call_once to avoid reintroducing handler-chaining races.

Test plan

New unit tests
Manual repro (synthetic + --torch) on Vlad/prof 14568 repro script #18911):
crashes on main, runs OK on this branch

User Verification:

https://datadoghq.atlassian.net/browse/AIPTS-1715

User deployed a binary with this and next PR (#18797) to their service; no crashes observed in 24 hours. (cc @askardog )

cit-pr-commenter-54b7da · 2026-06-29T21:05:38Z

Codeowners resolved as

ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_state.hpp  @DataDog/profiling-python
ddtrace/internal/datadog/profiling/dd_wrapper/src/profiler_stats.cpp    @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/include/sampler.hpp            @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/sampler.cpp                @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/stack.cpp                  @DataDog/profiling-python

datadog-prod-us1-5 · 2026-06-29T21:07:10Z

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 68d3838 | Docs | Datadog PR Page | Give us feedback!}

vlad-scherbich · 2026-07-02T00:34:12Z

@codex review plz

Copilot

Pull request overview

Hardens the stack v2 native sampler against crashes when another library takes over SIGSEGV (e.g., torch/CUDA, abseil), by preferring a syscall-based copy during startup and only using safe_memcpy when the profiler still owns the fault handlers.

Changes:

Add a configurable warmup window to start with syscall-based memory copying and then conditionally upgrade to safe_memcpy.
Re-check signal-handler ownership during sampling and permanently fall back to syscall copy if a foreign handler takes over.
Add a regression test and a standalone repro script for the foreign-handler scenario; include a release note.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/profiling/test_main.py	Adds subprocess regression test ensuring sampler degrades under a foreign `SIGSEGV` handler.
scripts/profiling/repro_prof_14568_foreign_handler.py	Adds a manual repro script to validate behavior with synthetic or torch-installed handlers.
releasenotes/notes/profiling-stack-sampler-foreign-handler-fallback-3d8f1b6a0e25c947.yaml	Documents the crash hardening and new warmup behavior.
ddtrace/internal/datadog/profiling/stack/src/sampler.cpp	Implements warmup + runtime ownership checks and fallback behavior.
ddtrace/internal/datadog/profiling/stack/src/echion/danger.cc	Adds handler-ownership probe used by the sampler.
ddtrace/internal/datadog/profiling/stack/echion/echion/danger.h	Declares the new handler-ownership probe API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 127a251509

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

cit-pr-commenter-54b7da · 2026-07-06T20:28:55Z

Circular import analysis

⚠️ Existing circular imports

There are 2524 circular imports that already exist on the base branch and have not been changed by this PR.

Show existing cycles (showing 5 of 2524 shortest)

ddtrace.internal.datastreams -> ddtrace.internal.datastreams.botocore -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.aiokafka -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.google_cloud_pubsub -> ddtrace.internal.datastreams
ddtrace.internal.core -> ddtrace._trace.span -> ddtrace.internal.core
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.kafka -> ddtrace.internal.datastreams

To see all cycles, download the cycles-base.json and cycles-pr.json artifacts from this CI job and run:
uv run --script scripts/import-analysis/cycles.py compare cycles-base.json cycles-pr.json

Address review feedback on PR #18798: - sampling_thread only (re)arms our SIGSEGV/SIGBUS handlers via init_segv_catcher when we still own them. If a foreign component (abseil via vLLM/gRPC, torch/CUDA) took over the dispositions before the sampling thread started, we no longer overwrite it - which had made segv_handler_installed() falsely report ownership and defeated the warmup/fallback. We now leave the foreign handler authoritative and stay on the safe syscall copy. - Reduce the fast-copy stat test deadline from 45s to 30s for faster CI feedback (warmup window is 15s).

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Address review feedback on PR #18798: - sampling_thread only (re)arms our SIGSEGV/SIGBUS handlers via init_segv_catcher when we still own them. If a foreign component (abseil via vLLM/gRPC, torch/CUDA) took over the dispositions before the sampling thread started, we no longer overwrite it - which had made segv_handler_installed() falsely report ownership and defeated the warmup/fallback. We now leave the foreign handler authoritative and stay on the safe syscall copy. - Reduce the fast-copy stat test deadline from 45s to 30s for faster CI feedback (warmup window is 15s).

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

The native stack sampler's safe_memcpy relies on owning the SIGSEGV handler for its fault recovery. Components such as PyTorch/CUDA and abseil (pulled in by vLLM/gRPC) install their own SIGSEGV handler during the import-heavy startup, after which a fault in safe_memcpy no longer recovers and instead crashes the process (PROF-14568). Start the sampler on the safe syscall-based copy for a warmup window, then upgrade to safe_memcpy only if we still own the handler, and keep checking ownership every cycle so we fall back permanently if a handler is installed later (e.g. lazy CUDA init on first GPU use). The warmup window is configurable via DD_PROFILING_STACK_FAST_COPY_WARMUP_S.

…ospection The stack fast-copy warmup window was overridable via a raw DD_PROFILING_STACK_FAST_COPY_WARMUP_S getenv that existed only so the regression test could skip the warmup. It was never a registered config and users never need to tune it, so hardcode the 15s warmup as a constant. To keep the foreign-handler behavior testable without that knob, expose segv_handler_installed() on the stack module (mirroring is_safe_copy_failed and reinstall_segv_handler) and assert the ownership predicate directly: our handler is installed at import, flips to not-owned when a foreign handler is installed, and is reclaimed by reinstall_segv_handler(). This also removes the flaky stderr-warning assertion from the previous test.

test_fast_copy_memory_enabled asserted fast_copy_memory_enabled=True within a 3s run, but the foreign-handler fix (PROF-14568) now starts the sampler on the safe syscall-based copy for a startup warmup window and only upgrades to safe_memcpy afterwards. During warmup the metadata correctly reports False, so the short run failed. Update the test to reflect the warmup-then-upgrade behavior: poll the emitted metadata until the sampler upgrades (fast_copy_memory_enabled=True), asserting it ran on the syscall copy during warmup first. Breaks as soon as the upgrade is observed to keep runtime bounded.

…n no safe fallback Address review feedback on the foreign-handler fix: - segv_handler_installed() now requires our SA_SIGINFO handler to own BOTH SIGSEGV and SIGBUS (init_segv_catcher installs both). Owning only one still lets safe_memcpy fault into a foreign handler and crash. - When the per-cycle fallback cannot install a safe copy method (e.g. process_vm_readv blocked on the host), stop stack sampling instead of leaving the unsafe safe_memcpy path active under a foreign handler. - Update comments, logs, docstrings and the repro env var to _DD_PROFILING_STACK_FAST_COPY, and extend the detection test to cover SIGBUS.

- Note the intentional call_once on init_segv_catcher (auto-fallback policy, not reinstall-and-chain) so the handler-chaining races from PROF-14568 are not reintroduced. - Expand the release note to mention SIGBUS, the stop-sampling fallback, and the complementary faulthandler integration.

Address review feedback on PR #18798: - sampling_thread only (re)arms our SIGSEGV/SIGBUS handlers via init_segv_catcher when we still own them. If a foreign component (abseil via vLLM/gRPC, torch/CUDA) took over the dispositions before the sampling thread started, we no longer overwrite it - which had made segv_handler_installed() falsely report ownership and defeated the warmup/fallback. We now leave the foreign handler authoritative and stay on the safe syscall copy. - Reduce the fast-copy stat test deadline from 45s to 30s for faster CI feedback (warmup window is 15s).

The foreign-handler repro is kept for reference on the stacked branch vlad/prof-14568-repro-script; it is not part of the shipped fix.

Fixes the prechecks style step, which pins clang-format 18.1.5 and splits `struct X{}` aggregate initializers onto their own line. The local pre-commit hook used a different clang-format version (Apple 17), so the skew slipped through locally.

Mirror the segv_handler_installed declaration into the native _stack extension stub so both stubs agree (addresses Copilot review on 18798).

Replace the metadata-file scraping in test_fast_copy_memory_enabled with an in-process check and a fast, deterministic warmup: - Add fast_copy_memory_active() native getter so tests can read the live copy mode directly instead of polling *.internal_metadata.json (which raced with mid-write files and depended on the upload cadence). Mirrors the existing segv_handler_installed()/is_safe_copy_failed() introspection. - Add a test-only _set_fast_copy_warmup_seconds() knob (underscore-prefixed, accessed via the _stack submodule) so the warmup->safe_memcpy upgrade can be observed in ~1s instead of waiting out the 15s production default. The default warmup is now a Sampler member (15s) rather than a file-local constant. The test now shrinks the warmup, confirms the sampler runs on the syscall copy during warmup, then confirms it upgrades to safe_memcpy, all via the getter.

…check The clang-tidy profiling job fails on clang-analyzer-optin.performance.Padding (warnings-as-errors): class Datadog::Sampler had 35 padding bytes where 3 is optimal after adding fast_copy_warmup_seconds. Reorder the data members into the alignment-descending order clang-tidy reports as optimal (pointers/atomics/8-byte scalars, then vectors, mutexes, condvars, then 4-byte and bool fields). Layout only; no behavior change (the constructor initializes only echion, which stays first, so no -Wreorder).

… unavailable Address review feedback on PR #18798: - _set_fast_copy_warmup_seconds now rejects non-finite (NaN/inf) and negative durations with a ValueError, instead of letting a nonsensical warmup deadline through this Python-exposed entry point. - test_stack_profiler_foreign_segv_handler_detection now skips when the native stack extension is unavailable (stack.is_available is False), avoiding an AttributeError on builds/platforms without it.

…ervability

Match #18798 member order so clang-tidy padding passes on the 4.12 backport. Track fast_copy_requested separately from fast_copy_active so warmup toggles do not confuse the sampler upgrade path, and make the warmup metadata test tolerate platforms without process_vm_readv.

ProfilerStats is swapped on each upload; the final stop() upload often only has heap-tracker counters and omitted fast_copy_memory_* fields. Keep a process-static snapshot on ProfilerState, seed it from the sampler, and always serialize the fast-copy keys with stat/snapshot/default fallbacks so test_copy_memory_error_count_present sees them in every file.

vlad-scherbich mentioned this pull request Jun 29, 2026

fix(profiling): stack-profiler SIGSEGV on shutdown (PROF-15342) #18797

Draft

4 tasks

vlad-scherbich changed the title ~~profiling: harden stack sampler against foreign SIGSEGV handlers (PROF-14568) [2/2]~~ fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-14568) [2/2] Jun 29, 2026

vlad-scherbich force-pushed the vlad/prof-14568-handler-ownership branch from d6649ac to 127a251 Compare July 1, 2026 20:02

vlad-scherbich changed the base branch from vlad/prof-14568-shutdown-race to main July 1, 2026 20:02

vlad-scherbich changed the title ~~fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-14568) [2/2]~~ fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-14568) Jul 1, 2026

vlad-scherbich requested a review from Copilot July 2, 2026 00:34

Copilot started reviewing on behalf of vlad-scherbich July 2, 2026 00:34 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread ddtrace/internal/datadog/profiling/stack/src/sampler.cpp Outdated

vlad-scherbich requested a review from Copilot July 6, 2026 18:44

Copilot started reviewing on behalf of vlad-scherbich July 6, 2026 18:45 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread ddtrace/internal/datadog/profiling/stack/src/sampler.cpp Outdated

Comment thread tests/profiling/collector/test_copy_memory_stats.py Outdated

vlad-scherbich force-pushed the vlad/prof-14568-handler-ownership branch from 4f3001a to 6a7790b Compare July 6, 2026 20:27

vlad-scherbich requested a review from Copilot July 7, 2026 14:04

Copilot started reviewing on behalf of vlad-scherbich July 7, 2026 14:04 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread ddtrace/internal/datadog/profiling/stack/__init__.pyi

vlad-scherbich force-pushed the vlad/prof-14568-handler-ownership branch 2 times, most recently from 6506c75 to 4f65361 Compare July 7, 2026 17:12

vlad-scherbich changed the title ~~fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-14568)~~ fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-15342) Jul 7, 2026

vlad-scherbich requested a review from Copilot July 7, 2026 19:31

Copilot started reviewing on behalf of vlad-scherbich July 7, 2026 19:32 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread ddtrace/internal/datadog/profiling/stack/src/stack.cpp

Comment thread tests/profiling/test_main.py

vlad-scherbich changed the title ~~fix(profiling): harden stack sampler vs foreign SIGSEGV handlers (PROF-15342)~~ fix(profiling): avoid crashing stack sampler when foreign SIGSEGV handlers are activated (PROF-15342) Jul 7, 2026

vlad-scherbich requested a review from a team as a code owner July 14, 2026 22:03

vlad-scherbich requested review from avara1986 and emmettbutler July 14, 2026 22:03

vlad-scherbich and others added 26 commits July 14, 2026 18:04

fix lints

944d990

chore(profiling): drop PROF-14568 repro script from PR

44a38ff

The foreign-handler repro is kept for reference on the stacked branch vlad/prof-14568-repro-script; it is not part of the shipped fix.

stubs(profiling): add segv_handler_installed to _stack.pyi

6f01f6a

Mirror the segv_handler_installed declaration into the native _stack extension stub so both stubs agree (addresses Copilot review on 18798).

shorten release note

adfb3b0

shorten comments and improve tests

b90edce

Thomas and bot comments addressed

0af7dc7

review: apply suggestion

ed778fc

review: apply suggestion

3eb3c39

review: apply suggestion

ca76510

review: apply suggestion

196ae62

review: apply suggestion

ffcd2a0

review: apply suggestion

34a930e

review: apply suggestion

e1bfe6d

feat(profiling): add ProfilerStats metrics for fast-copy fallback obs…

b527587

…ervability

vlad-scherbich force-pushed the vlad/prof-14568-handler-ownership branch from e1cee95 to 68d3838 Compare July 14, 2026 22:05

Uh oh!

Conversation

vlad-scherbich commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Test plan

User Verification:

Uh oh!

cit-pr-commenter-54b7da Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codeowners resolved as

Uh oh!

datadog-prod-us1-5 Bot commented Jun 29, 2026 • edited by datadog-prod-us1-6 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vlad-scherbich commented Jul 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

cit-pr-commenter-54b7da Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Circular import analysis

⚠️ Existing circular imports

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vlad-scherbich commented Jun 29, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Jun 29, 2026 •

edited

Loading

datadog-prod-us1-5 Bot commented Jun 29, 2026 •

edited by datadog-prod-us1-6 Bot

Loading

cit-pr-commenter-54b7da Bot commented Jul 6, 2026 •

edited

Loading