From 0804b27f97ef1c3061bb9f5a16446a9567f48440 Mon Sep 17 00:00:00 2001 From: Burak Dede Date: Thu, 30 Jul 2026 00:08:29 +0200 Subject: [PATCH] Fix terminal guard SIGTTOU hang --- src/auth/gemini.rs | 2 +- src/lib.rs | 1 - src/terminal.rs | 45 ++++++++++++++++++++++++++++++--- tests/cli_basic.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/auth/gemini.rs b/src/auth/gemini.rs index a4c3a83..a70283d 100644 --- a/src/auth/gemini.rs +++ b/src/auth/gemini.rs @@ -400,7 +400,7 @@ fn run_oauth_flow( crate::output::print_info(" 2. Stay in this terminal; aisw will detect completion"); let result = (|| { - let terminal = TerminalGuard::capture(); + let terminal = TerminalGuard::capture_for_restore(); let mut child = spawn_oauth_child(gemini_bin, &scratch, &scratch_workdir)?; let cache_dir = scratch.join(GEMINI_CACHE_DIR); diff --git a/src/lib.rs b/src/lib.rs index d2031cb..1631a23 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,7 +54,6 @@ pub fn run() -> Result<()> { err.exit() } }; - let _terminal_guard = terminal::TerminalGuard::capture(); runtime::configure(cli.non_interactive, cli.quiet, output_mode); output::configure(cli.no_color, cli.quiet); commands::dispatch(cli) diff --git a/src/terminal.rs b/src/terminal.rs index 44240b8..16df86f 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -1,6 +1,8 @@ #[cfg(unix)] pub struct TerminalGuard { original: Option, + restore_on_drop: bool, + restored: std::cell::Cell, } #[cfg(not(unix))] @@ -8,27 +10,58 @@ pub struct TerminalGuard; #[cfg(unix)] impl TerminalGuard { + /// Snapshot terminal state without restoring it on drop. + /// + /// Use this for read-only inspection only. A restore is a terminal-setting + /// operation and can stop a Linux background process group with SIGTTOU. pub fn capture() -> Self { + Self::capture_with_restore(false) + } + + /// Snapshot terminal state and restore it on explicit restore or drop. + /// + /// Use this only around code that may leave the shared terminal modified, + /// such as an interactive child process. + pub fn capture_for_restore() -> Self { + Self::capture_with_restore(true) + } + + fn capture_with_restore(restore_on_drop: bool) -> Self { if unsafe { libc::isatty(libc::STDIN_FILENO) } != 1 { - return Self { original: None }; + return Self { + original: None, + restore_on_drop, + restored: std::cell::Cell::new(true), + }; } let mut term = std::mem::MaybeUninit::::uninit(); let rc = unsafe { libc::tcgetattr(libc::STDIN_FILENO, term.as_mut_ptr()) }; if rc != 0 { - return Self { original: None }; + return Self { + original: None, + restore_on_drop, + restored: std::cell::Cell::new(true), + }; } Self { original: Some(unsafe { term.assume_init() }), + restore_on_drop, + restored: std::cell::Cell::new(false), } } pub fn restore(&self) { + if self.restored.get() { + return; + } + let Some(original) = self.original.as_ref() else { return; }; let _ = unsafe { libc::tcsetattr(libc::STDIN_FILENO, libc::TCSANOW, original) }; + self.restored.set(true); } } @@ -38,13 +71,19 @@ impl TerminalGuard { Self } + pub fn capture_for_restore() -> Self { + Self + } + pub fn restore(&self) {} } #[cfg(unix)] impl Drop for TerminalGuard { fn drop(&mut self) { - self.restore(); + if self.restore_on_drop { + self.restore(); + } } } diff --git a/tests/cli_basic.rs b/tests/cli_basic.rs index dbb8964..1c35868 100644 --- a/tests/cli_basic.rs +++ b/tests/cli_basic.rs @@ -2,6 +2,8 @@ mod common; use common::TestEnv; use predicates::str::contains; +#[cfg(target_os = "linux")] +use std::process::Command as StdCommand; fn strip_ansi(input: &str) -> String { let mut stripped = String::with_capacity(input.len()); @@ -97,3 +99,64 @@ fn no_color_env_removes_ansi_from_parse_errors() { let stderr = String::from_utf8_lossy(&output.stderr); assert_eq!(stderr.as_ref(), strip_ansi(&stderr)); } + +#[cfg(target_os = "linux")] +#[test] +fn status_does_not_stop_when_timeout_runs_it_in_background_process_group_with_tty() { + if !command_exists("script") || !command_exists("timeout") { + eprintln!("skipping SIGTTOU regression: script or timeout not found"); + return; + } + + let env = TestEnv::new(); + let status_path = env.dir.path().join("status.out"); + let exit_path = env.dir.path().join("status.exit"); + let command = format!( + "timeout 5 {} status >{}; printf 'EXIT=%s\\n' $? >{}", + shell_quote(&env.aisw_bin()), + shell_quote(&status_path), + shell_quote(&exit_path) + ); + + let output = StdCommand::new("script") + .args(["-q", "-e", "-c", &command, "/dev/null"]) + .env("AISW_HOME", &env.aisw_home) + .env("PATH", env.shell_path()) + .env("HOME", &env.fake_home) + .env("AISW_KEYRING_TEST_DIR", env.fake_home.join("keychain")) + .env_remove("CLAUDE_CONFIG_DIR") + .env_remove("CODEX_HOME") + .env_remove("XDG_CONFIG_HOME") + .env_remove("XDG_DATA_HOME") + .env_remove("AISW_SECURITY_BIN") + .env_remove("AISW_SECURITY_KEYCHAIN") + .env_remove("AISW_CLAUDE_AUTH_STORAGE") + .env_remove("AISW_CODEX_AUTH_STORAGE") + .output() + .expect("failed to run SIGTTOU regression under script"); + + assert!( + output.status.success(), + "script wrapper failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + let exit = std::fs::read_to_string(&exit_path).expect("missing timeout exit file"); + assert_eq!(exit.trim(), "EXIT=0"); +} + +#[cfg(target_os = "linux")] +fn command_exists(name: &str) -> bool { + StdCommand::new("sh") + .args(["-c", &format!("command -v {name} >/dev/null 2>&1")]) + .status() + .map(|status| status.success()) + .unwrap_or(false) +} + +#[cfg(target_os = "linux")] +fn shell_quote(path: &std::path::Path) -> String { + let raw = path.to_string_lossy(); + format!("'{}'", raw.replace('\'', "'\\''")) +}