Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ All notable changes to bssh will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- **PTY session mouse tracking leak**: after a PTY session disconnects (normal exit, Ctrl+C, network drop, or panic), the local terminal no longer prints raw SGR mouse escape sequences when the mouse is moved. All cleanup paths (`TerminalStateGuard::Drop`, `force_terminal_cleanup`, and the panic hook via `TerminalGuard`) now emit the full set of mouse-tracking-off sequences (modes 1000, 1002, 1003, 1006, 1015) plus cursor-show and alternate-screen-exit on teardown. (#189)

## [2.1.1] - 2026-04-17

### Fixed
Expand Down
21 changes: 7 additions & 14 deletions src/commands/interactive_signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,21 +133,14 @@ impl TerminalGuard {
}
}

/// Restore terminal to normal mode
/// Restore terminal to normal mode.
///
/// Delegates to `force_terminal_cleanup()` from `pty::terminal` so that the
/// panic-hook path shares exactly the same cleanup logic (mouse tracking reset,
/// alternate-screen restore, cursor show, raw-mode off) as every other cleanup
/// path. No bespoke logic is duplicated here.
pub fn restore_terminal() -> Result<()> {
use crossterm::{execute, terminal};
use std::io;

// Disable raw mode if it was enabled
let _ = terminal::disable_raw_mode();

// Show cursor
let _ = execute!(
io::stdout(),
crossterm::cursor::Show,
terminal::LeaveAlternateScreen
);

crate::pty::terminal::force_terminal_cleanup();
Ok(())
}
}
Expand Down
153 changes: 144 additions & 9 deletions src/pty/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,19 @@

//! Terminal state management for PTY sessions.

use std::io::Write;
use std::sync::{
Arc, Mutex,
atomic::{AtomicBool, Ordering},
};

use anyhow::{Context, Result};
use crossterm::{
event::{DisableBracketedPaste, EnableBracketedPaste},
execute,
terminal::{disable_raw_mode, enable_raw_mode},
};
use once_cell::sync::Lazy;
use std::sync::{
Arc, Mutex,
atomic::{AtomicBool, Ordering},
};

/// Global terminal cleanup synchronization
/// Ensures only one cleanup attempt happens even with multiple guards
Expand Down Expand Up @@ -165,6 +167,15 @@ impl TerminalStateGuard {
eprintln!("Warning: Failed to disable bracketed paste mode during cleanup: {e}");
}

// Best-effort: disable all mouse tracking modes that a remote program may have
// enabled. Each write is independent so one failure does not abort the rest.
// Modes: 1000 (X11), 1002 (button-event), 1003 (any-event), 1006 (SGR),
// 1015 (urxvt), plus restore cursor visibility and alternate screen.
let _ = std::io::stdout().write_all(
b"\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\x1b[?1049l\x1b[?25h",
);
let _ = std::io::stdout().flush();

// Exit raw mode if it's globally active
if RAW_MODE_ACTIVE.load(Ordering::SeqCst) {
if let Err(e) = disable_raw_mode() {
Expand All @@ -179,9 +190,6 @@ impl TerminalStateGuard {
self.is_raw_mode_active.store(false, Ordering::Relaxed);
}

// TODO: Restore other terminal settings if needed
// For now, just exiting raw mode is sufficient

Ok(())
}
}
Expand All @@ -194,9 +202,41 @@ impl Drop for TerminalStateGuard {
}
}

