diff options
| -rw-r--r-- | Cargo.toml | 2 | ||||
| -rw-r--r-- | src/client/mod.rs | 9 | ||||
| -rw-r--r-- | tests/client_mode.rs | 183 |
3 files changed, 190 insertions, 4 deletions
@@ -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" diff --git a/src/client/mod.rs b/src/client/mod.rs index 6be41c06..52fd6dd6 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -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( diff --git a/tests/client_mode.rs b/tests/client_mode.rs index 21978d51..10dcd19e 100644 --- a/tests/client_mode.rs +++ b/tests/client_mode.rs @@ -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. |