Skip to content

fix(core): quitting waits for every shell to be gone, on node-pty 1.2.0-beta.15 (#127) - #128

Merged
Maxaubert merged 1 commit into
mainfrom
fix/127-node-pty-beta
Oct 4, 2026
Merged

Maxaubert merged 1 commit into
mainfrom
fix/127-node-pty-beta

Conversation

@Maxaubert

Copy link
Copy Markdown
Owner

Closes #127.

What you saw: "Assertion failed! conpty.node ... remove_pty_baton(baton->id)" when the stable copy was closed for an update.

Two crashes at quit, both measured:

  1. The assertion dialog: node-pty 1.1.0's per-shell exit threads erase from one shared list with no lock, so shells dying together race. Fixed upstream (Windows ConPTY backend data race in ptyHandles can crash in remove_pty_baton microsoft/node-pty#921, PR #922), released only in 1.2.0-beta.13 and later. node-pty is pinned to 1.2.0-beta.15, the owner's pick. Its conpty.node no longer contains the assert.
  2. A silent abort (0xc0000409): about one quit in four with ten shells, on both versions. The WER dump stack is node::FreeEnvironment -> CleanupHandles -> ThreadSafeFunction::CallJS -> Napi::Error -> abort: a shell's exit callback landed while Node was tearing down.

Fix (core):

  • Every kill goes through killPty, which remembers the shell until it has exited.
  • will-quit holds the quit on shellsGone(3000), but only while a shell is actually dying. A hold that ended at once lost the follow-up app.quit() and left the app running; the opacityAlpha e2e caught that.
  • It waits for the agent's exitCode, which the native callback sets, not the exit event. The event lags 1 to 2.7 s for a pwsh killed mid-start (a warm shell). Quit stays at about 0.15 s.

Prism: gets the quit wait through the core bump. Its own node-pty upgrade will be a separate Prism PR.

Gate:

  • typecheck, lint (0 errors) and unit (1246) pass.
  • Full headless e2e: all scenarios pass.
  • New quitManyShells (ten shells, quit, three rounds, must exit 0 by itself): 24 of 24 rounds clean with the fix. Without it: 1 crash in 6 rounds on 1.1.0, 4 in 12 on the beta.

Core 0.23.1, app 0.28.3.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FHHaWKR4M5QtW7Wecyuk4t

….0-beta.15 (#127)

Closing the app with several shells could crash it at quit, two ways, both
measured. node-pty 1.1.0's exit threads raced on an unlocked vector and
asserted (the owner's "Assertion failed! remove_pty_baton" dialog), fixed
upstream in #922, so node-pty is pinned to 1.2.0-beta.15. And an exit
callback landing while Node tears down threw and Electron aborted
(0xc0000409, about one quit in four with ten shells; WER dump:
FreeEnvironment -> ThreadSafeFunction::CallJS -> abort). Every kill now goes
through killPty, and will-quit holds the quit on shellsGone (3 s cap) while
a shell is dying, reading the agent's exitCode, set by the native callback,
rather than the exit event that lags 1-2.7 s for a warm shell.

The quitManyShells e2e quits with ten shells three times and needs a clean
exit 0; it failed on both node-pty versions without the wait. Core 0.23.1,
app 0.28.3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FHHaWKR4M5QtW7Wecyuk4t
@Maxaubert
Maxaubert merged commit 672e7ae into main Oct 4, 2026
2 checks passed
@Maxaubert
Maxaubert deleted the fix/127-node-pty-beta branch October 4, 2026 21:07
github-actions Bot pushed a commit that referenced this pull request Oct 4, 2026
….0-beta.15 (#127) (#128)

Closing the app with several shells could crash it at quit, two ways, both
measured. node-pty 1.1.0's exit threads raced on an unlocked vector and
asserted (the owner's "Assertion failed! remove_pty_baton" dialog), fixed
upstream in #922, so node-pty is pinned to 1.2.0-beta.15. And an exit
callback landing while Node tears down threw and Electron aborted
(0xc0000409, about one quit in four with ten shells; WER dump:
FreeEnvironment -> ThreadSafeFunction::CallJS -> abort). Every kill now goes
through killPty, and will-quit holds the quit on shellsGone (3 s cap) while
a shell is dying, reading the agent's exitCode, set by the native callback,
rather than the exit event that lags 1-2.7 s for a warm shell.

The quitManyShells e2e quits with ten shells three times and needs a clean
exit 0; it failed on both node-pty versions without the wait. Core 0.23.1,
app 0.28.3.


Claude-Session: https://claude.ai/code/session_01FHHaWKR4M5QtW7Wecyuk4t

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

Closing the app with several shells can show node-pty's 'Assertion failed' dialog

1 participant