diff --git a/libshpool/src/attach.rs b/libshpool/src/attach.rs index 6693e6a7..38fbd3b6 100644 --- a/libshpool/src/attach.rs +++ b/libshpool/src/attach.rs @@ -421,11 +421,16 @@ enum AttachResult { struct SignalHandler { session_name: Arc>, socket: PathBuf, + last_sent_size: Mutex>, } impl SignalHandler { fn new(session_name: String, socket: PathBuf) -> Self { - SignalHandler { session_name: Arc::new(Mutex::new(session_name)), socket } + SignalHandler { + session_name: Arc::new(Mutex::new(session_name)), + socket, + last_sent_size: Mutex::new(None), + } } fn spawn(self) -> anyhow::Result>> { @@ -436,19 +441,44 @@ impl SignalHandler { let sigs = vec![SIGWINCH]; let mut signals = Signals::new(sigs).context("creating signal iterator")?; - thread::spawn(move || { - for signal in &mut signals { - let res = match signal { - SIGWINCH => self.handle_sigwinch(), + thread::spawn(move || loop { + let mut got_winch = false; + for signal in signals.wait() { + match signal { + SIGWINCH => got_winch = true, sig => { error!("unknown signal: {}", sig); panic!("unknown signal: {sig}"); } - }; - if let Err(e) = res { - error!("signal handler error: {:?}", e); } } + if !got_winch { + continue; + } + // Coalesce resize bursts. A fullscreen animation delivers a + // stream of SIGWINCHes; forwarding each one makes the attached + // application repaint per event, shoving one stale frame into + // the client's scrollback every time (operator finding X19, + // 2026-08-14). Wait for the reported size to stop moving and + // for the queue to run dry, then forward one resize carrying + // the final size. + loop { + let before = TtySize::from_fd(0).ok().map(|s| (s.rows, s.cols)); + thread::sleep(time::Duration::from_millis(120)); + let mut more = false; + for signal in signals.pending() { + if signal == SIGWINCH { + more = true; + } + } + let after = TtySize::from_fd(0).ok().map(|s| (s.rows, s.cols)); + if !more && before == after { + break; + } + } + if let Err(e) = self.handle_sigwinch() { + error!("signal handler error: {:?}", e); + } }); Ok(session_name_slot) @@ -467,6 +497,17 @@ impl SignalHandler { let tty_size = TtySize::from_fd(0).context("getting tty size")?; info!("handle_sigwinch: tty_size={:?}", tty_size); + // A size the daemon already holds triggers a whole-frame repaint + // for nothing; skip the duplicate instead of forwarding it. + { + let mut last = self.last_sent_size.lock().unwrap(); + if *last == Some((tty_size.rows, tty_size.cols)) { + info!("handle_sigwinch: size unchanged, not forwarding"); + return Ok(()); + } + *last = Some((tty_size.rows, tty_size.cols)); + } + // write the request on a new, seperate connection client .write_connect_header(ConnectHeader::SessionMessage(SessionMessageRequest { diff --git a/shpool/tests/regression.rs b/shpool/tests/regression.rs index e8a8e14d..e778bc42 100644 --- a/shpool/tests/regression.rs +++ b/shpool/tests/regression.rs @@ -334,3 +334,147 @@ fn pager_exit_transitions_to_shell() -> anyhow::Result<()> { Ok(()) } + +/// Regression test for resize bursts reaching the session one-for-one. A +/// fullscreen animation under a dragging window delivers a stream of +/// SIGWINCHes to the attach client; forwarding each one makes the attached +/// application repaint per event, shoving a stale frame into scrollback every +/// time. The client should coalesce a burst and forward one resize carrying +/// the final size. +/// +/// We attach through a real pty, have the session shell count the WINCHes it +/// receives, then deliver a rapid burst of client-side resizes. +#[test] +#[timeout(30000)] +fn resize_burst_reaches_session_as_one_resize() -> anyhow::Result<()> { + use std::os::fd::{AsRawFd, OwnedFd}; + + let mut daemon_proc = support::daemon::Proc::new("norc.toml", DaemonArgs::default()) + .context("starting daemon proc")?; + + let pty = nix::pty::openpty( + Some(&nix::pty::Winsize { ws_row: 24, ws_col: 80, ws_xpixel: 0, ws_ypixel: 0 }), + None, + ) + .context("opening pty pair")?; + let master: OwnedFd = pty.master; + let slave: OwnedFd = pty.slave; + + let attach_log = daemon_proc.tmp_dir.path().join("pty_attach.log"); + let mut cmd = Command::new(support::shpool_bin()?); + cmd.stdin(std::process::Stdio::from(slave.try_clone()?)) + .stdout(std::process::Stdio::from(slave.try_clone()?)) + .stderr(std::process::Stdio::from(slave)) + .arg("--config-file") + .arg(support::testdata_file("norc.toml")) + .arg("-v") + .arg("--log-file") + .arg(&attach_log) + .arg("--socket") + .arg(&daemon_proc.socket_path) + .arg("attach") + .arg("sh1") + .env_clear(); + if let Ok(path) = std::env::var("PATH") { + cmd.env("PATH", path); + } + if let Ok(xdg_runtime_dir) = std::env::var("XDG_RUNTIME_DIR") { + cmd.env("XDG_RUNTIME_DIR", xdg_runtime_dir); + } + if let Ok(home) = std::env::var("HOME") { + cmd.env("HOME", home); + } + let mut attach_proc = cmd.spawn().context("spawning pty attach")?; + std::thread::sleep(Duration::from_millis(1500)); + match attach_proc.try_wait() { + Ok(Some(status)) => { + let log = std::fs::read_to_string(&attach_log).unwrap_or_default(); + let mut pty_out = Vec::new(); + let flags = unsafe { nix::libc::fcntl(master.as_raw_fd(), nix::libc::F_GETFL) }; + unsafe { + nix::libc::fcntl( + master.as_raw_fd(), + nix::libc::F_SETFL, + flags | nix::libc::O_NONBLOCK, + ); + } + let mut buf = [0u8; 4096]; + while let Ok(n) = nix::unistd::read(&master, &mut buf) { + if n == 0 { + break; + } + pty_out.extend_from_slice(&buf[..n]); + } + panic!( + "pty attach exited early ({status:?}); log tail: {}; pty: {}", + &log[log.len().saturating_sub(2000)..], + String::from_utf8_lossy(&pty_out), + ); + } + _ => {} + } + daemon_proc.await_event("daemon-bidi-stream-enter")?; + + let mut reader = std::io::BufReader::new(std::fs::File::from(master.try_clone()?)); + let mut writer = std::fs::File::from(master.try_clone()?); + writer.write_all(b"trap 'echo W-I-N-C-H' WINCH; echo t-r-a-p-s-e-t\n")?; + let mut seen = String::new(); + { + use std::io::BufRead; + let mut line = String::new(); + loop { + line.clear(); + reader.read_line(&mut line)?; + if line.contains("t-r-a-p-s-e-t") && !line.contains("echo") { + break; + } + } + let _ = &seen; + } + + // A burst of eight client resizes inside the coalescing window, each with + // a genuinely different size so every forwarded resize would move the + // session pty and raise a WINCH in the shell. + for step in 0..24u16 { + let size = + nix::pty::Winsize { ws_row: 25 + step, ws_col: 81 + step, ws_xpixel: 0, ws_ypixel: 0 }; + let res = unsafe { nix::libc::ioctl(master.as_raw_fd(), nix::libc::TIOCSWINSZ, &size) }; + assert_eq!(res, 0, "setting pty size"); + // The client is not the pty's foreground process group, so deliver + // the signal a real terminal would raise. + let killed = + Command::new("kill").args(["-WINCH", &attach_proc.id().to_string()]).status()?; + assert!(killed.success(), "signalling attach client"); + std::thread::sleep(Duration::from_millis(10)); + } + + // Let the burst settle, then fence with a marker so every WINCH echo the + // shell will ever print is already in the stream when we count. + std::thread::sleep(Duration::from_millis(1200)); + writer.write_all(b"echo f-e-n-c-e\n")?; + { + use std::io::BufRead; + let mut line = String::new(); + loop { + line.clear(); + reader.read_line(&mut line)?; + if line.contains("W-I-N-C-H") && !line.contains("trap") { + seen.push('W'); + } + if line.contains("f-e-n-c-e") && !line.contains("echo") { + break; + } + } + } + + let _ = Command::new("kill").arg(attach_proc.id().to_string()).status(); + let _ = attach_proc.wait(); + + assert!( + seen.len() <= 2, + "a resize burst reached the session as {} resizes; it should coalesce to one", + seen.len() + ); + + Ok(()) +}