Visitar URL original
fix: end the editor and manager panes when the terminal hangs up by backnotprop · Pull Request #90 · plannotator/herdr-annotate · GitHub
Skip to content

fix: end the editor and manager panes when the terminal hangs up - #90

Open
backnotprop wants to merge 1 commit into
mainfrom
fix/editor-hangup-spin
Open

backnotprop wants to merge 1 commit into
mainfrom
fix/editor-hangup-spin

Conversation

@backnotprop

Copy link
Copy Markdown
Contributor

Refs plannotator/plannotator-tui#128 (the plannotator-tui half is plannotator/plannotator-tui#139). After the pane's terminal closes, the editor (and manager) process stays behind, adopted by launchd, at about 100% CPU, and SIGTERM does nothing.

Termination sets a flag on SIGTERM/SIGHUP, and the pane loops check it between event::poll calls. After a hang-up, crossterm's unix reader never returns: read gives Ok(0) forever and it only leaves its read loop on WouldBlock (crossterm-rs/crossterm#793, still open). So the loop never checks the flag again. This happens even when SIGHUP is delivered normally, because our handler replaces the default action.

Termination::install now also starts a watch thread (unix only) that wakes every 500 ms:

  • If the terminal reports POLLHUP, it calls ratatui::try_restore and exits with 129. It watches stdin, or stdout if stdin is not a terminal. A finished pipe reports POLLHUP too, so a pipe is never watched. tcgetattr/isatty cannot detect this: on macOS both keep succeeding on a hung-up pty.
  • If a signal is still pending one tick after it arrived, the loop is stuck, so the thread exits with 128 + the signal. It leaves the terminal alone in that case: a loop stuck in a write holds stdout.
  • The flag is now an AtomicUsize that holds the signal number (register_usize). requested() is unchanged for callers.

This only adds the event, stdio and termios features to the existing unix rustix dependency (pty for tests). Cargo.lock is unchanged.

Evidence

macOS, private tmux server, tmux kill-server:

before (main) after
editor, SIGHUP delivered orphan, PPID 1, state R, 99% CPU after 3 s; SIGTERM ignored 1.5 s later exits 0.12 s after the hang-up (3 runs)
editor, trap '' HUP same, SIGTERM ignored exits 0.1–0.6 s after the hang-up, status 129 (3 runs)
manager still running 3 s later exits, status 129
editor with the tty watch disabled (local experiment only, not in this diff) — the pending-signal fallback ends it 0.56–0.76 s after the hang-up

Esc in the editor and q in the manager still give 0, and SIGTERM on a live pane still takes the normal path (0, under 0.1 s).

Tests

termination::tests uses a real pty. It reads as hung up only after its controller is dropped, and a finished pipe on stdin is skipped for the terminal. cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test --all-targets (124 + 12) pass. On Windows the watch code is cfg(unix). cargo clippy --target x86_64-pc-windows-msvc --all-targets -- -D warnings passes locally, and CI's check-windows builds it for real.

Shipping this needs a helper release (rust-lite-v0.2.4) and a plugin pin bump, as for 0.2.3.

crossterm's unix reader never returns once its tty hangs up (read gives
Ok(0) forever, crossterm-rs/crossterm#793), so the panes never checked
the SIGTERM/SIGHUP flag again and spun at 100% CPU. A watch thread now
exits with status 129 when the terminal reports POLLHUP, and with
128+signal when a signal is still pending 500 ms after it arrived.

Fixes plannotator/plannotator-tui#128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant