PtyKill called twice causing assertion failure

[debben]

I'm not sure if this bug belongs in blessed or in pty.js but it affects ttystudio. I'm writing this here because my fix was specific to getting ttystudio working on Windows. I dug through the issues of all three projects while trying to setup ttystudio but there was no record of this problem.

I'd used ttystudio before on my personal machine(Linux) but had difficulty getting it to install in my Windows workstation at work. Once I had gotten passed issues with getting the pty.js dependency to build (I had to add the --msvs_version= argument for my visual studio when installing via npm), I ran into an issue in the pty.cc native code.

I set the SHELL environment variable to point to C:\Windows\System32\cmd.exe and was able to get ttystudio somewhat working. Once I'd hit ctrl+q though, I'd get a dialog about an assertion failure in pty.cc in the PtyKill method:

static NAN_METHOD(PtyKill) {
  Nan::HandleScope scope;

  if (info.Length() != 1
    || !info[0]->IsNumber()) // pid
  {
    return Nan::ThrowError("Usage: pty.kill(pid)");
  }

  int handle = info[0]->Int32Value();

  winpty_t *pc = get_pipe_handle(handle);

  assert(pc != nullptr);
  winpty_exit(pc);
  assert(true == remove_pipe_handle(handle));

  return info.GetReturnValue().SetUndefined();
}

I eventually clicked debug on the node.exe process in task manager and it was able to pull up the pty.cc source in Visual Studio to set a breakpoint inside the PtyKill method. I found that when I'd end the recording session with ctrl+q, the PtyKill method would get called once successfully, but then would be called again a second time. By the time the method was invoked again, the pipe handle had already been removed by the call to remove_pipe_handle at the end of the first PtyKill invocation. I eventually found where PtyKill was being called twice in one of the source files of the blessed project (lib/widgets/terminal.js):

Terminal.prototype.kill = function() {
  if (this.pty) {
    this.pty.destroy();
    this.pty.kill();
  }
  this.term.refresh = function() {};
  this.term.write('\x1b[H\x1b[J');
  if (this.term._blink) {
    clearInterval(this.term._blink);
  }
  this.term.destroy();
};

The call to destroy() and kill() on the pty object were redundant, as destroy() just contains a call to kill(). I commented out the call to pty.kill() and left the pty.destroy() method in place. This allowed ttystudio to work successfully on my Windows 7 pc with cmd.exe as the shell.