Repository navigation
fix(win): don't close the pseudoconsole twice on a second kill() - #989
Open
shrmguy (leroyomey) wants to merge 1 commit into
Open
shrmguy (leroyomey) wants to merge 1 commit into
shrmguy (leroyomey) wants to merge 1 commit into
Conversation
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.
Fixes #983.
PtyKillcloses the pseudoconsole but leaveshandle->hpcpointing at it, and the handle stays inptyHandlesuntil the exit watcher removes it. A secondkill()in that gap closes the same pseudoconsole again, which withuseConptyDll: truetakes the process down with 0xC0000374.PtyKillnow setshpcto null once it has closed it, and skips the close if it's already null. I added the same check toPtyResizeandPtyClear, since they can run in the same gap and were passing the closed handle along.The script from the issue crashed every time for me on Windows 11 (26200, Node 24) and survives with this change, with cmd.exe and powershell.exe. The new test takes down the mocha run without the fix.
npm testpasses.My Build Tools install has no Spectre libraries, so I built locally with
SpectreMitigationoff. I didn't try theonExittiming variant from the issue, it needs a loaded machine. It goes through the same second close, so this should cover it.