diff --git a/Cargo.lock b/Cargo.lock index 3edcd216..8cdfe142 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1664,7 +1664,6 @@ name = "process-backend" version = "0.1.0" dependencies = [ "libc", - "log", "process-reader", "serde", "thiserror 2.0.19", diff --git a/crates/linux/process-backend/Cargo.toml b/crates/linux/process-backend/Cargo.toml index da1a306b..d2329ec2 100644 --- a/crates/linux/process-backend/Cargo.toml +++ b/crates/linux/process-backend/Cargo.toml @@ -13,7 +13,6 @@ testing = [] [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] libc.workspace = true -log.workspace = true process-reader = { version = "0.1.0", path = "../../process-reader", features = ["serde"] } serde.workspace = true thiserror.workspace = true diff --git a/crates/linux/process-backend/src/drop_fail_handler.rs b/crates/linux/process-backend/src/drop_fail_handler.rs new file mode 100644 index 00000000..b9056ae7 --- /dev/null +++ b/crates/linux/process-backend/src/drop_fail_handler.rs @@ -0,0 +1,54 @@ +//! Set a custom handler for when a Drop failure occurs +//! +//! Normally, there is not much that can be done when an error occurs during execution of `drop()` +//! -- Common strategies are to just ignore it, or to just log it and ignore it. +//! +//! We prefer the latter approach, but the issue is that if a Rust global logger has been set up +//! in the crashed process that will run the Executor, that could result in `log` being used in +//! a signal handler context. And since common logging behavior is to print to stderr, that will +//! almost-certainly cause a deadlock or some kind of other re-entrancy issue. +//! +//! Instead, we don't prescribe a behavior - We allow the user of minidump-writer to set their +//! own handler for failed `drop()` calls. If the user knows it's safe to log or print or +//! whatever, they can do that. +//! +//! Note that using [`install_global()`] to install a new handler will always overwrite any +//! previously-installed handler. This is intentional, as the user of this crate may want to +//! disable a previously-installed handler after a crash occurs (if that handler would cause +//! re-entrancy issues) + +use core::sync::atomic::{AtomicPtr, Ordering}; + +static HANDLER: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + +pub struct Handler { + pub(crate) f: for<'a> fn(core::fmt::Arguments<'a>), +} + +impl Handler { + /// Decorate the given function as a potential global Drop handler + pub const fn new(f: for<'a> fn(core::fmt::Arguments<'a>)) -> Self { + Self { f } + } + /// Install this handler as the new global Drop handler + /// + /// Will overwrite any previously installed handler + pub fn install_global(&'static self) { + HANDLER.store((self as *const Handler).cast_mut(), Ordering::Release); + } +} + +pub(crate) fn get() -> Option<&'static Handler> { + let ptr: *const Handler = HANDLER.load(Ordering::Acquire).cast_const(); + if ptr.is_null() { + None + } else { + Some(unsafe { &*ptr }) + } +} + +macro_rules! report_drop_failed(($($arg: tt)*) => {{ + if let Some(handler) = $crate::drop_fail_handler::get() { + (handler.f)(::core::format_args!($($arg)*)); + } +}}); diff --git a/crates/linux/process-backend/src/lib.rs b/crates/linux/process-backend/src/lib.rs index 62c5030b..6c9c72ce 100644 --- a/crates/linux/process-backend/src/lib.rs +++ b/crates/linux/process-backend/src/lib.rs @@ -1,6 +1,11 @@ #![no_std] #![cfg(any(target_os = "linux", target_os = "android"))] +pub use drop_fail_handler::Handler as DropFailHandler; + +#[macro_use] +mod drop_fail_handler; + pub mod local; pub mod regs; diff --git a/crates/linux/process-backend/src/local/mod.rs b/crates/linux/process-backend/src/local/mod.rs index f96c75ef..288f4e06 100644 --- a/crates/linux/process-backend/src/local/mod.rs +++ b/crates/linux/process-backend/src/local/mod.rs @@ -387,7 +387,7 @@ impl Drop for DirReader { fn drop(&mut self) { let rv = unsafe { libc::closedir(self.dirp) }; if rv == -1 { - log::debug!("failed to close directory: {}", errno()); + report_drop_failed!("failed to close directory: {}", errno()); } } } @@ -418,7 +418,7 @@ impl Drop for OwnedFd { fn drop(&mut self) { let rv = unsafe { libc::close(self.0) }; if rv == -1 { - log::error!("failed to close file: {}", errno()); + report_drop_failed!("failed to close file: {}", errno()); } } } diff --git a/crates/linux/process-backend/src/local/module_reader.rs b/crates/linux/process-backend/src/local/module_reader.rs index 7871a567..dfcb5762 100644 --- a/crates/linux/process-backend/src/local/module_reader.rs +++ b/crates/linux/process-backend/src/local/module_reader.rs @@ -149,7 +149,7 @@ impl Drop for Mapped { fn drop(&mut self) { let rv = unsafe { libc::munmap(self.ptr, self.len) }; if rv == -1 { - log::error!("failed to unmap memory: {}", errno()); + report_drop_failed!("failed to unmap memory: {}", errno()); } } } diff --git a/src/linux/process_inspection/mod.rs b/src/linux/process_inspection/mod.rs index b186c2e1..52a03f58 100644 --- a/src/linux/process_inspection/mod.rs +++ b/src/linux/process_inspection/mod.rs @@ -31,6 +31,8 @@ pub enum Backend { impl ProcessInspector { pub fn local(pid: libc::pid_t) -> Self { + set_process_backend_drop_fail_handler(); + let backend = local::Backend::new(pid); ProcessInspector { @@ -258,3 +260,12 @@ pub enum Error { #[error("an error occurred running a syscall directly")] Local(#[source] local::Error), } + +fn set_process_backend_drop_fail_handler() { + fn drop_fail_handler(args: core::fmt::Arguments<'_>) { + log::error!("a drop() error occurred in process-backend: {args}"); + } + const HANDLER: process_backend::DropFailHandler = + process_backend::DropFailHandler::new(drop_fail_handler); + HANDLER.install_global(); +}