Fix cleanup code - Githubissues

microsoft / node-pty

Fork pseudoterminals in Node.JS

Other

1.42k stars 233 forks source link

Fix cleanup code #626

Closed rzhao271 closed 9 months ago

rzhao271 commented 10 months ago

Fixes https://github.com/microsoft/node-pty/issues/512

rzhao271 commented 10 months ago

I have confirmed that I have moved the code to a part of the file that is called later:

WindowsPtyAgent#ctor calls into PtyStartProcess and PtyConnect
PtyConnect sets up a process exit listener that calls OnProcessExit
PtyKill closes the hShell handle, but OnProcessExit is the part of the code that actually reads hShell's exit code, so I'm thinking that OnProcessExit is always run later than PtyKill

I still have to test this PR, though.

rzhao271 commented 10 months ago

I have tested the PR DisconnectNamedPipe causes the crash. I have removed those calls for now and added a comment referencing the issue.

Edit: DisconnectNamedPipe fails with error ERROR_INVALID_HANDLE. I wonder whether that means that after pfnClosePseudoConsole is called, the hIn and hOut handles are somehow invalidated?

Edit: I ran another experiment where I moved DisconnectNamedPipe back to the PtyKill implementation, and I got the error ERROR_ACCESS_DENIED.