/// Force terminal cleanup - can be called from anywhere to ensure terminal is restored
/// Force terminal cleanup - can be called from anywhere to ensure terminal is restored.
///
/// This is a best-effort, infallible cleanup that disables mouse tracking, resets
/// alternate screen and cursor visibility, and exits raw mode. Each operation is
/// performed independently so a failure in one does not prevent the rest.
///
/// # Panic-safety
///
/// This function is safe to call from a panic hook. It uses `try_lock()` rather than
/// `lock()` so it never deadlocks if the panicking thread already holds
/// `TERMINAL_MUTEX` (re-entrant acquisition of `std::sync::Mutex` on the same thread
/// would otherwise deadlock), and it tolerates a poisoned mutex without secondary
/// panics. The underlying operations (stdout writes, `disable_raw_mode`) are
/// individually safe to run without the mutex; the lock only serializes concurrent
/// teardown attempts.
///
/// When `try_lock()` fails — whether because the mutex is already held or because it
/// is poisoned — the cleanup body **always executes** regardless. The word
/// "unsynchronized" in the inline comment means the cleanup runs without holding the
/// lock; it does **not** mean the cleanup is skipped.
pub fn force_terminal_cleanup() {
let _guard = TERMINAL_MUTEX.lock().unwrap();
// Acquire the mutex if we can, but never block or panic on it. If the mutex is
// already held by this thread (re-entrant via panic hook) or poisoned by a
// previous panic, fall through and run the cleanup unsynchronized — the
// operations below are individually safe.
let _guard = TERMINAL_MUTEX.try_lock().ok();

// Best-effort: disable all mouse tracking modes, restore cursor, and leave alternate
// screen. Written as a single atomic blob to minimize partial-state risk.
// Modes: 1000 (X11), 1002 (button-event), 1003 (any-event), 1006 (SGR),
// 1015 (urxvt); then restore cursor visibility and normal screen buffer.
let _ = std::io::stdout()
.write_all(b"\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\x1b[?1049l\x1b[?25h");
let _ = std::io::stdout().flush();

if RAW_MODE_ACTIVE.load(Ordering::SeqCst) {
let _ = disable_raw_mode();
RAW_MODE_ACTIVE.store(false, Ordering::SeqCst);
Expand Down Expand Up @@ -284,3 +324,98 @@ impl TerminalOps {
Ok(())
}
}

#[cfg(test)]
mod tests {
use super::*;

// -----------------------------------------------------------------------
// force_terminal_cleanup tests
//
// Terminal-state mutation (raw mode, alternate screen) cannot be unit-
// tested safely inside `cargo test` because:
// • the test runner does not allocate a real TTY — crossterm operations
// that require a TTY (e.g. enable_raw_mode) will fail or behave
// unexpectedly;
// • the escape-sequence writes go to cargo's captured stdout, which is
// harmless but not observable in a meaningful way;
// • the global statics (TERMINAL_MUTEX, RAW_MODE_ACTIVE) are shared
// across all tests in the same process, so tests that mutate them
// must be carefully ordered or marked #[serial].
//
// What *can* be reliably tested here:
// 1. Idempotency: calling force_terminal_cleanup() twice does not panic.
// 2. Poisoned-mutex resilience: the `try_lock().ok()` pattern correctly
// yields None (rather than panicking) when the mutex is poisoned.
// Verified using a local Mutex so the global TERMINAL_MUTEX is never
// poisoned, keeping other tests unaffected.
// 3. Held-mutex resilience: `try_lock()` returns WouldBlock (not a
// deadlock) when a lock is already held. Verified using a local Mutex
// for the same isolation reason.
//
// Manual reproduction of the actual terminal fix (mouse tracking escape
// sequences) requires a real TTY (vim/tmux) and cannot be automated here.
// -----------------------------------------------------------------------

/// Calling force_terminal_cleanup() twice in succession must not panic.
///
/// In a non-TTY test environment RAW_MODE_ACTIVE is false (no test calls
/// enable_raw_mode), so disable_raw_mode() is never invoked. The escape-
/// sequence writes succeed silently against cargo's stdout pipe.
#[test]
fn test_force_terminal_cleanup_idempotent() {
force_terminal_cleanup();
force_terminal_cleanup();
// Reaching here without a panic is the assertion.
}

/// When a Mutex is poisoned, try_lock() returns Err(TryLockError::Poisoned)
/// and .ok() converts it to None — no secondary panic occurs. This mirrors
/// the exact pattern used inside force_terminal_cleanup() for TERMINAL_MUTEX.
///
/// We verify the property on a local Mutex so we never poison the global
/// TERMINAL_MUTEX (which would break other tests in this process).
#[test]
fn test_try_lock_ok_survives_poisoned_mutex() {
let m = Mutex::new(());

// Poison the mutex by panicking while holding the lock.
let _ = std::panic::catch_unwind(|| {
let _guard = m.lock().unwrap();
panic!("intentional poison");
});

assert!(m.is_poisoned(), "mutex should be poisoned after the above");

// try_lock().ok() must yield None without panicking — the same
// guarantee force_terminal_cleanup() relies on for TERMINAL_MUTEX.
let guard = m.try_lock().ok();
assert!(guard.is_none(), "expected None for a poisoned mutex");
// Reaching here without a panic confirms resilience.
}

/// When a Mutex is currently held, try_lock() returns
/// Err(TryLockError::WouldBlock) and .ok() yields None immediately —
/// no blocking or deadlock. This mirrors the re-entrant panic-hook
/// scenario that force_terminal_cleanup() is designed to survive.
#[test]
fn test_try_lock_ok_does_not_block_when_held() {
let m = Mutex::new(());
let _held = m.lock().unwrap(); // hold the lock on this thread

// On std::sync::Mutex a second try_lock from the same thread is
// Err(WouldBlock) (not a deadlock), and .ok() converts it to None.
let guard = m.try_lock().ok();
assert!(guard.is_none(), "expected None when lock is already held");
// Reaching here without blocking confirms the non-deadlock guarantee.
}

#[test]
fn test_terminal_state_default() {
let state = TerminalState::default();
assert!(!state.was_raw_mode);
assert!(!state.was_alternate_screen);
assert!(!state.was_mouse_enabled);
assert_eq!(state.size, (80, 24));
}
}
Loading