Skip to content

Stop a displaced PTY with SIGHUP so an interactive bash actually exits - #1070

Merged
nedtwigg merged 1 commit into
mainfrom
fix/stop-displaced-pty-sighup
Oct 8, 2026
Merged

nedtwigg merged 1 commit into
mainfrom
fix/stop-displaced-pty-sighup

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

When a spawn lands on a PTY id that still has a live shell, pty-core replaces it and stops the old one, a behavior added in #1050. On macOS and Linux it stopped the old shell with the graceful SIGTERM, but an interactive bash ignores SIGTERM when it has no trap. The old generation is no longer in ptys, so kill, killAll, and gracefulKill can never reach it again, and the shell kept running until the app exited. That is the leak the stop was meant to prevent, and it contradicts docs/specs/transport.md: "a spawn over a live generation stops the displaced one".

This change stops the displaced PTY with node-pty's argument-less kill() instead, which sends SIGHUP on POSIX. Windows behavior is unchanged, since stopPty already ignores the signal there. The quit path's gracefulKill keeps SIGTERM, as the transport spec's "Graceful shutdown" section requires; Rust's process-group kill follows it.

The existing test in standalone/sidecar/pty-core.test.js pinned ['SIGTERM'] and now expects the default signal. It fails before the fix and passes after it, and the whole file passes (127 tests). The test never runs a real shell. I checked the shell's side directly: bash -i under a Python pty.fork() was still alive one second after SIGTERM and exited after SIGHUP.

An interactive bash ignores SIGTERM, and once a spawn replaces a live
generation in `ptys`, nothing can reach the old one again. The graceful
stop therefore left the displaced shell running until the app exited.
Stop it with node-pty's default kill() (SIGHUP on POSIX).
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6468acf
Status: ✅  Deploy successful!
Preview URL: https://1e58dc59.mouseterm.pages.dev
Branch Preview URL: https://fix-stop-displaced-pty-sighu.mouseterm.pages.dev

View logs

@nedtwigg
nedtwigg merged commit f7ecc95 into main Oct 8, 2026
12 checks passed
@nedtwigg
nedtwigg deleted the fix/stop-displaced-pty-sighup branch October 8, 2026 20:47

This branch is waiting to be deployed

1 waiting deployment
hosted-preview — 6468acfe Waiting Oct 8, 2026 by nedtwigg via cleanup #1237
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