Skip to content

Commit c0e6ff5

Browse files
committed
fix(daemon): keep a detached daemon out of the launching terminal's session
Detaching left the daemon running, but closing the terminal that originally launched it killed the daemon anyway — so `fresh -a` came back with `Connection refused`, and a second client attached from another terminal was torn down along with it. The docs promise the opposite: "Terminal closes, but the Fresh daemon keeps running in the background." `spawn_server_detached` spawned the server as an ordinary child, so it inherited the client's session and process group: the terminal going away SIGHUP'd it, and a signal to that process group reached it. Windows already passed `DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`; the Unix side had no equivalent, and the `daemonize()` helper that would have provided one is not called from any path. Call `setsid()` from `pre_exec` so the spawned daemon leads its own session with no controlling terminal. `setsid` rather than `daemonize` because the daemon has to keep the inherited working directory — it serves that project — and `daemonize` chdirs to `/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjzTLqa27FEwYb7E2djB3B
1 parent 5d3695b commit c0e6ff5

2 files changed

Lines changed: 182 additions & 4 deletions

File tree

crates/fresh-editor/src/server/daemon/unix.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,21 @@ pub fn daemonize() -> io::Result<()> {
6363
/// spawned daemon boots into an SSH authority instead of the default
6464
/// `Authority::local()` (see `EditorServerConfig.startup_authority`).
6565
/// Returns the PID of the spawned server (intermediate, not final daemon PID).
66+
///
67+
/// The child calls `setsid()` before exec so the daemon leads its own session
68+
/// with no controlling terminal — the Unix counterpart of the Windows
69+
/// `DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`. Without it the daemon stayed
70+
/// in the launching terminal's session and process group: closing that terminal
71+
/// SIGHUP'd the daemon even though the user had already detached, and killing
72+
/// the terminal's process group took the daemon down with it — along with any
73+
/// client attached from another terminal.
74+
///
75+
/// `setsid()` rather than [`daemonize`] because the daemon must keep the
76+
/// inherited working directory (it serves that project); `daemonize` chdirs
77+
/// to `/`.
6678
pub fn spawn_server_detached(session_name: Option<&str>, ssh_url: Option<&str>) -> io::Result<u32> {
79+
use std::os::unix::process::CommandExt;
80+
6781
let exe = std::env::current_exe()?;
6882

6983
let mut args = vec!["--server".to_string()];
@@ -79,12 +93,27 @@ pub fn spawn_server_detached(session_name: Option<&str>, ssh_url: Option<&str>)
7993
}
8094

8195
// Use Command to spawn, which properly handles the process
82-
let child = std::process::Command::new(&exe)
83-
.args(&args)
96+
let mut cmd = std::process::Command::new(&exe);
97+
cmd.args(&args)
8498
.stdin(std::process::Stdio::null())
8599
.stdout(std::process::Stdio::null())
86-
.stderr(std::process::Stdio::null())
87-
.spawn()?;
100+
.stderr(std::process::Stdio::null());
101+
102+
// SAFETY: runs in the forked child between fork and exec, where only
103+
// async-signal-safe calls are allowed. `setsid` is one such call, and it
104+
// touches no allocator or lock this process holds. It fails with EPERM only
105+
// when the caller is already a session leader — harmless here, since that
106+
// means the child is already free of a controlling terminal, so the error is
107+
// deliberately not propagated (returning would abort an otherwise usable
108+
// daemon).
109+
unsafe {
110+
cmd.pre_exec(|| {
111+
libc::setsid();
112+
Ok(())
113+
});
114+
}
115+
116+
let child = cmd.spawn()?;
88117

89118
Ok(child.id())
90119
}
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
//! A detached daemon must outlive the terminal that launched it (issue #2808).
2+
//!
3+
//! Reported symptom: `fresh -a`, then `Detach` — the client exits and
4+
//! `fresh --cmd daemon list` still shows the daemon. Close the terminal that
5+
//! originally launched it and the daemon dies anyway, so `fresh -a` comes back
6+
//! with `Connection refused`; worse, a *second* client attached from another
7+
//! terminal is killed along with it. The docs promise the opposite: "Terminal
8+
//! closes, but the Fresh daemon keeps running in the background."
9+
//!
10+
//! Cause: `spawn_server_detached` spawned the server as a plain child, so it
11+
//! stayed in the launching terminal's session and process group. Windows already
12+
//! passed `DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`; the Unix side had no
13+
//! equivalent, and the `daemonize()` helper that would have provided one was
14+
//! never called from any path.
15+
//!
16+
//! Asserted here at the process level, because that is where the bug lives: the
17+
//! daemon must lead its own session, and must survive a `SIGKILL` delivered to
18+
//! the launching client's entire process group. `SIGKILL` to a process group is
19+
//! delivered to every member before `killpg` returns, so "still alive
20+
//! afterwards" is a deterministic verdict rather than a race.
21+
#![cfg(unix)]
22+
23+
use std::path::{Path, PathBuf};
24+
use std::time::Duration;
25+
26+
fn pty_available() -> bool {
27+
use portable_pty::{native_pty_system, PtySize};
28+
native_pty_system()
29+
.openpty(PtySize {
30+
rows: 24,
31+
cols: 80,
32+
pixel_width: 0,
33+
pixel_height: 0,
34+
})
35+
.is_ok()
36+
}
37+
38+
fn session_id_of(pid: u32) -> Option<i32> {
39+
let sid = unsafe { libc::getsid(pid as i32) };
40+
if sid == -1 {
41+
None
42+
} else {
43+
Some(sid)
44+
}
45+
}
46+
47+
fn process_alive(pid: u32) -> bool {
48+
unsafe { libc::kill(pid as i32, 0) == 0 }
49+
}
50+
51+
/// Block until `pid_file` names a live process, which is how the client itself
52+
/// waits for daemon readiness. Unbounded on purpose (see CONTRIBUTING: no
53+
/// in-test timeouts); `cargo nextest` bounds the whole test externally.
54+
fn wait_for_daemon_pid(pid_file: &Path) -> u32 {
55+
loop {
56+
if let Ok(text) = std::fs::read_to_string(pid_file) {
57+
if let Ok(pid) = text.trim().parse::<u32>() {
58+
if process_alive(pid) {
59+
return pid;
60+
}
61+
}
62+
}
63+
std::thread::sleep(Duration::from_millis(20));
64+
}
65+
}
66+
67+
#[test]
68+
fn detached_daemon_outlives_its_launching_process_group() {
69+
if !pty_available() {
70+
eprintln!("SKIP: no PTY available");
71+
return;
72+
}
73+
use portable_pty::{native_pty_system, CommandBuilder, PtySize};
74+
75+
let sandbox = tempfile::tempdir().unwrap();
76+
let mk = |n: &str| -> PathBuf {
77+
let p = sandbox.path().join(n);
78+
std::fs::create_dir_all(&p).unwrap();
79+
p
80+
};
81+
// Isolate the daemon's sockets/pid files and all persistence into the
82+
// sandbox so this test cannot see or disturb a real daemon.
83+
let runtime_dir = mk("run");
84+
let data_home = mk("data");
85+
let home = mk("home");
86+
let project = mk("project");
87+
std::fs::write(project.join("file.txt"), "hello\n").unwrap();
88+
89+
// A daemon name unique to this test run, so the pid file path is known and
90+
// cannot collide with anything else.
91+
let session_name = format!("t{}", std::process::id());
92+
let pid_file = runtime_dir
93+
.join("fresh")
94+
.join(format!("{session_name}.pid"));
95+
96+
// Launch the client on a PTY, exactly as a terminal would. The PTY child
97+
// becomes its own session leader, standing in for "the launching terminal";
98+
// the daemon it spawns must not join that session.
99+
let pair = native_pty_system()
100+
.openpty(PtySize {
101+
rows: 30,
102+
cols: 100,
103+
pixel_width: 0,
104+
pixel_height: 0,
105+
})
106+
.unwrap();
107+
let mut cmd = CommandBuilder::new(env!("CARGO_BIN_EXE_fresh"));
108+
cmd.args(["--no-upgrade-check", "-a", &session_name]);
109+
cmd.cwd(&project);
110+
cmd.env("XDG_RUNTIME_DIR", &runtime_dir);
111+
cmd.env("XDG_DATA_HOME", &data_home);
112+
cmd.env("HOME", &home);
113+
cmd.env("TERM", "xterm-256color");
114+
let mut client = pair.slave.spawn_command(cmd).unwrap();
115+
drop(pair.slave);
116+
117+
let daemon_pid = wait_for_daemon_pid(&pid_file);
118+
let client_pid = client.process_id().expect("client pid");
119+
120+
let client_sid = session_id_of(client_pid).expect("client session id");
121+
let daemon_sid = session_id_of(daemon_pid).expect("daemon session id");
122+
123+
// The daemon leads its own session. Sharing the client's session is what let
124+
// the terminal's teardown reach it.
125+
assert_ne!(
126+
daemon_sid, client_sid,
127+
"daemon (pid {daemon_pid}) must not share the launching client's session \
128+
(pid {client_pid}, sid {client_sid})"
129+
);
130+
131+
// The user-visible promise: tearing the launching terminal down — its whole
132+
// process group, the way a closing terminal does — leaves the daemon running.
133+
let client_pgid = unsafe { libc::getpgid(client_pid as i32) };
134+
assert!(client_pgid > 0, "could not read the client's process group");
135+
assert_eq!(
136+
unsafe { libc::killpg(client_pgid, libc::SIGKILL) },
137+
0,
138+
"failed to signal the launching process group"
139+
);
140+
let _ = client.wait();
141+
142+
assert!(
143+
process_alive(daemon_pid),
144+
"daemon (pid {daemon_pid}) died with the process group that launched it"
145+
);
146+
147+
// Leave nothing behind: the sandbox only isolates files, not processes.
148+
unsafe { libc::kill(daemon_pid as i32, libc::SIGKILL) };
149+
}

0 commit comments

Comments
 (0)