From 5ac01fa743db475834df2ccf8bdf01f24208bba9 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 25 Mar 2026 08:04:43 -0700 Subject: [PATCH] fix: reap zombie Chrome process and fast-detect crash for auto-restart (#1023) When Chrome crashes (e.g. SIGTRAP from CHECK() assertion), the daemon now: 1. Reaps the zombie immediately via a SIGCHLD handler in the event loop that calls waitpid(-1, WNOHANG) 2. Detects the crash instantly on the next command via a non-blocking try_wait() check (has_process_exited), avoiding the 3-second CDP timeout that is_connection_alive() would incur 3. Auto-relaunches Chrome transparently for the caller Fixes #1017 Co-authored-by: ctate <366502+ctate@users.noreply.github.com> --- cli/src/native/actions.rs | 15 +++++++++------ cli/src/native/browser.rs | 19 +++++++++++++++++++ cli/src/native/cdp/chrome.rs | 11 +++++++++++ cli/src/native/daemon.rs | 23 +++++++++++++++++++++++ 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index f502c0a..2365689 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -993,9 +993,11 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value { | "device_list" ); if !skip_launch { - // Check if existing connection is stale and needs re-launch - let needs_launch = if let Some(ref mgr) = state.browser { - !mgr.is_connection_alive().await + // Check if existing connection is stale and needs re-launch. + // First do a fast, non-blocking check: did the browser process crash/exit? + // This avoids a 3-second CDP timeout when Chrome is already dead. + let needs_launch = if let Some(ref mut mgr) = state.browser { + mgr.has_process_exited() || !mgr.is_connection_alive().await } else { true }; @@ -1356,11 +1358,12 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result p.kill(), } } + + /// Non-blocking check whether the browser process has exited. + pub fn has_exited(&mut self) -> bool { + match self { + BrowserProcess::Chrome(p) => p.has_exited(), + BrowserProcess::Lightpanda(_) => false, + } + } } pub struct BrowserManager { @@ -643,6 +651,17 @@ impl BrowserManager { } } + /// Non-blocking check whether the locally-launched browser process has exited + /// (crashed or terminated). Also reaps the zombie if it has exited. + /// Returns false for external CDP connections (no child process to monitor). + pub fn has_process_exited(&mut self) -> bool { + if let Some(ref mut process) = self.browser_process { + process.has_exited() + } else { + false + } + } + pub fn get_cdp_url(&self) -> &str { &self.ws_url } diff --git a/cli/src/native/cdp/chrome.rs b/cli/src/native/cdp/chrome.rs index 249aa7d..a7d5db1 100644 --- a/cli/src/native/cdp/chrome.rs +++ b/cli/src/native/cdp/chrome.rs @@ -17,6 +17,17 @@ impl ChromeProcess { let _ = self.child.wait(); } + /// Returns the OS process ID of the Chrome child process. + pub fn id(&self) -> u32 { + self.child.id() + } + + /// Non-blocking check whether Chrome has exited. + /// Returns `true` if the process has exited (and reaps it), `false` if still running. + pub fn has_exited(&mut self) -> bool { + matches!(self.child.try_wait(), Ok(Some(_)) | Err(_)) + } + /// Wait for Chrome to exit on its own (after Browser.close CDP command), /// falling back to kill() if it doesn't exit within the timeout. /// This allows Chrome to flush cookies and other state to the user-data-dir. diff --git a/cli/src/native/daemon.rs b/cli/src/native/daemon.rs index faf616e..3d2b2d8 100644 --- a/cli/src/native/daemon.rs +++ b/cli/src/native/daemon.rs @@ -109,6 +109,12 @@ async fn run_socket_server( let (reset_tx, mut reset_rx) = mpsc::channel::<()>(64); let reset_tx = idle_timeout_ms.map(|_| Arc::new(reset_tx)); + // Listen for SIGCHLD to reap zombie child processes (e.g. crashed Chrome). + // Without this, a crashed Chrome becomes and is never reaped until + // the daemon exits. + let mut sigchld = signal::unix::signal(signal::unix::SignalKind::child()) + .map_err(|e| format!("Failed to install SIGCHLD handler: {}", e))?; + loop { let sleep_future = idle_timeout_ms.map(|ms| tokio::time::sleep(Duration::from_millis(ms))); let mut sleep_pin = sleep_future.map(Box::pin); @@ -128,6 +134,12 @@ async fn run_socket_server( } } } + _ = sigchld.recv() => { + // Reap all zombie children. The browser will be re-launched + // automatically on the next command via the has_process_exited() + // check in execute_command. + reap_children(); + } _ = async { if let Some(ref mut s) = sleep_pin { s.as_mut().await @@ -157,6 +169,17 @@ async fn run_socket_server( Ok(()) } +/// Reap all zombie child processes by calling waitpid(-1, WNOHANG) in a loop. +#[cfg(unix)] +fn reap_children() { + loop { + let result = unsafe { libc::waitpid(-1, std::ptr::null_mut(), libc::WNOHANG) }; + if result <= 0 { + break; + } + } +} + #[cfg(windows)] async fn run_socket_server( socket_path: &PathBuf,