Skip to content

fix(daemon): survive thread spawn failures #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jlongster wants to merge 1 commit into from
Closed

fix(daemon): survive thread spawn failures #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 35 additions & 19 deletions src/daemon/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,26 @@ pub fn run(directory: &Path) -> Result<()> {
let shutdown = Arc::clone(&shutdown);
let ownership = Arc::clone(&ownership);
let registration = registration.clone();
let handle = thread::spawn(move || {
if let Err(error) = handle_connection(
stream,
&service,
&registration,
&shutdown,
&ownership,
) {
eprintln!("opencode-pty request failed: {error:#}");
let spawned = thread::Builder::new()
.name("opencode-pty-connection".to_string())
.spawn(move || {
if let Err(error) = handle_connection(
stream,
&service,
&registration,
&shutdown,
&ownership,
) {
eprintln!("opencode-pty request failed: {error:#}");
}
});
// A failed spawn drops the connection, so the peer sees EOF.
match spawned {
Ok(handle) => handlers.push((control, handle)),
Err(error) => {
eprintln!("opencode-pty could not start connection handler: {error}");
}
});
handlers.push((control, handle));
}
}
Ok(None) => {
thread::sleep(Duration::from_millis(10));
Expand All @@ -73,13 +81,19 @@ pub fn run(directory: &Path) -> Result<()> {
}
let (cleanup_tx, cleanup_rx) = crossbeam_channel::bounded::<()>(1);
let cleanup = runtime.cleanup.clone();
let watchdog = thread::spawn(move || {
if cleanup_rx.recv_timeout(Duration::from_secs(5)).is_err() {
eprintln!("opencode-pty cleanup timed out; forcing exit");
let _ = cleanup.run();
std::process::exit(1);
}
});
let watchdog = thread::Builder::new()
.name("opencode-pty-watchdog".to_string())
.spawn(move || {
if cleanup_rx.recv_timeout(Duration::from_secs(5)).is_err() {
eprintln!("opencode-pty cleanup timed out; forcing exit");
let _ = cleanup.run();
std::process::exit(1);
}
})
.inspect_err(|error| {
eprintln!("opencode-pty could not start cleanup watchdog: {error}");
})
.ok();
service.shutdown();
for (_, handler) in handlers {
let _ = handler.join();
Expand All @@ -88,7 +102,9 @@ pub fn run(directory: &Path) -> Result<()> {
let cleanup = runtime.finish();
drop(listener);
let _ = cleanup_tx.send(());
let _ = watchdog.join();
if let Some(watchdog) = watchdog {
let _ = watchdog.join();
}
result.and(cleanup)
}

Expand Down
14 changes: 11 additions & 3 deletions src/daemon/unix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::transport::Listener;
pub(super) struct Runtime {
pub registration: Registration,
pub cleanup: Cleanup,
sweeper: JoinHandle<()>,
sweeper: Option<JoinHandle<()>>,
// Held until the shared server has joined its handlers and removed registration.
lock: File,
}
Expand Down Expand Up @@ -52,7 +52,13 @@ impl Runtime {
write_registration(&directory, &registration)?;
let sweeper = {
let directory = directory.clone();
thread::spawn(move || super::sweep::stale_runtimes(&directory))
thread::Builder::new()
.name("opencode-pty-sweeper".to_string())
.spawn(move || super::sweep::stale_runtimes(&directory))
.inspect_err(|error| {
eprintln!("opencode-pty could not start stale runtime sweeper: {error}");
})
.ok()
};
Ok((
Self {
Expand All @@ -71,7 +77,9 @@ impl Runtime {

/// Removes this daemon's runtime state after the server has joined its handlers.
pub fn finish(self) -> Result<()> {
let _ = self.sweeper.join();
if let Some(sweeper) = self.sweeper {
let _ = sweeper.join();
}
let result = self.cleanup.run();
drop(self.lock);
result
Expand Down
14 changes: 11 additions & 3 deletions src/daemon/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub(super) fn read_registration(directory: &Path) -> Result<Registration> {
pub(super) struct Runtime {
pub registration: Registration,
pub cleanup: Cleanup,
sweeper: JoinHandle<()>,
sweeper: Option<JoinHandle<()>>,
// Neither handle shares delete access, so the private directory and lock
// cannot be replaced while the daemon runs.
lock: File,
Expand Down Expand Up @@ -117,7 +117,13 @@ impl Runtime {
write_registration(&directory, &directory_handle, &registration, &security)?;
let sweeper = {
let directory = directory.clone();
thread::spawn(move || sweep::stale_runtimes(&directory))
thread::Builder::new()
.name("opencode-pty-sweeper".to_string())
.spawn(move || sweep::stale_runtimes(&directory))
.inspect_err(|error| {
eprintln!("opencode-pty could not start stale runtime sweeper: {error}");
})
.ok()
};
Ok((
Self {
Expand All @@ -137,7 +143,9 @@ impl Runtime {
/// Removes this daemon's runtime state after the server has joined its
/// handlers. The caller keeps the pipe namespace until this returns.
pub fn finish(self) -> Result<()> {
let _ = self.sweeper.join();
if let Some(sweeper) = self.sweeper {
let _ = sweeper.join();
}
let result = self.cleanup.run();
let directory = self.cleanup.directory;
// The protected handles deny deletion; release them first. A daemon that
Expand Down
Loading
Loading