From f569525b63a1e1d11c14b98b3e5484464c490406 Mon Sep 17 00:00:00 2001 From: sanchda <838104+sanchda@users.noreply.github.com> Date: Fri, 22 Nov 2024 18:37:10 +0000 Subject: [PATCH 1/2] Fix const, port to libc --- crashtracker/src/collector/crash_handler.rs | 41 +++++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/crashtracker/src/collector/crash_handler.rs b/crashtracker/src/collector/crash_handler.rs index f31b476c59..faf80b17c2 100644 --- a/crashtracker/src/collector/crash_handler.rs +++ b/crashtracker/src/collector/crash_handler.rs @@ -13,7 +13,7 @@ use libc::{ c_void, execve, mmap, sigaltstack, siginfo_t, MAP_ANON, MAP_FAILED, MAP_PRIVATE, PROT_NONE, PROT_READ, PROT_WRITE, SIGSTKSZ, }; -use nix::poll::{poll, PollFd, PollFlags}; +use libc::{poll, pollfd, POLLHUP}; use nix::sys::signal::{self, SaFlags, SigAction, SigHandler, SigSet}; use nix::sys::socket; use nix::sys::wait::{waitpid, WaitPidFlag, WaitStatus}; @@ -22,7 +22,7 @@ use std::ffi::CString; use std::fs::{File, OpenOptions}; use std::io::Write; use std::os::unix::{ - io::{BorrowedFd, FromRawFd, IntoRawFd, RawFd}, + io::{FromRawFd, IntoRawFd, RawFd}, net::UnixStream, }; use std::ptr; @@ -228,20 +228,29 @@ fn run_receiver_child(uds_parent: RawFd, uds_child: RawFd, stderr: RawFd, stdout } fn wait_for_pollhup(target_fd: RawFd, timeout_ms: i32) -> anyhow::Result { - // Need to convert the RawFd into a BorrowedFd to satisfy the PollFd prototype - let target_fd = unsafe { BorrowedFd::borrow_raw(target_fd) }; - let poll_fd = PollFd::new(&target_fd, PollFlags::POLLHUP); - - match poll(&mut [poll_fd], timeout_ms)? { - -1 => Err(anyhow::anyhow!("poll failed")), - 0 => Ok(false), - _ => match poll_fd - .revents() - .ok_or_else(|| anyhow::anyhow!("No revents found"))? - { - revents if revents.contains(PollFlags::POLLHUP) => Ok(true), - _ => Err(anyhow::anyhow!("poll returned unexpected result")), - }, + let mut poll_fds = [pollfd { + fd: target_fd, + events: POLLHUP, + revents: 0, + }]; + + match unsafe { poll(poll_fds.as_mut_ptr(), 1, timeout_ms) } { + -1 => Err(anyhow::anyhow!( + "poll failed with errno: {}", + std::io::Error::last_os_error() + )), + 0 => Ok(false), // Timeout occurred + _ => { + let revents = poll_fds[0].revents; + if revents & POLLHUP != 0 { + Ok(true) // POLLHUP detected + } else { + Err(anyhow::anyhow!( + "poll returned unexpected result: revents = {}", + revents + )) + } + } } } From d17c01f5a37d3ce23b023b370b68cbad166f9538 Mon Sep 17 00:00:00 2001 From: sanchda <838104+sanchda@users.noreply.github.com> Date: Fri, 22 Nov 2024 19:21:36 +0000 Subject: [PATCH 2/2] Also give some extra time for reaping --- crashtracker/src/collector/crash_handler.rs | 6 +++++- crashtracker/src/shared/constants.rs | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crashtracker/src/collector/crash_handler.rs b/crashtracker/src/collector/crash_handler.rs index faf80b17c2..51d5228b04 100644 --- a/crashtracker/src/collector/crash_handler.rs +++ b/crashtracker/src/collector/crash_handler.rs @@ -8,6 +8,7 @@ use super::emitters::emit_crashreport; use super::saguard::SaGuard; use crate::crash_info::CrashtrackerMetadata; use crate::shared::configuration::{CrashtrackerConfiguration, CrashtrackerReceiverConfig}; +use crate::shared::constants::*; use anyhow::Context; use libc::{ c_void, execve, mmap, sigaltstack, siginfo_t, MAP_ANON, MAP_FAILED, MAP_PRIVATE, PROT_NONE, @@ -493,7 +494,10 @@ fn receiver_finish(receiver: Receiver, start_time: Instant, timeout_ms: u32) { } let receiver_pid_as_pid = Pid::from_raw(receiver.receiver_pid); - let reaping_allowed_ms = timeout_ms.saturating_sub(start_time.elapsed().as_millis() as u32); + let reaping_allowed_ms = std::cmp::min( + timeout_ms.saturating_sub(start_time.elapsed().as_millis() as u32), + DD_CRASHTRACK_MINIMUM_REAP_TIME_MS, + ); let _ = reap_child_non_blocking(receiver_pid_as_pid, reaping_allowed_ms); } diff --git a/crashtracker/src/shared/constants.rs b/crashtracker/src/shared/constants.rs index 60b977f0cf..55206033c1 100644 --- a/crashtracker/src/shared/constants.rs +++ b/crashtracker/src/shared/constants.rs @@ -21,3 +21,5 @@ pub const DD_CRASHTRACK_END_SPAN_IDS: &str = "DD_CRASHTRACK_END_SPAN_IDS"; pub const DD_CRASHTRACK_END_STACKTRACE: &str = "DD_CRASHTRACK_END_STACKTRACE"; pub const DD_CRASHTRACK_END_TRACE_IDS: &str = "DD_CRASHTRACK_END_TRACE_IDS"; pub const DD_CRASHTRACK_DEFAULT_TIMEOUT_MS: u32 = 5_000; +pub const DD_CRASHTRACK_MINIMUM_REAP_TIME_MS: u32 = 160; // 4ms per sched slice, give ~4x10 slices + // for safety