You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(profiling): keep fast-copy intent across warmup and fork - #19972
The sampler treated fast_copy_active as "we want safe_memcpy". Warmup clears that flag while our SIGSEGV handler is still installed, which caused four permanent / unsafe outcomes:
faulthandler.enable() during warmup took the handler unnoticed, and the warmup deadline pinned the process to the syscall copy for life.
A child forked mid-warmup inherited fast_copy_active == false and never upgraded (prefork workers).
After a foreign SIGSEGV takeover, the sticky fast_copy_foreign_takeover flag must survive stop → set_fast_copy(True) → restart so we never reclaim the handler.
After a foreign takeover with no process_vm_readv, a forked child must not keep sampling leftover safe_memcpy under the foreign handler.
Code Changes
Persistent fast_copy_desired (warmup does not clear it).
Sticky fast_copy_foreign_takeover across fork and across stop / set_fast_copy / restart.
Drop leftover memcpy sampling when takeover leaves no syscall fallback.
Publish desired + foreign-takeover in internal metadata.
Testing
Against origin/main's native tree these fail; with the fix they pass in test_copy_memory_stats.py / test_faulthandler.py:
test_fast_copy_faulthandler_enable_during_warmup
test_fast_copy_fork_during_warmup
test_fast_copy_foreign_handler_takeover_metadata (sticky across stop / set_fast_copy / restart)
test_fast_copy_fork_after_takeover_without_syscall (no leftover memcpy in the child)
Pre-review checklist
The changes have been tested in real services in staging
Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.
Your PR has warnings. Please review the issues below.
❄️ 2 New flaky tests detected
test_fast_copy_reenable_clears_failed_safe_copy[py3.10] from test_copy_memory_stats.py
Expected status 0, got 1.
=== Captured STDOUT ===
b''=== End of captured STDOUT ===
=== Captured STDERR ===
b'No safe memory copy method available (safe_memcpy and process_vm_readv both failed); disabling stack profiling\nddtrace stack profiler: no longer owns SIGSEGV and SIGBUS; falling back to syscall-based memory copy to avoid crashing. Handler owners: SIGSEGV=SIG_DFL, SIGBUS=SIG_DFL\nNo safe memory copy method available (safe_memcpy and process_vm_readv both failed); disabling stack profiling\nddtrace stack profiler: no safe memory-copy fallback available; stopping stack sampling to avoid crashing. Handler owners: SIGSEGV=SIG_DFL, SIGBUS=SIG_DFL\nTraceback (most recent call last):\n File "tests/profiling/collector/test_copy_memory_stats.py", line 542, in <module>\n assert wait_for_fast_copy_state(_stack, True, timeout=20.0), "sampler failed to restart after re-enable"\nAssertionError: sampler failed to restart after re-enable\n'=== End of captured STDERR ===
test_fast_copy_reenable_clears_failed_safe_copy[py3.11] from test_copy_memory_stats.py
Expected status 0, got 1.
=== Captured STDOUT ===
b''=== End of captured STDOUT ===
=== Captured STDERR ===
b'No safe memory copy method available (safe_memcpy and process_vm_readv both failed); disabling stack profiling\nddtrace stack profiler: no longer owns SIGSEGV and SIGBUS; falling back to syscall-based memory copy to avoid crashing. Handler owners: SIGSEGV=SIG_DFL, SIGBUS=SIG_DFL\nNo safe memory copy method available (safe_memcpy and process_vm_readv both failed); disabling stack profiling\nddtrace stack profiler: no safe memory-copy fallback available; stopping stack sampling to avoid crashing. Handler owners: SIGSEGV=SIG_DFL, SIGBUS=SIG_DFL\nTraceback (most recent call last):\n File "tests/profiling/collector/test_copy_memory_stats.py", line 542, in <module>\n assert wait_for_fast_copy_state(_stack, True, timeout=20.0), "sampler failed to restart after re-enable"\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nAssertionError: sampler failed to restart after re-enable\n'=== End of captured STDERR ===
vlad-scherbich
changed the title
fix(profiling): keep fast-copy intent across warmup and fork (PROF-15…
fix(profiling): keep fast-copy intent across warmup and fork (PROF-15342)
Sep 1, 2026
Same as above: when falling back after a mid-run handler takeover, update the process-static ProfilerStats metadata after flipping fast_copy_foreign_takeover / syscall fallback. Otherwise profiles can continue to report the initial fast-copy metadata even after the permanent fallback.
Found 1 performance improvements and 0 performance regressions! Performance is the same for 586 metrics, 10 unstable metrics, 24 flaky benchmarks without significant changes.
Explanation
This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:
🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline
We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.
If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.
Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.
More details about the CI and significant changes
You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.
CIs of the difference of means are often centered around 0%, because often changes are not that big:
---------------------------------(------|---^--------)-------------------------------->
-0.6% 0% 0.3% +1.2%
| | |
lower bound of the CI --' | |
sample mean (center of the CI) -------------' |
upper bound of the CI ----------------------'
As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).
For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:
----------------------------------------|---------|---(---------^---------)---------->
0% 1% 1.3% 2.2% 3.1%
| | | |
significant impact threshold --------------' | | |
lower bound of CI --------------' | |
sample mean (center of the CI) --------------------------' |
upper bound of CI ----------------------------------'
scenario:iastaspects-upper_aspect
🟩 execution_time [-22.458µs; -18.770µs] or [-8.645%; -7.226%]
Unstable benchmarks
These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.
vlad-scherbich
changed the title
fix(profiling): keep fast-copy intent across warmup and fork (PROF-15342)
fix(profiling): keep fast-copy intent across warmup and fork
Sep 29, 2026
stack_uninstall_segv_handler is also called by crashtracking.start() without pausing the sampler (ddtrace/internal/core/crashtracking.py:277-283), not only by the faulthandler wrapper. When a foreign handler is already installed, this new branch calls set_fast_copy_enabled(false) and may call Sampler::stop() while the sampling thread is in safe_memcpy/reading fast_copy_active, creating a data race and an unsafe handler/copy transition. Move this fallback/stop to the sampling thread, or make the crashtracker path coordinate a pause before entering this branch.
Post-warmup foreign takeover without process_vm_readv snapshotted
enabled before disable and broke before the end-of-cycle update.
Seed before launching the sampling thread to avoid racing the
non-atomic strategy flags, and reseed when disable/start fails.
uninstall_segv_handler can disable fast copy or stop the sampler on the
sticky-foreign path; crashtracking.start must pause first, matching the
faulthandler wrapper, so that does not race the sampling thread.
On pause timeout, skip faulthandler/crashtracker install so an in-flight
safe_memcpy is not left without its recovery handler. After a failed
disable with no process_vm_readv, uninstall leftover recovery handlers
so later coordinated installs do not sit above us.
Avoid enabling handlers while safe-copy sampling remains active
ddtrace/profiling/_faulthandler.py:48
When pause_sampling() returns None, the sampler is still running and its contract explicitly says the caller must not swap signal handlers. However, this path still calls _original_enable below, replacing SIGSEGV/SIGBUS while an in-flight safe_memcpy may be executing; a fault before the sampling loop notices the takeover will reach faulthandler instead of the recovery handler and can terminate the process. Defer the enable (or otherwise stop/disable safe-copy sampling) until the handler swap is safe; skipping only uninstall/reinstall is not sufficient.
Failed disable without process_vm_readv uninstalled the handler, but
set_fast_copy(True) only restored the copy function. Clear the user-
disabled latch on re-enable, gate crashtracker coordination on
stack.is_available, and assert the handler is back in the regression
test.
…sable
A successful set_fast_copy(False) left the SIGSEGV/SIGBUS recovery handler
installed while handler ops were off, so faulthandler/crashtracker could
chain to the dormant handler and set_fast_copy(True) reclaim formed a cycle.
Avoid overwriting a foreign SIGSEGV/SIGBUS owner on set_fast_copy(True),
fall back when init_segv_catcher fails after arming safe_memcpy, and
reinstall the stack recovery handler in crashtracker start finally
before resuming sampling.
The reason will be displayed to describe this comment to others. Learn more.
During warmup, faulthandler.disable() can reclaim a foreign signal handler without detecting takeover, allowing fast-copy activation and potentially creating a handler cycle that bypasses crash reporting.
The reason will be displayed to describe this comment to others. Learn more.
Check foreign ownership before reinstalling through disable
During warmup, if another library takes over a recovery signal and the application calls faulthandler.disable() while faulthandler is already disabled, _patched_disable() calls reinstall_segv_handler() without checking ownership. The new desired-intent gate permits this reinstall despite fast copy being inactive. It overwrites the foreign handler without marking takeover, so warmup upgrades to safe_memcpy instead of retaining syscall copy. A foreign handler that saved the profiler handler can also form a cycle that bypasses crash reporting. Detect takeover before disable restores dispositions, while preserving legitimate coordinated restores; the fix requires coordinating the Python wrapper and native handling.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
👍 Useful: correct, and you're fixing it or will.
👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
😕 Unclear: you can't tell what it means or why it matters.
👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
This PR is getting out of hand, with new AI comments appearing after each past one has been addressed. Moving into Draft status, will try to break it into several components, as the LoC size has ballooned with all the suggestions to fix.
Split into a draft stack of 4 PRs (one Summary bullet each) to cut review thrash / LoC size. Leaving this PR open as the old tip; tip SHA preserved as refs/preserved/pr-19972-tip-20261023 (6c7a5d0f1d).
PR4 tree matches the preserved tip. Prefer reviewing the stack; we can close this PR once you migrate.
If you instead want this PR retitled as PR1 with 2–4 stacked on top, say so and we can reshuffle.
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
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.
vlad-scherbich
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
taegyunkim
left a comment
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
Description
The sampler treated
fast_copy_activeas "we want safe_memcpy". Warmup clears that flag while our SIGSEGV handler is still installed, which caused four permanent / unsafe outcomes:faulthandler.enable()during warmup took the handler unnoticed, and the warmup deadline pinned the process to the syscall copy for life.fast_copy_active == falseand never upgraded (prefork workers).fast_copy_foreign_takeoverflag must survive stop →set_fast_copy(True)→ restart so we never reclaim the handler.process_vm_readv, a forked child must not keep sampling leftoversafe_memcpyunder the foreign handler.Code Changes
fast_copy_desired(warmup does not clear it).fast_copy_foreign_takeoveracross fork and across stop /set_fast_copy/ restart.Testing
Against
origin/main's native tree these fail; with the fix they pass intest_copy_memory_stats.py/test_faulthandler.py:test_fast_copy_faulthandler_enable_during_warmuptest_fast_copy_fork_during_warmuptest_fast_copy_foreign_handler_takeover_metadata(sticky across stop /set_fast_copy/ restart)test_fast_copy_fork_after_takeover_without_syscall(no leftover memcpy in the child)Pre-review checklist
https://datadoghq.atlassian.net/browse/PROF-16020