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>
This commit is contained in:
@@ -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<Value, St
|
||||
.and_then(|v| v.as_bool())
|
||||
.unwrap_or(false);
|
||||
|
||||
// Relaunch logic: check if we can reuse the existing connection
|
||||
let needs_relaunch = if let Some(ref mgr) = state.browser {
|
||||
// Relaunch logic: check if we can reuse the existing connection.
|
||||
// Fast process-exit check first to avoid expensive CDP timeout.
|
||||
let needs_relaunch = if let Some(ref mut mgr) = state.browser {
|
||||
let is_external = cdp_url.is_some() || cdp_port.is_some() || auto_connect;
|
||||
let was_external = mgr.is_cdp_connection();
|
||||
is_external != was_external || !mgr.is_connection_alive().await
|
||||
is_external != was_external || mgr.has_process_exited() || !mgr.is_connection_alive().await
|
||||
} else {
|
||||
true
|
||||
};
|
||||
|
||||
@@ -180,6 +180,14 @@ impl BrowserProcess {
|
||||
BrowserProcess::Lightpanda(p) => 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
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 <defunct> 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,
|
||||
|
||||
Reference in New Issue
Block a user