Skip to content

Commit 6e72415

Browse files
committed
Only retry register wait timeout
1 parent edd03dc commit 6e72415

1 file changed

Lines changed: 28 additions & 21 deletions

File tree

libdd-crashtracker/src/receiver/ptrace_collector.rs

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -392,38 +392,45 @@ pub fn capture_thread_context(
392392
/// Maximum time to wait for a single thread to enter ptrace-stop.
393393
const STOP_TIMEOUT_PER_THREAD: Duration = Duration::from_millis(50);
394394

395-
/// Number of retry attempts for a failed capture_thread_context call.
396-
const CAPTURE_RETRIES: u32 = 1;
397-
398395
/// Delay between retry attempts.
399396
const RETRY_DELAY: Duration = Duration::from_millis(5);
400397

401-
/// Attempt to capture a thread context with retries on transient failures.
398+
/// Returns true if err is worth retrying.
399+
///
400+
/// Only ETIMEDOUT (register-wait deadline expired before IP became readable)
401+
/// should be retired.
402402
///
403-
/// On some kernels, ptrace operations can fail intermittently due to scheduling
404-
/// races or brief permission windows. Retrying once after a short delay resolves
405-
/// most transient issues without significantly impacting overall collection time.
403+
/// EPERM (Yama denial / missing PR_SET_PTRACER) and ESRCH (thread exited)
404+
/// are permanent for the receiver's lifetime and are not retried.
405+
fn is_transient_ptrace_error(err: &PtraceError) -> bool {
406+
matches!(err, PtraceError::Attach(_, libc::ETIMEDOUT))
407+
}
408+
409+
/// Attempt to capture a thread context, retrying once on transient failures.
410+
///
411+
/// Uses a single per-thread deadline across attempts so that a slow thread
412+
/// cannot consume more than STOP_TIMEOUT_PER_THREAD total.
406413
fn capture_with_retry(
407414
tid: libc::pid_t,
408415
resolve_frames: crate::StacktraceCollection,
409416
addr_space: UnwAddrSpaceT,
410417
overall_deadline: Instant,
411418
) -> Option<CapturedThreadContext> {
412-
for attempt in 0..=CAPTURE_RETRIES {
413-
let now = Instant::now();
414-
if now >= overall_deadline {
415-
break;
416-
}
417-
let stop_deadline = (now + STOP_TIMEOUT_PER_THREAD).min(overall_deadline);
418-
match capture_thread_context(tid, resolve_frames, addr_space, stop_deadline) {
419-
Ok(ctx) => return Some(ctx),
420-
Err(_) if attempt < CAPTURE_RETRIES => {
421-
std::thread::sleep(RETRY_DELAY);
422-
}
423-
Err(_) => break,
424-
}
419+
let thread_deadline = (Instant::now() + STOP_TIMEOUT_PER_THREAD).min(overall_deadline);
420+
421+
match capture_thread_context(tid, resolve_frames, addr_space, thread_deadline) {
422+
Ok(ctx) => return Some(ctx),
423+
Err(ref e) if is_transient_ptrace_error(e) => {}
424+
Err(_) => return None,
425425
}
426-
None
426+
427+
let remaining = thread_deadline.saturating_duration_since(Instant::now());
428+
if remaining <= RETRY_DELAY {
429+
return None;
430+
}
431+
std::thread::sleep(RETRY_DELAY);
432+
433+
capture_thread_context(tid, resolve_frames, addr_space, thread_deadline).ok()
427434
}
428435

429436
/// Stream thread contexts to a callback one at a time.

0 commit comments

Comments
 (0)