diff --git a/libdd-crashtracker/src/receiver/ptrace_collector.rs b/libdd-crashtracker/src/receiver/ptrace_collector.rs index 3857541c54..8f09e9ab32 100644 --- a/libdd-crashtracker/src/receiver/ptrace_collector.rs +++ b/libdd-crashtracker/src/receiver/ptrace_collector.rs @@ -173,7 +173,15 @@ fn attach_thread(tid: libc::pid_t, stop_deadline: Instant) -> Result<(), PtraceE // On older kernels, the register state may not be // immediately readable after waitpid reports the stop. Spin briefly // until PEEKUSER returns a non-zero IP, proving registers are committed. - wait_for_registers(tid, stop_deadline); + // + // If the deadline expires before we see a non-zero IP, proceed anyway: + // libunwind uses PTRACE_GETREGSET which may succeed even when PEEKUSER + // returns zero. If registers are truly uncommitted, unwind_remote_thread + // will return 0 frames and capture_with_retry will retry without needing + // a costly detach/re-attach cycle (which can fail with EPERM under CPU + // pressure because the kernel hasn't fully released the prior ptrace + // state). + let _ = wait_for_registers(tid, stop_deadline); Ok(()) } @@ -181,7 +189,11 @@ fn attach_thread(tid: libc::pid_t, stop_deadline: Instant) -> Result<(), PtraceE /// Poll the thread's instruction pointer using PTRACE_PEEKUSER until it is /// non-zero or the deadline expires. On modern kernels this should return on the /// first iteration; on older ones, it may take a few microseconds. -fn wait_for_registers(tid: libc::pid_t, deadline: Instant) { +/// +/// Returns `true` if a non-zero IP was observed or the check is not applicable +/// (PTRACE_PEEKUSER unsupported on the architecture), `false` if the +/// deadline expired without reading a valid IP on a platform that supports it. +fn wait_for_registers(tid: libc::pid_t, deadline: Instant) -> bool { #[cfg(target_arch = "x86_64")] const IP_OFFSET: libc::c_long = 16 * std::mem::size_of::() as libc::c_long; // RIP @@ -190,17 +202,33 @@ fn wait_for_registers(tid: libc::pid_t, deadline: Instant) { const SPIN_SLEEP: Duration = Duration::from_micros(100); + // First probe: if PTRACE_PEEKUSER returns EIO, the kernel doesn't support it + // In that case, skip the check. libunwind uses PTRACE_GETREGSET which works + // regardless, and modern kernels commit register state synchronously on ptrace-stop. + unsafe { *libc::__errno_location() = 0 }; + let ip = unsafe { libc::ptrace(libc::PTRACE_PEEKUSER, tid as libc::c_long, IP_OFFSET, 0) }; + let errno = unsafe { *libc::__errno_location() }; + if errno == libc::EIO { + return true; + } + if ip != 0 && errno == 0 { + return true; + } + loop { + if Instant::now() >= deadline { + return false; + } + std::thread::sleep(SPIN_SLEEP); unsafe { *libc::__errno_location() = 0 }; let ip = unsafe { libc::ptrace(libc::PTRACE_PEEKUSER, tid as libc::c_long, IP_OFFSET, 0) }; - // PEEKUSER returns the register value; errno==0 means success. - if ip != 0 && unsafe { *libc::__errno_location() } == 0 { - return; + let errno = unsafe { *libc::__errno_location() }; + if errno == libc::EIO { + return true; } - if Instant::now() >= deadline { - return; + if ip != 0 && errno == 0 { + return true; } - std::thread::sleep(SPIN_SLEEP); } } @@ -222,6 +250,14 @@ fn detach_thread(tid: libc::pid_t) -> Result<(), PtraceError> { return Err(PtraceError::Detach(tid, errno)); } } + + // Drain any pending waitpid event so the kernel fully releases the thread. + // Without this, a rapid re-attach (PTRACE_SEIZE) can fail with EPERM under + // CPU pressure because the kernel hasn't finished processing the detach. + unsafe { + libc::waitpid(tid, ptr::null_mut(), libc::__WALL | libc::WNOHANG); + } + Ok(()) } @@ -367,7 +403,66 @@ pub fn capture_thread_context( } /// Maximum time to wait for a single thread to enter ptrace-stop. -const STOP_TIMEOUT_PER_THREAD: Duration = Duration::from_millis(50); +const STOP_TIMEOUT_PER_THREAD: Duration = Duration::from_millis(200); + +/// Delay between retry attempts that is used as a base for an exponential back-off starting from +/// this value (10ms, 20ms, 40ms). +const RETRY_BASE_DELAY: Duration = Duration::from_millis(10); + +/// Maximum number of retry attempts per thread. +const MAX_RETRIES: u32 = 3; + +/// Returns true if err is worth retrying. +/// +/// EPERM (Yama denial / missing PR_SET_PTRACER) and ESRCH (thread exited) +/// are permanent for the receiver's lifetime and are not retried. +fn is_transient_ptrace_error(err: &PtraceError) -> bool { + matches!(err, PtraceError::Attach(_, libc::ETIMEDOUT)) +} + +/// Attempt to capture a thread context, retrying on transient failures. +/// +/// Each attempt gets its own `STOP_TIMEOUT_PER_THREAD` budget (capped at the +/// overall deadline) so that a retry after a timeout-induced failure actually +/// has enough time to succeed. On older kernels (CentOS 7 / kernel 3.10) +/// the first attempt can consume its entire budget waiting for registers to +/// become readable; reusing that exhausted deadline would make the retry a +/// no-op. +/// +/// A capture that succeeds but produces zero frames is also retried: on a +/// running thread with a confirmed non-zero IP, empty frames indicates a +/// transient issue. +fn capture_with_retry( + tid: libc::pid_t, + resolve_frames: crate::StacktraceCollection, + addr_space: UnwAddrSpaceT, + overall_deadline: Instant, +) -> Option { + for attempt in 0..=MAX_RETRIES { + let thread_deadline = (Instant::now() + STOP_TIMEOUT_PER_THREAD).min(overall_deadline); + + match capture_thread_context(tid, resolve_frames, addr_space, thread_deadline) { + Ok(ctx) if !ctx.stack_trace.frames.is_empty() => return Some(ctx), + Ok(_) => {} // 0 frames -- retry + Err(ref e) if is_transient_ptrace_error(e) => {} // ETIMEDOUT -- retry + Err(_) => return None, // permanent error + } + + if attempt == MAX_RETRIES { + break; + } + + let delay = RETRY_BASE_DELAY * 2u32.saturating_pow(attempt); + if Instant::now() + delay >= overall_deadline { + break; + } + std::thread::sleep(delay); + } + + // All attempts produced 0 frames or timed out; return None so the caller + // records the thread with an incomplete stack rather than frames: []. + None +} /// Stream thread contexts to a callback one at a time. /// @@ -412,9 +507,8 @@ where // Process the crashing thread first so it is never dropped by the cap. if crashing_tid != 0 && tids.contains(&crashing_tid) { - let stop_deadline = (Instant::now() + STOP_TIMEOUT_PER_THREAD).min(overall_deadline); let context = - capture_thread_context(crashing_tid, resolve_frames, addr_space, stop_deadline).ok(); + capture_with_retry(crashing_tid, resolve_frames, addr_space, overall_deadline); callback(crashing_tid, context.as_ref()); processed += 1; } @@ -423,16 +517,11 @@ where if tid == crashing_tid { continue; } - let now = Instant::now(); - if now >= overall_deadline || processed >= max_threads { + if Instant::now() >= overall_deadline || processed >= max_threads { break; } - // Cap the per-thread stop wait at STOP_TIMEOUT_PER_THREAD but never - // past the overall deadline, so one thread can't consume the budget. - let stop_deadline = (now + STOP_TIMEOUT_PER_THREAD).min(overall_deadline); - - let context = capture_thread_context(tid, resolve_frames, addr_space, stop_deadline).ok(); + let context = capture_with_retry(tid, resolve_frames, addr_space, overall_deadline); callback(tid, context.as_ref()); processed += 1; }