Crash on latest beta

[Nokel81]

Environment details

• OS: windows
• OS version: 21H1 (19043.1348)
• node-pty version: 0.11.0-beta11

Issue description

I was trying to debug an issue where if I kill a pty (that spawns powershell) within my electron app's main process while there is still input on (namely the last character sent was not \r or no characters were sent) then the whole application would freeze.

In doing so I tried to run the following code:

const pty = require("node-pty");

const shell = pty.spawn(process.env.PTYSHELL, [], {
    rows: 30,
    cols: 80,
    cwd: "C:\\",
    env: process.env,
    name: "xterm-256color",
});

shell.onExit((e) => {
    console.log(e); 
});

console.log("writing");
shell.write("a");
console.log("killing");
shell.kill();

console.log("killed");

and the output that I get is:

writing
killing
killed

events.js:377
      throw er; // Unhandled 'error' event
      ^
Error: read EPIPE
    at Pipe.onStreamRead (internal/stream_base_commons.js:209:20)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:578:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:403:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26) {
  errno: -4047,
  code: 'EPIPE',
  syscall: 'read'
}

On beta10 I get the following:

writing
killing
killed
{ exitCode: 0, signal: undefined }

but then it hangs and does not exit.

This might be related to #471 but I thought that was supposed to be fixed in beta9

[Tyriar]

This sounds a lot like https://github.com/microsoft/node-pty/issues/375, can you verify the worker (_conoutSocketWorker) is getting spawned? conpty can hang the process if it's not drained in a worker

[Tyriar]

Also, I'd recommend spawning the pty on a different process, the electron main process is special and should be as free as possible.

[Nokel81]

I got that crash running a normal node process node bug.js though, so I don't think that this is just about electron.

I will try to verify that it is being spawned

[Nokel81]

So the _conoutSocketWorker.onReady() handler is being called.

[Nokel81]

So I can also confirm that I am not seeing this bug if I set useConpty: false

[Tyriar]

This hang wasn't specific to Electron, but not connecting to conpty in its own thread as a deadlock could occur when input happens during exit. Still a little puzzled here as I thought it was fixed, and unfortunately build retention in ADO doesn't let me get the exact commit beta11 was based on.

[Nokel81]

If it is necessary to spawn the connections on separate threads, should that be documented?

[Tyriar]

It's meant to be handled for you, I don't think there is a fallback. I guess you're just hitting some other problem that I've never seen 🤷

[LabhanshAgrawal]

@Tyriar I'm facing the same issue in Hyper, the first bad commit seems to be https://github.com/microsoft/node-pty/commit/ba424e16d615ae6bab43a67c0979702548cce3bb

[Tyriar]

@rzhao271 FYI