diff options
| author | akbash <akbash@herdr.dev> | 2026-08-09 17:45:40 +0300 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-08-09 17:45:40 +0300 |
| commit | ca1af383a82733608a0563fce2252058b478498c (patch) | |
| tree | 82e1f8ab817b637cecac29ea40b34b9404f2ff07 | |
| parent | e7c38ab37f967d16e2eacdb0bbde2d933a332057 (diff) | |
fix(client): avoid abort on terminal hangup (#2427)
* fix(client): avoid abort on terminal hangup
refs #2424
* test(client): gate PTY integration on unix
refs #2424
---------
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: Can Celik <ogulcancelik@gmail.com>
| -rw-r--r-- | src/client/mod.rs | 2 | ||||
| -rw-r--r-- | tests/client_mode.rs | 92 |
2 files changed, 88 insertions, 6 deletions
diff --git a/src/client/mod.rs b/src/client/mod.rs index 301377c9..0d28b47a 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -606,7 +606,7 @@ fn restore_terminal_state( restore_windows_input_mode_value(mode); } - ratatui::restore(); + let _ = ratatui::try_restore(); let _ = write_terminal_restore_postlude(&mut io::stdout(), reset_host_color_scheme_reports); #[cfg(windows)] diff --git a/tests/client_mode.rs b/tests/client_mode.rs index 10dcd19e..c92a62d1 100644 --- a/tests/client_mode.rs +++ b/tests/client_mode.rs @@ -1,5 +1,7 @@ //! Integration tests for thin client mode. +#![cfg(unix)] + mod support; use std::fs; @@ -756,11 +758,9 @@ fn client_restores_terminal_on_server_eof() { }); } -/// 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. +/// A direct SIGHUP/SIGTERM with a writable terminal follows the graceful quit +/// path and emits the terminal teardown. Actual terminal-window closure also +/// makes the PTY unwritable and is covered separately below. #[test] fn client_restores_terminal_on_sighup() { assert_client_restores_terminal(|_server, client| { @@ -771,6 +771,88 @@ fn client_restores_terminal_on_sighup() { }); } +fn read_until_client_attaches(client: &SpawnedHerdr) -> String { + let master = client._master.as_ref().expect("thin client master"); + let fd = master.as_raw_fd().expect("thin client PTY file descriptor"); + let flags = unsafe { libc::fcntl(fd, libc::F_GETFL) }; + assert_ne!(flags, -1, "read thin client PTY flags"); + assert_ne!( + unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) }, + -1, + "make thin client PTY nonblocking" + ); + + let mut reader = master.try_clone_reader().expect("clone client PTY reader"); + let mut output = String::new(); + let deadline = Instant::now() + Duration::from_secs(8); + while Instant::now() < deadline { + let mut buf = [0u8; 4096]; + match reader.read(&mut buf) { + Ok(0) => break, + Ok(n) => output.push_str(&String::from_utf8_lossy(&buf[..n])), + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(20)); + } + Err(err) => panic!("read thin client PTY: {err}"), + } + if output.contains('\u{2500}') + || output.contains("workspace") + || output.contains("pane") + || output.contains("terminal") + { + return output; + } + } + panic!("thin client must attach and render a frame; output: {output:?}"); +} + +#[test] +fn client_exits_cleanly_when_terminal_hangs_up() { + 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 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 mut thin_client = spawn_client_process(&config_home, &runtime_dir, &api_socket); + let attached_output = read_until_client_attaches(&thin_client); + + // Closing the final PTY master models the outer terminal disappearing: the + // foreground client receives SIGHUP and writes to stdout/stderr fail. + thin_client.close_master(); + let deadline = Instant::now() + Duration::from_secs(12); + let status = loop { + if let Some(status) = thin_client.child.try_wait().expect("poll thin client") { + break Some(status); + } + if Instant::now() >= deadline { + break None; + } + thread::sleep(Duration::from_millis(20)); + }; + let server_response = ping_socket(&api_socket); + + drop(spawned_server); + cleanup_spawned_herdr(thin_client, base); + + let status = status.unwrap_or_else(|| { + panic!("thin client did not exit after PTY hangup; attach output: {attached_output:?}") + }); + assert!( + status.success(), + "thin client should exit cleanly after PTY hangup, got {status}; attach output: {attached_output:?}" + ); + assert!( + server_response.contains("pong"), + "server should survive client PTY hangup: {server_response}" + ); +} + #[test] fn client_receives_frame_after_pane_output() { // End-to-end test: server renders, client receives Frame. |
