fix: prevent orphaned Chrome processes on daemon exit (#1137)
Three changes to ensure headless Chrome process trees are fully cleaned up when the daemon exits, whether gracefully or abnormally: 1. Spawn Chrome in its own process group (`setpgid(0,0)`) and kill the entire group (`kill(-pgid, SIGKILL)`) in `ChromeProcess::kill()`. This takes down all helper processes (GPU, renderer, utility, crashpad) instead of only the main Chrome PID. 2. On Linux, set `PR_SET_PDEATHSIG(SIGKILL)` on the Chrome process so the kernel automatically kills it when the daemon dies for any reason, including SIGKILL/OOM. No macOS equivalent exists. 3. Replace `process::exit(0)` in the daemon's close handler with a `Notify` signal back to the main loop, so Rust destructors (including `ChromeProcess::Drop`) actually run. Co-authored-by: ctate <366502+ctate@users.noreply.github.com>
This commit is contained in:
@@ -9,11 +9,24 @@ pub struct ChromeProcess {
|
||||
child: Child,
|
||||
pub ws_url: String,
|
||||
temp_user_data_dir: Option<PathBuf>,
|
||||
/// On Unix, the process group ID used to kill the entire Chrome process tree.
|
||||
#[cfg(unix)]
|
||||
pgid: Option<i32>,
|
||||
}
|
||||
|
||||
impl ChromeProcess {
|
||||
pub fn kill(&mut self) {
|
||||
let _ = self.child.kill();
|
||||
// On Unix, kill the entire process group to ensure Chrome helper
|
||||
// processes (GPU, renderer, utility, crashpad) are also terminated.
|
||||
// This prevents orphaned Chrome processes from blocking the user's
|
||||
// normal Chrome (issue #1113).
|
||||
#[cfg(unix)]
|
||||
if let Some(pgid) = self.pgid {
|
||||
unsafe {
|
||||
libc::kill(-pgid, libc::SIGKILL);
|
||||
}
|
||||
}
|
||||
let _ = self.child.wait();
|
||||
}
|
||||
|
||||
@@ -276,13 +289,39 @@ fn try_launch_chrome(chrome_path: &Path, options: &LaunchOptions) -> Result<Chro
|
||||
}
|
||||
};
|
||||
|
||||
let mut child = Command::new(chrome_path)
|
||||
.args(&args)
|
||||
let mut cmd = Command::new(chrome_path);
|
||||
cmd.args(&args)
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::piped())
|
||||
.spawn()
|
||||
.map_err(|e| {
|
||||
.stderr(Stdio::piped());
|
||||
|
||||
// Place Chrome in its own process group so we can kill the entire tree
|
||||
// (main process + GPU/renderer/utility/crashpad helpers) with a single
|
||||
// killpg(), preventing orphaned processes (issue #1113).
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::process::CommandExt;
|
||||
// SAFETY: pre_exec runs between fork() and exec() in the child.
|
||||
// Both prctl and setpgid are async-signal-safe.
|
||||
unsafe {
|
||||
cmd.pre_exec(|| {
|
||||
// On Linux, ask the kernel to send SIGKILL to this process
|
||||
// when the parent (daemon) dies for any reason, including
|
||||
// SIGKILL. This is the most robust orphan prevention
|
||||
// available and has no macOS equivalent.
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL);
|
||||
}
|
||||
// Create a new process group (PGID = own PID) so the
|
||||
// daemon can kill the entire Chrome tree in one call.
|
||||
libc::setpgid(0, 0);
|
||||
Ok(())
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let mut child = cmd.spawn().map_err(|e| {
|
||||
cleanup_temp_dir(&temp_user_data_dir);
|
||||
format!("Failed to launch Chrome at {:?}: {}", chrome_path, e)
|
||||
})?;
|
||||
@@ -317,10 +356,20 @@ fn try_launch_chrome(chrome_path: &Path, options: &LaunchOptions) -> Result<Chro
|
||||
}
|
||||
};
|
||||
|
||||
#[cfg(unix)]
|
||||
let pgid = {
|
||||
let pid = child.id() as i32;
|
||||
// The child called setpgid(0,0) via process_group(0), so its PGID
|
||||
// equals its own PID.
|
||||
Some(pid)
|
||||
};
|
||||
|
||||
Ok(ChromeProcess {
|
||||
child,
|
||||
ws_url,
|
||||
temp_user_data_dir,
|
||||
#[cfg(unix)]
|
||||
pgid,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1138,6 +1187,8 @@ mod tests {
|
||||
child,
|
||||
ws_url: String::new(),
|
||||
temp_user_data_dir: Some(dir.clone()),
|
||||
#[cfg(unix)]
|
||||
pgid: None,
|
||||
};
|
||||
// _process dropped here
|
||||
}
|
||||
|
||||
@@ -9,7 +9,7 @@ use std::time::Duration;
|
||||
|
||||
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||
use tokio::signal;
|
||||
use tokio::sync::{mpsc, RwLock};
|
||||
use tokio::sync::{mpsc, Notify, RwLock};
|
||||
|
||||
use super::actions::{execute_command, DaemonState};
|
||||
use super::cdp::client::CdpClient;
|
||||
@@ -176,6 +176,11 @@ 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));
|
||||
|
||||
// Notifier used by handle_connection to signal the daemon loop to exit
|
||||
// after a "close" command, instead of calling process::exit() which skips
|
||||
// destructors and can leave Chrome processes orphaned (issue #1113).
|
||||
let close_notify = Arc::new(Notify::new());
|
||||
|
||||
let mut drain_interval = tokio::time::interval(Duration::from_millis(100));
|
||||
drain_interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
||||
|
||||
@@ -190,8 +195,9 @@ async fn run_socket_server(
|
||||
let state = state.clone();
|
||||
let reset_tx = reset_tx.clone();
|
||||
let sf = stream_file.clone();
|
||||
let cn = close_notify.clone();
|
||||
tokio::spawn(async move {
|
||||
handle_connection(stream, state, reset_tx, sf).await;
|
||||
handle_connection(stream, state, reset_tx, sf, cn).await;
|
||||
});
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -229,6 +235,12 @@ async fn run_socket_server(
|
||||
.map(|ms| Box::pin(tokio::time::sleep(Duration::from_millis(ms))));
|
||||
continue;
|
||||
}
|
||||
_ = close_notify.notified() => {
|
||||
// "close" command was handled; browser already closed by
|
||||
// handle_close(). Break to run cleanup and exit gracefully
|
||||
// so destructors fire.
|
||||
break;
|
||||
}
|
||||
_ = shutdown_signal() => {
|
||||
let mut s = state.lock().await;
|
||||
if let Some(ref mut mgr) = s.browser {
|
||||
@@ -283,6 +295,8 @@ 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));
|
||||
|
||||
let close_notify = Arc::new(Notify::new());
|
||||
|
||||
let idle_sleep = idle_timeout_ms.map(|ms| tokio::time::sleep(Duration::from_millis(ms)));
|
||||
let mut idle_sleep_pin = idle_sleep.map(Box::pin);
|
||||
|
||||
@@ -294,8 +308,9 @@ async fn run_socket_server(
|
||||
let state = state.clone();
|
||||
let reset_tx = reset_tx.clone();
|
||||
let sf = stream_file.clone();
|
||||
let cn = close_notify.clone();
|
||||
tokio::spawn(async move {
|
||||
handle_connection(stream, state, reset_tx, sf).await;
|
||||
handle_connection(stream, state, reset_tx, sf, cn).await;
|
||||
});
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -321,6 +336,10 @@ async fn run_socket_server(
|
||||
.map(|ms| Box::pin(tokio::time::sleep(Duration::from_millis(ms))));
|
||||
continue;
|
||||
}
|
||||
_ = close_notify.notified() => {
|
||||
let _ = fs::remove_file(&port_path);
|
||||
break;
|
||||
}
|
||||
_ = shutdown_signal() => {
|
||||
let mut s = state.lock().await;
|
||||
if let Some(ref mut mgr) = s.browser {
|
||||
@@ -340,6 +359,7 @@ async fn handle_connection<S>(
|
||||
state: std::sync::Arc<tokio::sync::Mutex<DaemonState>>,
|
||||
idle_reset_tx: Option<Arc<mpsc::Sender<()>>>,
|
||||
stream_file_cleanup: Option<PathBuf>,
|
||||
close_notify: Arc<Notify>,
|
||||
) where
|
||||
S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin,
|
||||
{
|
||||
@@ -396,8 +416,12 @@ async fn handle_connection<S>(
|
||||
if let Some(ref path) = stream_file_cleanup {
|
||||
let _ = fs::remove_file(path);
|
||||
}
|
||||
// Signal the daemon loop to exit gracefully instead of
|
||||
// calling process::exit(), which skips destructors and
|
||||
// can leave Chrome processes orphaned (issue #1113).
|
||||
tokio::time::sleep(tokio::time::Duration::from_millis(100)).await;
|
||||
process::exit(0);
|
||||
close_notify.notify_one();
|
||||
return;
|
||||
}
|
||||
}
|
||||
Err(_) => break,
|
||||
|
||||
Reference in New Issue
Block a user