Repository navigation
1.44.1: end terminals one at a time (no remove_pty_baton assertion on quit / Update all) - #62
Merged
Merged
Conversation
…ndows) Quitting or Update all with two or more sessions intermittently showed "Assertion failed: remove_pty_baton(baton->id)" from conpty.node, or left DevDeck hanging or crashing (0xC0000005) on the way out. node-pty's Windows exit threads remove a terminal from one shared std::vector without a lock (a mutex lands only in 0.15.0-beta); two terminals ending together corrupted it. PtyHost.kill now chains: the next terminal is killed once the previous exit has been reported (by then its thread has finished with the list), or after 4 s. Shutdown's default budget grows with the number of terminals (1.5 s each, at least 5 s). qa/claude-update.mjs failed at quit on 3/20 and 3/32 runs before; 0/50 after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NLGwiBY5uEivJqERQzcoY
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NLGwiBY5uEivJqERQzcoY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom (reported with a screenshot): on Windows, quitting or running Update all with two or more sessions open sometimes shows a Microsoft Visual C++ Runtime Library dialog:
Other times DevDeck stays alive after its window is gone, or exits with 0xC0000005.
Cause: in
conpty.cc, each terminal's exit thread callsremove_pty_baton(). That function edits the globalstd::vector ptyHandleswithout a lock. When two terminals end together, the two threads race on the vector.@homebridge/node-pty-prebuilt-multiarchadds a mutex only in 0.15.0-beta (0.14.1 stable does not have it).Fix:
PtyHost.killends terminals one at a time. The next kill starts once the previous exit has reached JS, by which point its thread has finished with the vector, or after 4 s. The default shutdown budget scales with the number of terminals (1.5 s each, at least 5 s).Evidence:
exit 0, during native teardown.qa/claude-update.mjsstops all sessions at once twice, at Update all and at quit:vitest: 1511 passed, including a new test that terminals end one after another and that a missing exit holds the next one up for at most 4 s.🤖 Generated with Claude Code
https://claude.ai/code/session_016NLGwiBY5uEivJqERQzcoY