Skip to content

Keep nmshd alive when a resize races a shell's exit (#199) - #205

Merged
raiseCatError merged 2 commits into
feature/175-mux-interopfrom
fix/199-pty-resize-ebadf
Sep 29, 2026
Merged

raiseCatError merged 2 commits into
feature/175-mux-interopfrom
fix/199-pty-resize-ebadf

Conversation

@raiseCatError

@raiseCatError raiseCatError commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Closes #199

Stacked PR. Base: feature/175-mux-interop (PR #204). Merge order: #204 first, then this PR. Once #204 is merged, retarget this PR to dev. The only code difference from #204 is this fix.

Root cause

  • node-pty's UnixTerminal.resize() calls pty.resize(fd, …) synchronously with no state check.
  • node-pty closes the fd when the PTY socket closes (_close()), which can happen before it emits exit. A frontend can also send a resize just before it hears of the exit.
  • Resizing a shell that has exited therefore throws ioctl(2) failed, EBADF. This reproduces deterministically.

Production paths that could hit it:

  1. Service resize messages, handled inside the socket data handler: the connection keeps its owned session after the shell exits, so a synchronous throw there is uncaught and ends nmshd with every live session.
  2. The attach redraw nudge (immediate resize, then a 40 ms timer): sessions.has() stays true between the fd closing and exit.
  3. In-process mode: TerminalApp resizing during teardown.

Conclusion: resize after close is an expected teardown race, not a separate ordering bug.

Fix

  • ShellSession: it tracks exited (set on exit), and resizes after that are no-ops. Only EBADF (isClosedPtyError) is classified as the race; it marks the session closed and is ignored. Any other error still throws.
  • SessionService: every resize (client messages and both redraw steps) goes through one guarded resize(). An unexpected failure is reported to the client that caused it as {type: 'error', code: 'resize'} instead of escaping and ending the service.
  • Unchanged: session cleanup, exit reporting, attach and reattach redraw, fullscreen repaint, mouse and input mode restoration, and ordinary resizes.

Tests (tests/ptyResizeRace.test.ts)

  • Resize after exit and after kill.
  • A burst of resizes racing the exit, over five rounds.
  • Only EBADF classified; other errors still thrown.
  • The service survives late resizes before, during and after a shell exit, while another session keeps working and still resizes (stty size → 33x111).
  • An unexpected resize error is reported to the client, and the session stays usable.
  • On the old code, four behavioral tests fail with EBADF, deterministically rather than under load.
  • Repeated 5/5. The related service, lifecycle, hardening, in-process and mux tests pass 3/3.
  • Build, typecheck and git diff --check are clean. The full suite passed 604/604 twice.

CI

This repository's CI runs automatically only on PRs into dev/master, so a stacked PR has no automatic checks. It was run manually on this branch with workflow_dispatch: run 36518253918, Node 22 and Node 26 both passed.

node-pty closes a PTY's descriptor when its socket closes, which can be
before it reports the shell's exit, and a frontend can always send a
resize just before it hears of that exit. pty.resize() then threw
'ioctl(2) failed, EBADF' synchronously. In the session service that
happened inside the socket data handler (and the attach redraw timer),
so one late resize could end nmshd and every live session it owns.

ShellSession now tracks its own lifecycle: resizes after the shell has
exited are no-ops, and only the closed-descriptor error is classified as
that teardown race; any other failure still throws. The service routes
every resize through one guarded path that reports an unexpected failure
to the client that caused it instead of letting it end the service.

Regression tests reproduce the EBADF deterministically on the old code:
resize after exit and after kill, a burst of resizes racing the exit, a
service that must survive late resizes while another session keeps
working and resizing, and an unexpected resize error reported as a
protocol error.
After service.close() the shells exit asynchronously and write their
spool, recreating the runtime directory the test had just removed. The
tests now wait for those shells before cleaning up, so no directories
are left in TMPDIR.
@raiseCatError

Copy link
Copy Markdown
Owner Author

CI at current head d58503f (workflow_dispatch): run 36520050082. Node 22 and Node 26 both passed.

@raiseCatError
raiseCatError added this pull request to stack #210 September 29, 2026 10:19
@raiseCatError
raiseCatError merged commit c9308a5 into dev Sep 29, 2026
4 checks passed
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