mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(daemon): a kill -9 left the server unable to start again
`kill -9` the server and it never comes back:
tty7-server did not open its endpoints within 10s —
it had already exited with an error
with the real reason only visible by running it by hand:
bind /var/…/T/tty7-<hash>.sock failed: Address already in use
A unix endpoint is a socket *file*. Killed uncleanly, the daemon leaves
it on disk, and `bind` on a path that exists fails instead of replacing
it. The user is stuck until they delete a file out of a temp directory
nothing points them at — after SIGKILL, an OOM kill, or a lost machine.
The startup path already had exactly the right code — probe the
endpoint, refuse if something answers, unlink it if nothing does — and
skipped it whenever the recorded daemon was gone. The comment gives the
reasoning: a dead recorded daemon cannot own a live endpoint, so there
is no point paying the refusal delay, and the bind below "overwrites the
file". That last part holds for a Windows daemon.port, which is a
recorded number; it does not hold for a socket file. So the shortcut
skipped the unlink precisely when the daemon had died the way that
leaves one behind.
Keep the shortcut for the probe, which is what costs, and unlink
whichever way the probe went.
Verified end to end on an isolated config dir: capture the socket path,
kill -9, confirm the file survives, and `server start` now recovers with
no manual cleanup. A live server is still refused and keeps serving —
the singleton seat turns the second one away before it reaches this
code, and only one server is left running.
The test pins the platform fact the fix rests on rather than the fix
itself: bind fails on a leftover socket path, and succeeds once it is
removed.
This commit is contained in:
@@ -543,23 +543,28 @@ fn report_conpty_host() {
|
||||
fn run_with(registry: Arc<Registry>) -> anyhow::Result<()> {
|
||||
crate::daemon::control::server_started();
|
||||
|
||||
// A stale daemon.port whose recorded daemon is gone cannot belong to a
|
||||
// live server: skip the probe (which would pay the OS's refusal delay on
|
||||
// the dead port) and let the bind below overwrite the file. A live
|
||||
// recorded daemon still gets the connect — the singleton seat is held by
|
||||
// this process, so it can only be a foreign server worth refusing.
|
||||
if transport::endpoint_exists() && !crate::daemon::spawn::recorded_daemon_is_dead() {
|
||||
match transport::connect() {
|
||||
Ok(_) => {
|
||||
anyhow::bail!(
|
||||
"daemon already running at {}",
|
||||
transport::endpoint_display()
|
||||
);
|
||||
}
|
||||
Err(_) => {
|
||||
transport::remove_stale_endpoint();
|
||||
}
|
||||
// A stale endpoint whose recorded daemon is gone cannot belong to a live
|
||||
// server, so there is nothing worth asking it: skip the probe, which would
|
||||
// otherwise pay the OS's refusal delay on a dead port. A live recorded
|
||||
// daemon still gets the connect — the singleton seat is held by this
|
||||
// process, so anything answering is a foreign server worth refusing.
|
||||
//
|
||||
// The removal is not part of that shortcut, though, and used to be: the
|
||||
// endpoint on unix is a socket *file*, and `bind` on a path that exists
|
||||
// fails with EADDRINUSE rather than replacing it, where a Windows
|
||||
// daemon.port is a recorded number the next bind simply overwrites. So the
|
||||
// one case that most needs the unlink — the recorded daemon gone, which is
|
||||
// every unclean death: SIGKILL, OOM, a lost machine — was the case that
|
||||
// skipped it, and the server could not start again until somebody deleted
|
||||
// a socket out of the temp dir by hand.
|
||||
if transport::endpoint_exists() {
|
||||
if !crate::daemon::spawn::recorded_daemon_is_dead() && transport::connect().is_ok() {
|
||||
anyhow::bail!(
|
||||
"daemon already running at {}",
|
||||
transport::endpoint_display()
|
||||
);
|
||||
}
|
||||
transport::remove_stale_endpoint();
|
||||
}
|
||||
|
||||
let listener = transport::bind()?;
|
||||
|
||||
@@ -167,6 +167,38 @@ mod tests {
|
||||
assert!(!endpoint_exists(), "endpoint cleared after removal");
|
||||
}
|
||||
|
||||
/// A socket file left behind by a killed daemon blocks the next `bind`.
|
||||
///
|
||||
/// This is the fact `run_with` turns on: a unix endpoint is a *file*, and
|
||||
/// binding a path that exists fails rather than replacing it, so the
|
||||
/// startup path has to unlink a dead predecessor's socket itself. Windows
|
||||
/// records a port instead, which the next bind simply overwrites — that
|
||||
/// difference is what let the unlink get skipped on the one path where it
|
||||
/// was load-bearing, and a `kill -9` then left the daemon unable to start.
|
||||
#[test]
|
||||
fn a_leftover_socket_file_blocks_the_next_bind_until_it_is_removed() {
|
||||
pin_config_dir();
|
||||
remove_stale_endpoint();
|
||||
|
||||
let dead = bind().expect("first bind");
|
||||
drop(dead); // as a killed daemon leaves it: file on disk, nobody listening
|
||||
assert!(endpoint_exists(), "SIGKILL leaves the socket file behind");
|
||||
assert!(
|
||||
connect().is_err(),
|
||||
"nothing is listening, so the path is provably stale"
|
||||
);
|
||||
|
||||
assert!(
|
||||
bind().is_err(),
|
||||
"binding an existing socket path fails — this is the whole bug"
|
||||
);
|
||||
|
||||
remove_stale_endpoint();
|
||||
let fresh = bind().expect("bind succeeds once the stale file is gone");
|
||||
drop(fresh);
|
||||
remove_stale_endpoint();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn socket_path_stays_in_config_dir_when_it_fits() {
|
||||
let dir = std::path::PathBuf::from("/tmp/tty7-short");
|
||||
|
||||
Reference in New Issue
Block a user