diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_stats.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_stats.hpp index 49d91779a65..3802b83d1f1 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_stats.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_stats.hpp @@ -31,6 +31,15 @@ class ProfilerStats // Whether fast_copy_memory (safe_memcpy) is enabled; unset until the sampler starts std::optional fast_copy_memory_enabled; + // User opted out of fast copy (env var or set_fast_copy(false)); static per process + std::optional fast_copy_memory_user_disabled; + + // Whether safe_memcpy initialized at startup; static per process + std::optional fast_copy_memory_capable; + + // Sticky: fell back to syscall copy (init failure, foreign handler, etc.) + std::optional fast_copy_memory_syscall_fallback; + // Number of copy_memory errors accumulated since the last profile reset (i.e. since the last upload) size_t copy_memory_error_count = 0; @@ -69,6 +78,18 @@ class ProfilerStats void set_fast_copy_memory_enabled(bool enabled); std::optional get_fast_copy_memory_enabled() const; + void set_fast_copy_memory_user_disabled(bool disabled); + std::optional get_fast_copy_memory_user_disabled() const; + + void set_fast_copy_memory_capable(bool capable); + std::optional get_fast_copy_memory_capable() const; + + void set_fast_copy_memory_syscall_fallback(bool fallback); + std::optional get_fast_copy_memory_syscall_fallback() const; + + // fast_copy_memory_* are process-static; carry them across ProfilerStats swaps. + void copy_fast_copy_metadata_from(const ProfilerStats& other); + void add_copy_memory_error_count(size_t count); size_t get_copy_memory_error_count() const; diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profiler_stats.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profiler_stats.cpp index 7352673a13f..f0c74973cf0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profiler_stats.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profiler_stats.cpp @@ -12,6 +12,19 @@ append_to_string(std::string& s, size_t value) s.append(buf, ptr); } +void +append_optional_bool(std::string& s, const char* key, const std::optional& value) +{ + if (!value.has_value()) { + return; + } + s += '"'; + s += key; + s += "\": "; + s += *value ? "true" : "false"; + s += ','; +} + } // namespace void @@ -51,7 +64,7 @@ Datadog::ProfilerStats::reset_state() asyncio_task_count = std::nullopt; greenlet_count = std::nullopt; sample_capture_cpu_time_us = 0; - // fast_copy_memory_enabled is intentionally not reset: it reflects a static configuration + // fast_copy_memory_* static fields are intentionally not reset (see setters). } void @@ -66,6 +79,59 @@ Datadog::ProfilerStats::get_fast_copy_memory_enabled() const return fast_copy_memory_enabled; } +void +Datadog::ProfilerStats::set_fast_copy_memory_user_disabled(bool disabled) +{ + fast_copy_memory_user_disabled = disabled; +} + +std::optional +Datadog::ProfilerStats::get_fast_copy_memory_user_disabled() const +{ + return fast_copy_memory_user_disabled; +} + +void +Datadog::ProfilerStats::set_fast_copy_memory_capable(bool capable) +{ + fast_copy_memory_capable = capable; +} + +std::optional +Datadog::ProfilerStats::get_fast_copy_memory_capable() const +{ + return fast_copy_memory_capable; +} + +void +Datadog::ProfilerStats::set_fast_copy_memory_syscall_fallback(bool fallback) +{ + fast_copy_memory_syscall_fallback = fallback; +} + +std::optional +Datadog::ProfilerStats::get_fast_copy_memory_syscall_fallback() const +{ + return fast_copy_memory_syscall_fallback; +} + +void +Datadog::ProfilerStats::copy_fast_copy_metadata_from(const ProfilerStats& other) +{ + if (auto value = other.get_fast_copy_memory_user_disabled()) { + set_fast_copy_memory_user_disabled(*value); + } + if (auto value = other.get_fast_copy_memory_capable()) { + set_fast_copy_memory_capable(*value); + } + if (auto value = other.get_fast_copy_memory_syscall_fallback()) { + set_fast_copy_memory_syscall_fallback(*value); + } + if (auto value = other.get_fast_copy_memory_enabled()) { + set_fast_copy_memory_enabled(*value); + } +} + void Datadog::ProfilerStats::add_copy_memory_error_count(size_t count) { @@ -203,6 +269,11 @@ Datadog::ProfilerStats::get_internal_metadata_json() internal_metadata_json += ","; } + append_optional_bool(internal_metadata_json, "fast_copy_memory_user_disabled", fast_copy_memory_user_disabled); + append_optional_bool(internal_metadata_json, "fast_copy_memory_capable", fast_copy_memory_capable); + append_optional_bool( + internal_metadata_json, "fast_copy_memory_syscall_fallback", fast_copy_memory_syscall_fallback); + auto maybe_heap_tracker_count = get_heap_tracker_size(); if (maybe_heap_tracker_count) { internal_metadata_json += R"("heap_tracker_count": )"; diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp index 24d5f3cce3f..e83cc5141a6 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp @@ -205,6 +205,7 @@ Datadog::UploaderBuilder::build() // Swap the ProfilerStats (which replaces the one being written to with an empty state). // We do this first as we still want to reset ProfilerStats if the serialization fails. std::swap(stats, borrowed.stats()); + borrowed.stats().copy_fast_copy_metadata_from(stats); // Try to encode the Profile (which will also reset it) encoded = ddog_prof_Profile_serialize(&borrowed.profile(), nullptr, nullptr); diff --git a/ddtrace/internal/datadog/profiling/stack/__init__.pyi b/ddtrace/internal/datadog/profiling/stack/__init__.pyi index dea8a30197b..b41e62760f2 100644 --- a/ddtrace/internal/datadog/profiling/stack/__init__.pyi +++ b/ddtrace/internal/datadog/profiling/stack/__init__.pyi @@ -42,6 +42,10 @@ def set_interval(new_interval: float) -> None: ... # Memory copy strategy def set_fast_copy(enabled: bool) -> None: ... def is_safe_copy_failed() -> bool: ... +def fast_copy_memory_active() -> bool: ... # test introspection: is safe_memcpy active? + +# _set_fast_copy_warmup_seconds is test-only; accessed via _stack (import * skips it). + def uninstall_segv_handler() -> None: ... def reinstall_segv_handler() -> None: """Reinstall SIGSEGV/SIGBUS handlers after another component overwrites them. @@ -52,6 +56,14 @@ def reinstall_segv_handler() -> None: """ ... +def segv_handler_installed() -> bool: + """Return True if our handler is the installed disposition for SIGSEGV and SIGBUS. + + Primarily test introspection: it queries the live disposition via sigaction(2) + for both signals on every call, so it is not free. Do not call it on hot paths. + """ + ... + # Pause/resume sampling def pause_sampling() -> bool | None: """Pause the sampling thread and wait for any in-flight sample to complete. diff --git a/ddtrace/internal/datadog/profiling/stack/_stack.pyi b/ddtrace/internal/datadog/profiling/stack/_stack.pyi index ee7fdee446e..50449e05bb1 100644 --- a/ddtrace/internal/datadog/profiling/stack/_stack.pyi +++ b/ddtrace/internal/datadog/profiling/stack/_stack.pyi @@ -23,6 +23,15 @@ def set_interval(new_interval: float) -> None: ... # Memory copy strategy def set_fast_copy(enabled: bool) -> None: ... def is_safe_copy_failed() -> bool: ... +def fast_copy_memory_active() -> bool: ... # test introspection: is safe_memcpy active? +def _set_fast_copy_warmup_seconds(seconds: float) -> None: ... # test-only; before start +def segv_handler_installed() -> bool: + """Return True if our handler is the installed disposition for SIGSEGV and SIGBUS. + + Primarily test introspection: it queries the live disposition via sigaction(2) + for both signals on every call, so it is costly. Do not call it on hot paths. + """ + ... # span <-> profile association def link_span( diff --git a/ddtrace/internal/datadog/profiling/stack/echion/echion/danger.h b/ddtrace/internal/datadog/profiling/stack/echion/echion/danger.h index 71de5f12e1b..17c49397b88 100644 --- a/ddtrace/internal/datadog/profiling/stack/echion/echion/danger.h +++ b/ddtrace/internal/datadog/profiling/stack/echion/echion/danger.h @@ -27,6 +27,10 @@ init_segv_catcher(); void uninstall_segv_handler(); +// Returns true only if our signal handler owns both SIGSEGV and SIGBUS; false on any error. +bool +segv_handler_installed(); + #if defined PL_LINUX ssize_t safe_memcpy_wrapper(pid_t, diff --git a/ddtrace/internal/datadog/profiling/stack/echion/echion/vm.h b/ddtrace/internal/datadog/profiling/stack/echion/echion/vm.h index ccb54bbf334..a615a6a467d 100644 --- a/ddtrace/internal/datadog/profiling/stack/echion/echion/vm.h +++ b/ddtrace/internal/datadog/profiling/stack/echion/echion/vm.h @@ -54,8 +54,19 @@ inline kern_return_t (*safe_copy)(vm_map_read_t, // Whether safe_copy is currently set to the memcpy-based wrapper. inline bool fast_copy_active = false; -// Whether init_segv_catcher succeeded at constructor time. Persists even if -// fast_copy_active is later toggled off by set_fast_copy_enabled. +// User opted out via _DD_PROFILING_STACK_FAST_COPY or set_fast_copy(false). +inline bool fast_copy_user_disabled = false; + +// Sticky: fell back to syscall copy (init failure, foreign handler, warmup miss). +inline bool fast_copy_syscall_fallback = false; + +inline void +mark_fast_copy_syscall_fallback() +{ + fast_copy_syscall_fallback = true; +} + +// Set at init; survives toggling fast_copy_active. inline bool safe_memcpy_initialized = false; #if defined PL_LINUX diff --git a/ddtrace/internal/datadog/profiling/stack/include/sampler.hpp b/ddtrace/internal/datadog/profiling/stack/include/sampler.hpp index 0ef9ecb0489..9d3b4490c07 100644 --- a/ddtrace/internal/datadog/profiling/stack/include/sampler.hpp +++ b/ddtrace/internal/datadog/profiling/stack/include/sampler.hpp @@ -45,9 +45,6 @@ class Sampler // The mutex + condition variable pair is used to avoid the "lost wake-up" race condition // where stop() could miss the notification and hang forever (or until timeout). std::atomic thread_running{ false }; - std::mutex thread_exit_mutex; - std::condition_variable thread_exit_cv; - // Whether the sampler is currently active. Unlike thread_running (which tracks // the thread's actual lifecycle) this is set synchronously in start/stop, so // it is not subject to the sampling-thread startup race. @@ -56,6 +53,8 @@ class Sampler // prefork reads this to decide whether to restart the sampler after fork, // ensuring a sampler that stopped due to an error is not silently restarted. std::atomic sampler_active_{ false }; + std::mutex thread_exit_mutex; + std::condition_variable thread_exit_cv; // Pause synchronization — allows the faulthandler wrapper to temporarily // suspend sampling so signal handlers can be safely swapped without racing @@ -101,6 +100,9 @@ class Sampler // Percentile (0..1) used for p_stable; configurable, default p95. double p_stable_percentile_frac = 0.95; + // Fast-copy startup warmup in seconds. + double fast_copy_warmup_seconds = 15.0; + // Rolling window duration in seconds; controls the ring buffer capacity. uint32_t p_stable_window_s = 600; @@ -164,6 +166,8 @@ class Sampler // Set the percentile (0–100) used to compute p_stable from the rolling window. void set_p_stable_percentile(double percentile) { p_stable_percentile_frac = percentile / 100.0; } + void set_fast_copy_warmup_seconds(double value) { fast_copy_warmup_seconds = value; } + // Delegates to the StackRenderer to clear its caches after fork void postfork_child(); @@ -178,4 +182,7 @@ class Sampler bool restart_after_fork(); }; +void +seed_fast_copy_profiler_stats(); + } // namespace Datadog diff --git a/ddtrace/internal/datadog/profiling/stack/src/echion/danger.cc b/ddtrace/internal/datadog/profiling/stack/src/echion/danger.cc index 4fe7e1a57b3..be1b3ee500a 100644 --- a/ddtrace/internal/datadog/profiling/stack/src/echion/danger.cc +++ b/ddtrace/internal/datadog/profiling/stack/src/echion/danger.cc @@ -140,6 +140,24 @@ init_segv_catcher() return 0; } +bool +segv_handler_installed() +{ + // Recovery needs our handler to own BOTH SIGSEGV and SIGBUS + // (a copy fault can arrive as either); anything else means we can't recover. + const int signals[] = { SIGSEGV, SIGBUS }; + for (int signo : signals) { + struct sigaction current; + if (sigaction(signo, nullptr, ¤t) != 0) { + return false; + } + if (current.sa_sigaction != segv_handler || (current.sa_flags & SA_SIGINFO) == 0) { + return false; + } + } + return true; +} + void uninstall_segv_handler() { diff --git a/ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc b/ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc index ef21bc207a1..a776e7b191c 100644 --- a/ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc +++ b/ddtrace/internal/datadog/profiling/stack/src/echion/vm.cc @@ -41,6 +41,7 @@ init_safe_copy() // Honor the fast-copy opt-out: when disabled via env var, skip installing // the SIGSEGV/SIGBUS handlers and alt stack entirely. if (fast_copy_env_disabled()) { + fast_copy_user_disabled = true; if (process_vm_readv_available) { safe_copy = process_vm_readv; } else { @@ -60,6 +61,7 @@ init_safe_copy() fprintf(stderr, "Failed to initialize segv catcher. Trying process_vm_readv.\n"); if (process_vm_readv_available) { safe_copy = process_vm_readv; + mark_fast_copy_syscall_fallback(); } else { fprintf(stderr, "Failed to initialize safe copy interface\n"); failed_safe_copy = true; @@ -72,6 +74,7 @@ init_safe_copy() { // Honor the fast-copy opt-out: skip installing signal handlers when disabled. if (fast_copy_env_disabled()) { + fast_copy_user_disabled = true; return; } @@ -84,6 +87,7 @@ init_safe_copy() // std::cerr might not be fully initialized at constructor time. fprintf(stderr, "Failed to initialize segv catcher. Using mach_vm_read_overwrite instead.\n"); + mark_fast_copy_syscall_fallback(); } #endif // PL_DARWIN @@ -99,6 +103,7 @@ set_fast_copy_enabled(bool enabled) } fprintf(stderr, "Warning: fast copy requested but safe_memcpy was not initialized; falling back to process_vm_readv\n"); + mark_fast_copy_syscall_fallback(); // Fall through to process_vm_readv. } diff --git a/ddtrace/internal/datadog/profiling/stack/src/sampler.cpp b/ddtrace/internal/datadog/profiling/stack/src/sampler.cpp index 90cbd5abcb2..c0f36cc2313 100644 --- a/ddtrace/internal/datadog/profiling/stack/src/sampler.cpp +++ b/ddtrace/internal/datadog/profiling/stack/src/sampler.cpp @@ -25,6 +25,21 @@ using namespace Datadog; +static void +update_fast_copy_stats(ProfilerStats& stats) +{ + stats.set_fast_copy_memory_user_disabled(fast_copy_user_disabled); + stats.set_fast_copy_memory_capable(safe_memcpy_initialized); + stats.set_fast_copy_memory_syscall_fallback(fast_copy_syscall_fallback); + stats.set_fast_copy_memory_enabled(fast_copy_active); +} + +void +Datadog::seed_fast_copy_profiler_stats() +{ + update_fast_copy_stats(Sample::profile_borrow().stats()); +} + // Helper class for spawning a std::thread with control over its default stack size #ifdef __linux__ #include @@ -341,20 +356,50 @@ Sampler::sampling_thread(const uint64_t seq_num) // Mark thread as running thread_running.store(true); - // Re-install SIGSEGV/SIGBUS handlers here, after Python initialization. - // The handlers may have been installed during static init, but Python or - // libraries (faulthandler, Django, FastAPI) can overwrite them afterwards. - // Re-installing here ensures our handler is active when the sampling thread runs. - // Only do this once to avoid overwriting g_old_segv with our own handler. + seed_fast_copy_profiler_stats(); + + // (Re)install our SIGSEGV/SIGBUS handlers once, but ONLY if we still own them. + // + // safe_memcpy recovers only when our handler owns BOTH signals (see danger.cc). + // We can chain on top of handlers we coordinate with (faulthandler, crashtracker: + // pause + uninstall/reinstall in stack.cpp / crashtracking.py). Libraries such as + // abseil (vLLM/gRPC) or PyTorch/CUDA install their own handlers independently—often + // lazily on other threads—so overwriting them breaks their crash path and faults + // during sampling may still reach their handler instead of our siglongjmp (PROF-14568). + // If a foreign owner is already authoritative, leave it in place and fall back to + // the syscall copy rather than reclaiming on top. static std::once_flag segv_handler_once; if (fast_copy_active) { - std::call_once(segv_handler_once, init_segv_catcher); + std::call_once(segv_handler_once, []() { + if (segv_handler_installed()) { + init_segv_catcher(); + } + }); } using namespace std::chrono; auto sample_time_prev = steady_clock::now(); auto interval_adjust_time_prev = sample_time_prev; + // safe_memcpy recovery needs us to own both handlers (PROF-14568): warm up on the + // syscall copy, upgrade only if we still own them, then re-check and fall back. + const bool fast_copy_desired = fast_copy_active; +#if defined PL_LINUX + const bool syscall_copy_available = process_vm_readv_available; +#else + const bool syscall_copy_available = true; // mach_vm_read_overwrite is always available +#endif + // Warm up only when fast copy is wanted and a safe fallback path exists to run on. + const bool fast_copy_warmup = fast_copy_desired && syscall_copy_available; + bool fast_copy_upgraded = !fast_copy_warmup; + bool handler_fallback_done = false; + const auto fast_copy_warmup_deadline = + sample_time_prev + duration_cast(duration(fast_copy_warmup_seconds)); + if (fast_copy_warmup) { + // Drop to the safe syscall copy for the startup window. + set_fast_copy_enabled(false); + } + while (seq_num == thread_seq_num.load()) { // Check if a pause has been requested (e.g., for signal handler swapping). // Block until resumed or the thread is asked to stop. @@ -379,6 +424,50 @@ Sampler::sampling_thread(const uint64_t seq_num) auto wall_time_us = duration_cast(sample_time_now - sample_time_prev).count(); sample_time_prev = sample_time_now; + // Foreign handler handling (see notes before the loop); faulthandler's + // transient swaps are safe since the sampler is paused around them. + if (fast_copy_desired) { + if (!fast_copy_upgraded) { + // Warmup window: still on the safe syscall copy. Once it elapses, + // upgrade to safe_memcpy only if we still own the handlers. + if (sample_time_now >= fast_copy_warmup_deadline) { + fast_copy_upgraded = true; // decide once + if (segv_handler_installed()) { + set_fast_copy_enabled(true); + } else { + // Another component already owns a handler; stay on the safe + // syscall copy (already active from warmup) for the life of + // the process. + handler_fallback_done = true; + mark_fast_copy_syscall_fallback(); + std::cerr << "ddtrace stack profiler: another component owns the SIGSEGV/SIGBUS " + "handler; keeping the syscall-based memory copy to avoid crashing." + << std::endl; + } + } + } else if (fast_copy_active && !handler_fallback_done && !segv_handler_installed()) { + // A handler was taken over after upgrading; fall back permanently + // (no debounce). This is not free: it pins the process to the slower + // syscall copy for its remaining lifetime, which can meaningfully + // degrade sample quality (e.g. on asyncio workloads). We still prefer + // it over the alternative, which is crashing under a foreign handler. + handler_fallback_done = true; + mark_fast_copy_syscall_fallback(); + std::cerr << "ddtrace stack profiler: SIGSEGV/SIGBUS handler was taken over by another " + "component; falling back to syscall-based memory copy to avoid crashing." + << std::endl; + if (!set_fast_copy_enabled(false)) { + // No safe fallback available (e.g. process_vm_readv blocked), so + // safe_memcpy is still active; reading under a foreign handler would + // crash - stop sampling instead. + std::cerr << "ddtrace stack profiler: no safe memory-copy fallback available; " + "stopping stack sampling to avoid crashing." + << std::endl; + break; + } + } + } + // Reset per-cycle asyncio task accumulator before iterating sampled threads echion->reset_asyncio_task_count(); @@ -417,7 +506,7 @@ Sampler::sampling_thread(const uint64_t seq_num) borrow.stats().increment_sampling_event_count(); borrow.stats().set_string_table_count(echion->string_table().size()); - borrow.stats().set_fast_copy_memory_enabled(fast_copy_active); + update_fast_copy_stats(borrow.stats()); borrow.stats().set_asyncio_task_count(echion->asyncio_task_count()); borrow.stats().set_greenlet_count(greenlet_count); diff --git a/ddtrace/internal/datadog/profiling/stack/src/stack.cpp b/ddtrace/internal/datadog/profiling/stack/src/stack.cpp index e87f2522463..b5777dace00 100644 --- a/ddtrace/internal/datadog/profiling/stack/src/stack.cpp +++ b/ddtrace/internal/datadog/profiling/stack/src/stack.cpp @@ -8,6 +8,7 @@ #include "echion/echion_sampler.h" #include "echion/vm.h" +#include #include #include @@ -32,6 +33,7 @@ stack_start_impl(PyObject* self, PyObject* args, PyObject* kwargs) Py_BEGIN_ALLOW_THREADS; OriginTaskLinks::get_instance().enable(); Py_END_ALLOW_THREADS; + seed_fast_copy_profiler_stats(); Py_RETURN_TRUE; } Py_RETURN_FALSE; @@ -850,7 +852,11 @@ stack_set_fast_copy(PyObject* Py_UNUSED(self), PyObject* args) return nullptr; } - set_fast_copy_enabled(static_cast(enabled)); + const bool want = static_cast(enabled); + if (!want) { + fast_copy_user_disabled = true; + } + set_fast_copy_enabled(want); Py_RETURN_NONE; } @@ -882,6 +888,15 @@ stack_reinstall_segv_handler(PyObject* Py_UNUSED(self), PyObject* Py_UNUSED(args Py_RETURN_NONE; } +static PyObject* +stack_segv_handler_installed(PyObject* Py_UNUSED(self), PyObject* Py_UNUSED(args)) +{ + if (segv_handler_installed()) { + Py_RETURN_TRUE; + } + Py_RETURN_FALSE; +} + static PyObject* stack_is_safe_copy_failed(PyObject* Py_UNUSED(self), PyObject* Py_UNUSED(args)) { @@ -894,6 +909,41 @@ stack_is_safe_copy_failed(PyObject* Py_UNUSED(self), PyObject* Py_UNUSED(args)) Py_RETURN_FALSE; } +static PyObject* +stack_fast_copy_memory_active(PyObject* Py_UNUSED(self), PyObject* Py_UNUSED(args)) +{ + if (fast_copy_active) { + Py_RETURN_TRUE; + } + Py_RETURN_FALSE; +} + +static PyObject* +stack_set_fast_copy_warmup_seconds(PyObject* Py_UNUSED(self), PyObject* args) +{ + double seconds_value = 0.0; + + if (!PyArg_ParseTuple(args, "d", &seconds_value)) { + return NULL; + } + + if (!std::isfinite(seconds_value) || seconds_value < 0.0) { + PyErr_SetString(PyExc_ValueError, + "_set_fast_copy_warmup_seconds requires a finite, non-negative number of seconds"); + return NULL; + } + + if (Sampler::get().is_running()) { + PyErr_SetString(PyExc_RuntimeError, + "_set_fast_copy_warmup_seconds must be called before the sampler is started"); + return NULL; + } + + Sampler::get().set_fast_copy_warmup_seconds(seconds_value); + + Py_RETURN_NONE; +} + static PyMethodDef stack_methods[] = { { "start", reinterpret_cast(stack_start), METH_VARARGS | METH_KEYWORDS, "Start the sampler" }, { "stop", stack_stop, METH_VARARGS, "Stop the sampler" }, @@ -956,6 +1006,14 @@ static PyMethodDef stack_methods[] = { stack_is_safe_copy_failed, METH_NOARGS, "Check if all safe copy methods failed to initialize" }, + { "fast_copy_memory_active", + stack_fast_copy_memory_active, + METH_NOARGS, + "Return True if the fast safe_memcpy copy path is currently active" }, + { "_set_fast_copy_warmup_seconds", + stack_set_fast_copy_warmup_seconds, + METH_VARARGS, + "Test-only: set the fast-copy startup warmup duration in seconds (before start)" }, { "uninstall_segv_handler", stack_uninstall_segv_handler, METH_NOARGS, @@ -964,6 +1022,10 @@ static PyMethodDef stack_methods[] = { stack_reinstall_segv_handler, METH_NOARGS, "Reinstall SIGSEGV handler after another component overwrites it" }, + { "segv_handler_installed", + stack_segv_handler_installed, + METH_NOARGS, + "Return True if ddtrace's handler is the currently installed disposition for SIGSEGV and SIGBUS" }, // Sampling pause/resume for safe signal handler swapping { "pause_sampling", stack_pause_sampling, diff --git a/releasenotes/notes/profiling-stack-sampler-foreign-handler-fallback-3d8f1b6a0e25c947.yaml b/releasenotes/notes/profiling-stack-sampler-foreign-handler-fallback-3d8f1b6a0e25c947.yaml new file mode 100644 index 00000000000..cbd3305a823 --- /dev/null +++ b/releasenotes/notes/profiling-stack-sampler-foreign-handler-fallback-3d8f1b6a0e25c947.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + profiling: crash fix for stack sampler in case where another + component installs its own ``SIGSEGV``/``SIGBUS`` handler that the profiler cannot safely chain with (e.g. CUDA, PyTorch, abseil via vLLM/gRPC; see sampler.cpp). + The sampler upgrades to the faster fault-recovery copy if it still owns both fault handlers afterwards. + Otherwise, it permanently falls back to the syscall-based copy. diff --git a/tests/profiling/collector/test_copy_memory_stats.py b/tests/profiling/collector/test_copy_memory_stats.py index 25430c926c9..b979a503eba 100644 --- a/tests/profiling/collector/test_copy_memory_stats.py +++ b/tests/profiling/collector/test_copy_memory_stats.py @@ -32,6 +32,13 @@ def test_copy_memory_error_count_present(): metadata = json.load(fp) assert "copy_memory_error_count" in metadata, f"Missing copy_memory_error_count in {f}: {metadata}" assert metadata["copy_memory_error_count"] >= 0, f"copy_memory_error_count must be non-negative: {metadata}" + assert "fast_copy_memory_user_disabled" in metadata, ( + f"Missing fast_copy_memory_user_disabled in {f}: {metadata}" + ) + assert "fast_copy_memory_capable" in metadata, f"Missing fast_copy_memory_capable in {f}: {metadata}" + assert "fast_copy_memory_syscall_fallback" in metadata, ( + f"Missing fast_copy_memory_syscall_fallback in {f}: {metadata}" + ) @pytest.mark.subprocess( @@ -70,6 +77,8 @@ def test_fast_copy_memory_disabled(): assert metadata["fast_copy_memory_enabled"] is False, ( f"Expected fast_copy_memory_enabled=false when _DD_PROFILING_STACK_FAST_COPY=false: {metadata}" ) + assert metadata["fast_copy_memory_user_disabled"] is True, metadata + assert metadata["fast_copy_memory_syscall_fallback"] is False, metadata @pytest.mark.subprocess( @@ -80,31 +89,50 @@ def test_fast_copy_memory_disabled(): ), err=None, ) -def test_fast_copy_memory_enabled(): - """fast_copy_memory_enabled is True when _DD_PROFILING_STACK_FAST_COPY=1.""" +def test_fast_copy_memory_enabled() -> None: + """Sampler runs on the syscall copy during warmup, then upgrades to safe_memcpy (PROF-14568).""" import glob import json import os import time + # Underscore-prefixed, so only on the _stack submodule (`import *` skips it). + from ddtrace.internal.datadog.profiling.stack import _stack from ddtrace.profiling import profiler from ddtrace.trace import tracer - p = profiler.Profiler(tracer=tracer) + _stack._set_fast_copy_warmup_seconds(1.0) + + p: profiler.Profiler = profiler.Profiler(tracer=tracer) p.start() - time.sleep(3) + + # Require warmup (False) before accepting the upgrade (True), so the brief + # constructor-time True isn't mistaken for it. + saw_warmup: bool = False + saw_upgrade: bool = False + deadline: float = time.monotonic() + 10 + while time.monotonic() < deadline: + active: bool = _stack.fast_copy_memory_active() + if not saw_warmup: + if active is False: + saw_warmup = True + elif active is True: + saw_upgrade = True + break + time.sleep(0.05) + p.stop() output_filename = os.environ["DD_PROFILING_OUTPUT_PPROF"] + "." + str(os.getpid()) files = sorted(glob.glob(output_filename + ".*.internal_metadata.json")) assert files, "Expected at least one internal_metadata.json file" - for i, f in enumerate(files): - is_last_file = i == len(files) - 1 - with open(f) as fp: - metadata = json.load(fp) - if not is_last_file: - assert "fast_copy_memory_enabled" in metadata, f"Missing fast_copy_memory_enabled in {f}: {metadata}" - assert metadata["fast_copy_memory_enabled"] is True, ( - f"Expected fast_copy_memory_enabled=true by default: {metadata}" - ) + with open(files[-1]) as fp: + metadata = json.load(fp) + assert metadata["fast_copy_memory_user_disabled"] is False, metadata + assert metadata["fast_copy_memory_capable"] is True, metadata + assert metadata["fast_copy_memory_syscall_fallback"] is False, metadata + assert metadata["fast_copy_memory_enabled"] is True, metadata + + assert saw_warmup, "Expected the sampler to run on the syscall copy during the warmup window" + assert saw_upgrade, "Expected the sampler to upgrade to safe_memcpy after warmup" diff --git a/tests/profiling/test_main.py b/tests/profiling/test_main.py index bf1adea360c..ea78158fe29 100644 --- a/tests/profiling/test_main.py +++ b/tests/profiling/test_main.py @@ -6,6 +6,7 @@ import pytest +from ddtrace.internal.datadog.profiling import stack as _stack_ext from tests.profiling.collector import lock_utils from tests.profiling.collector import pprof_utils from tests.utils import call_program @@ -247,3 +248,40 @@ def test_profiler_start_up_with_module_clean_up_in_protobuf_app() -> None: from google.protobuf import empty_pb2 # noqa:F401 print("OK") + + +@pytest.mark.skipif(sys.platform == "win32", reason="stack v2 profiler is not available on Windows") +@pytest.mark.skipif(not _stack_ext.is_available, reason="stack v2 native extension not available") +@pytest.mark.subprocess( + env=dict(_DD_PROFILING_STACK_FAST_COPY="1"), + out="OK\n", + err=None, +) +def test_stack_profiler_foreign_segv_handler_detection() -> None: + # Regression test for PROF-14568: safe_memcpy's fault recovery needs us to own + # BOTH SIGSEGV and SIGBUS. segv_handler_installed() drives the sampler's + # detect-and-fallback decision; assert it flips when either signal is taken over. + import signal + + from ddtrace.internal.datadog.profiling import stack + + # Our handler is installed by the extension's constructor on import. + assert stack.segv_handler_installed() is True + + # A foreign component overwrites the SIGSEGV disposition: ownership must flip. + signal.signal(signal.SIGSEGV, signal.SIG_DFL) + assert stack.segv_handler_installed() is False + + # Reclaiming the handler is detected too. + stack.reinstall_segv_handler() + assert stack.segv_handler_installed() is True + + # Losing only SIGBUS is just as unsafe as losing SIGSEGV: the predicate must + # report we no longer own the handler even though SIGSEGV is still ours. + signal.signal(signal.SIGBUS, signal.SIG_DFL) + assert stack.segv_handler_installed() is False + + stack.reinstall_segv_handler() + assert stack.segv_handler_installed() is True + + print("OK")