Skip to content

fix: preserve Unix PTY lifecycle and final output - #1930

Open
raphamorim wants to merge 8 commits into
mainfrom
fix/unix-pty-lifecycle
Open

raphamorim wants to merge 8 commits into
mainfrom
fix/unix-pty-lifecycle

Conversation

@raphamorim

@raphamorim raphamorim commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Supersedes #1927 (thanks @rawkode; the original commit is preserved) with six review fixes on top.

Review fixes over #1927

  • Reader errors now close the terminal: the finalization path on Err emits ChildExited and calls terminal.exit(), so a reader failure no longer SIGKILLs the shell while leaving a live-looking pane whose keystrokes go into a dead channel.
  • Windows final output: Ok(0) from the ConPTY ring means momentarily empty, not closed, so the post-exit drain retries it until the 100ms deadline instead of stopping at the first empty read. Unix EOF semantics are unchanged and the drain tests pass on both platforms.
  • App quit no longer orphans shells. Route::quit was a bare process::exit(0) that skipped every destructor; quit paths now exit the event loop so exiting drops all routes. Context/Surface drops also deliver SIGHUP synchronously through a new ChildTerminator handle that shares the lifecycle state, so a reaped PID is never signaled (the handle no-ops on contention rather than stalling the UI thread behind a running escalation).
  • Signals target the process group (killpg(getpgid(pid))), like ghostty and kitty, reaching descendants and shrinking the PID-reuse window; it falls back to kill(pid) while the child still shares our own group.
  • SIGHUP grace raised from 100ms to 1s (shell HUP traps and history flushes routinely exceed 100ms; kitty and ghostty never escalate at all), and the post-SIGKILL reap is bounded: a child in uninterruptible sleep leaves terminate with TimedOut instead of blocking the lifecycle mutex forever. Lifecycle states HungUp/Killed ensure exactly one SIGHUP per close (HUP traps run once) and make retries after a timed-out reap a brief re-poll instead of a repeated 2s escalation.
  • killpg EPERM is tolerated on BSD-family targets: BSD killpg reports EPERM when any group member is unsignalable (setuid login(1) wrapper, sudo children) even though the signal reached the others; failing there would skip escalation and reaping and leak a zombie (see ghostty#2273).
  • Removed the dead kill_pid export and updated the librio child_pid() doc, which told embedders to killpg the pid themselves on teardown and would now race the internal escalation.

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.

2 participants