mirror of
https://github.com/herdrdev/herdr.git
synced 2026-09-21 16:01:04 +00:00
fix(client): restore terminal on sighup and sigterm
Enable termination-signal handling so SIGHUP and SIGTERM follow the graceful quit path and restore terminal state. Log handler registration failures and add PTY coverage for direct SIGHUP and server EOF restoration Co-authored-by: Matt Coles <macols@amazon.com>
This commit is contained in:
+1
-1
@@ -26,7 +26,7 @@ bytes = "1"
|
||||
clap = { version = "4.5", default-features = false, features = ["std", "help", "usage"] }
|
||||
clap_complete = "4.5"
|
||||
crossterm = "0.29"
|
||||
ctrlc = "3"
|
||||
ctrlc = { version = "3", features = ["termination"] }
|
||||
interprocess = "2.4.2"
|
||||
libc = "0.2"
|
||||
portable-pty = "=0.9.0"
|
||||
|
||||
+6
-3
@@ -1225,11 +1225,14 @@ fn run_client_with_mode(
|
||||
|
||||
let should_quit = Arc::new(AtomicBool::new(false));
|
||||
|
||||
// Install Ctrl+C handler.
|
||||
// ctrlc's "termination" feature also catches SIGTERM/SIGHUP so direct
|
||||
// termination signals still run the quit path and TerminalGuard::Drop.
|
||||
let quit_flag = should_quit.clone();
|
||||
let _ = ctrlc::set_handler(move || {
|
||||
if let Err(err) = ctrlc::set_handler(move || {
|
||||
quit_flag.store(true, Ordering::Release);
|
||||
});
|
||||
}) {
|
||||
warn!(%err, "failed to install termination handler; terminal restore relies on TerminalGuard::Drop and the panic hook");
|
||||
}
|
||||
|
||||
let result = rt.block_on(async {
|
||||
run_client_loop(
|
||||
|
||||
@@ -588,6 +588,189 @@ fn server_crash_after_attach_causes_lost_connection_error() {
|
||||
cleanup_test_base(&base);
|
||||
}
|
||||
|
||||
/// Any of the mouse-disable modes emitted by `clear_host_mouse_reporting` on
|
||||
/// terminal restore. Their presence in the client's PTY output proves the
|
||||
/// restore path (`TerminalGuard::Drop` → `restore_terminal_state`) ran.
|
||||
const MOUSE_TEARDOWN_MARKERS: [&str; 2] = ["\u{1b}[?1003l", "\u{1b}[?1000l"];
|
||||
|
||||
fn output_has_mouse_teardown(output: &str) -> bool {
|
||||
MOUSE_TEARDOWN_MARKERS
|
||||
.iter()
|
||||
.all(|marker| output.contains(marker))
|
||||
}
|
||||
|
||||
/// Shared buffer fed by a background PTY reader thread. Reading on a thread
|
||||
/// keeps the blocking `Box<dyn Read>` (which has no timeout) off the test's
|
||||
/// main thread, so a client that never exits fails the deadline instead of
|
||||
/// hanging the whole test forever.
|
||||
type SharedOutput = std::sync::Arc<Mutex<String>>;
|
||||
|
||||
fn spawn_pty_drain(mut reader: Box<dyn Read + Send>) -> SharedOutput {
|
||||
let output: SharedOutput = std::sync::Arc::new(Mutex::new(String::new()));
|
||||
let thread_output = output.clone();
|
||||
thread::spawn(move || {
|
||||
let mut buf = [0u8; 4096];
|
||||
loop {
|
||||
match reader.read(&mut buf) {
|
||||
Ok(0) => break,
|
||||
Ok(n) => thread_output
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
.push_str(&String::from_utf8_lossy(&buf[..n])),
|
||||
Err(_) => break,
|
||||
}
|
||||
}
|
||||
});
|
||||
output
|
||||
}
|
||||
|
||||
fn read_output(output: &SharedOutput) -> String {
|
||||
output.lock().unwrap_or_else(|p| p.into_inner()).clone()
|
||||
}
|
||||
|
||||
/// Current captured byte length, used as a watermark so a test can search only
|
||||
/// the output emitted *after* a trigger. The teardown markers also appear in
|
||||
/// normal attach-phase output, so matching the whole buffer is meaningless.
|
||||
fn output_len(output: &SharedOutput) -> usize {
|
||||
output.lock().unwrap_or_else(|p| p.into_inner()).len()
|
||||
}
|
||||
|
||||
/// Spawns a server + real thin client under a PTY and waits until the client
|
||||
/// has attached and rendered a frame. Returns the pieces plus a shared buffer
|
||||
/// that keeps accumulating PTY output (including teardown) on a background
|
||||
/// thread.
|
||||
fn attach_thin_client(
|
||||
config_home: &PathBuf,
|
||||
runtime_dir: &PathBuf,
|
||||
api_socket: &PathBuf,
|
||||
client_socket: &PathBuf,
|
||||
) -> (SpawnedHerdr, SpawnedHerdr, SharedOutput) {
|
||||
let spawned_server = spawn_server(config_home, runtime_dir, api_socket, client_socket);
|
||||
wait_for_socket(api_socket, Duration::from_secs(10));
|
||||
wait_for_socket(client_socket, Duration::from_secs(10));
|
||||
|
||||
let thin_client = spawn_client_process(config_home, runtime_dir, api_socket);
|
||||
let reader = thin_client
|
||||
._master
|
||||
.as_ref()
|
||||
.expect("thin client master")
|
||||
.try_clone_reader()
|
||||
.expect("clone client PTY reader");
|
||||
let output = spawn_pty_drain(reader);
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(8);
|
||||
let mut attached = false;
|
||||
while Instant::now() < deadline {
|
||||
let out = read_output(&output);
|
||||
if out.contains('\u{2500}')
|
||||
|| out.contains("workspace")
|
||||
|| out.contains("pane")
|
||||
|| out.contains("terminal")
|
||||
{
|
||||
attached = true;
|
||||
break;
|
||||
}
|
||||
if out.to_lowercase().contains("herdr:") {
|
||||
break;
|
||||
}
|
||||
thread::sleep(Duration::from_millis(30));
|
||||
}
|
||||
assert!(
|
||||
attached,
|
||||
"thin client must attach and render a frame; output: {:?}",
|
||||
read_output(&output)
|
||||
);
|
||||
|
||||
(spawned_server, thin_client, output)
|
||||
}
|
||||
|
||||
/// Polls until the client exits, then returns only the output captured after
|
||||
/// the `since` byte watermark. Panics if the client does not exit within the
|
||||
/// deadline.
|
||||
fn drain_until_client_exits(
|
||||
thin_client: &mut SpawnedHerdr,
|
||||
output: &SharedOutput,
|
||||
since: usize,
|
||||
) -> String {
|
||||
let deadline = Instant::now() + Duration::from_secs(12);
|
||||
let mut exited = false;
|
||||
while Instant::now() < deadline {
|
||||
if thin_client.child.try_wait().ok().flatten().is_some() {
|
||||
exited = true;
|
||||
break;
|
||||
}
|
||||
thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
// Give the reader thread a beat to flush trailing teardown bytes.
|
||||
thread::sleep(Duration::from_millis(100));
|
||||
let full = read_output(output);
|
||||
assert!(exited, "thin client should exit; output: {full:?}");
|
||||
full.get(since..).unwrap_or_default().to_string()
|
||||
}
|
||||
|
||||
/// Attaches a thin client, runs `trigger` to force an exit, and asserts the
|
||||
/// client emits the mouse teardown after that point. The teardown markers also
|
||||
/// appear in normal attach output, so only bytes emitted after the trigger
|
||||
/// (past the watermark) count.
|
||||
fn assert_client_restores_terminal(trigger: impl FnOnce(&mut SpawnedHerdr, &mut SpawnedHerdr)) {
|
||||
let _lock = test_lock();
|
||||
let base = unique_test_dir();
|
||||
let config_home = base.join("config");
|
||||
let runtime_dir = base.join("runtime");
|
||||
let api_socket = runtime_dir.join("herdr.sock");
|
||||
let client_socket = runtime_dir.join("herdr-client.sock");
|
||||
|
||||
let (mut spawned_server, mut thin_client, pty_output) =
|
||||
attach_thin_client(&config_home, &runtime_dir, &api_socket, &client_socket);
|
||||
|
||||
let since = output_len(&pty_output);
|
||||
trigger(&mut spawned_server, &mut thin_client);
|
||||
|
||||
let output = drain_until_client_exits(&mut thin_client, &pty_output, since);
|
||||
assert!(
|
||||
output_has_mouse_teardown(&output),
|
||||
"client must emit mouse teardown after trigger; output after trigger: {output:?}"
|
||||
);
|
||||
|
||||
// SpawnedHerdr::Drop kills and reaps both processes with a bounded wait.
|
||||
drop(spawned_server);
|
||||
cleanup_spawned_herdr(thin_client, base);
|
||||
}
|
||||
|
||||
/// The `--remote` ssh-death path: killing the bridge closes the socket, the
|
||||
/// client sees EOF and unwinds normally, so the terminal is restored. This is
|
||||
/// the path that does NOT deliver a signal to the client. Guards against a
|
||||
/// regression that would leave mouse reporting on after an ssh disconnect.
|
||||
#[test]
|
||||
fn client_restores_terminal_on_server_eof() {
|
||||
assert_client_restores_terminal(|server, _client| {
|
||||
// Kill the server unexpectedly; the client socket closes and the
|
||||
// client reader hits EOF, mirroring the ssh bridge dying under
|
||||
// `herdr --remote`.
|
||||
if let Some(pid) = server.child.process_id() {
|
||||
unsafe {
|
||||
libc::kill(pid as libc::pid_t, libc::SIGKILL);
|
||||
}
|
||||
}
|
||||
server.close_master();
|
||||
});
|
||||
}
|
||||
|
||||
/// The path this PR actually fixes: a direct SIGHUP/SIGTERM to the client (e.g.
|
||||
/// a terminal emulator SIGHUPing its foreground child on window close). With
|
||||
/// `ctrlc`'s `termination` feature the handler sets `should_quit`, the loop
|
||||
/// exits, and `TerminalGuard::Drop` restores the terminal. Without it the
|
||||
/// process would die un-unwound and leak mouse reporting.
|
||||
#[test]
|
||||
fn client_restores_terminal_on_sighup() {
|
||||
assert_client_restores_terminal(|_server, client| {
|
||||
let pid = client.child.process_id().expect("thin client pid") as libc::pid_t;
|
||||
unsafe {
|
||||
libc::kill(pid, libc::SIGHUP);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_receives_frame_after_pane_output() {
|
||||
// End-to-end test: server renders, client receives Frame.
|
||||
|
||||
Reference in New Issue
Block a user