Handle writes to non-blocking pty instead of wrapping via Node's tty api

src/unix/pty.cc

@@ -448,10 +448,10 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) {
   }
 #endif
 
-  Napi::Object obj = Napi::Object::New(napiEnv);  
-  obj.Set("fd", Napi::Number::New(napiEnv, master));  
-  obj.Set("pid", Napi::Number::New(napiEnv, pid));  
-  obj.Set("pty", Napi::String::New(napiEnv, ptsname(master)));  
+  Napi::Object obj = Napi::Object::New(napiEnv);
+  obj.Set("fd", Napi::Number::New(napiEnv, master));
+  obj.Set("pid", Napi::Number::New(napiEnv, pid));
+  obj.Set("pty", Napi::String::New(napiEnv, ptsname(master)));
 
   // Set up process exit callback.
   Napi::Function cb = info[10].As<Napi::Function>();
@@ -492,10 +492,10 @@ Napi::Value PtyOpen(const Napi::CallbackInfo& info) {
     throw Napi::Error::New(env, "Could not set slave fd to nonblocking.");
   }
 
-  Napi::Object obj = Napi::Object::New(env);  
-  obj.Set("master", Napi::Number::New(env, master));  
-  obj.Set("slave", Napi::Number::New(env, slave));  
-  obj.Set("pty", Napi::String::New(env, ptsname(master)));  
+  Napi::Object obj = Napi::Object::New(env);
+  obj.Set("master", Napi::Number::New(env, master));
+  obj.Set("slave", Napi::Number::New(env, slave));
+  obj.Set("pty", Napi::String::New(env, ptsname(master)));
 
   return obj;
 }

src/unixTerminal.ts

@@ -3,6 +3,7 @@
  * Copyright (c) 2016, Daniel Imms (MIT License).
  * Copyright (c) 2018, Microsoft Corporation (MIT License).
  */
+import * as fs from 'fs';
 import * as net from 'net';
 import * as path from 'path';
 import * as tty from 'tty';
@@ -34,6 +35,11 @@ export class UnixTerminal extends Terminal {
 
   private _boundClose: boolean = false;
   private _emittedClose: boolean = false;
+
+  private readonly _writeQueue: (string | Buffer)[] = [];
+  private _writeInProgress: boolean = false;
+  private _writeTimeout: NodeJS.Timeout | undefined;
+
   private _master: net.Socket | undefined;
   private _slave: net.Socket | undefined;
 
@@ -162,7 +168,69 @@ export class UnixTerminal extends Terminal {
   }
 
   protected _write(data: string | Buffer): void {
-    this._socket.write(data);
+    // Writes are put in a queue and processed asynchronously in order to handle
+    // backpressure from the kernel buffer.
+    this._writeQueue.push(data);
+    if (this._writeInProgress) {
+      return;
+    }
+    this._writeInProgress = true;
+    this._processWriteQueue();
+  }
+
+  private async _processWriteQueue(): Promise<void> {
+    const data = this._writeQueue.shift();
+    if (!data) {
+      this._writeInProgress = false;
+      return;
+    }
+
+    // Write to the underlying file descriptor and handle it directly, rather
+    // than using the `net.Socket`/`tty.WriteStream` wrappers which swallow the
+    // errors and cause the thread to block indefinitely.
+    fs.write(this._fd, data, (err, written) => {
+      if (err) {
+        const errno = (err as any).errno;
+        switch (errno) {
+          case -35: // EAGAIN (macOS)
+          case -11: // EAGAIN (Linux)
+            // This error appears to get swallowed and translated into
+            // `ERR_SYSTEM_ERROR` when using tty.WriteStream and not fs.write
+            // directly.
+            // This can happen during a regular partial write, but the most
+            // reliable way to test this is to run `sleep 10` in the shell and
+            // paste enough data to fill the kernel-level buffer. Once the sleep
+            // ends, the pty should accept the data again. Re-process after a
+            // short break.
+            this._writeTimeout = setTimeout(() => this._processWriteQueue(), 5);
+            return;
+          case -5:  // EIO
+          case -32: // EPIPE
+            // Stop processing writes immediately as the pty is closed.
+            this._writeInProgress = false;
+            return;
+          default:
+            console.error('Unhandled pty write error', errno, err);
+            // Fall through as it's important to finish processing the queue
+            break;
+        }
+      }
+
+      // Requeue any partial writes
+      if (written < data.length) {
+        this._writeQueue.unshift(data.slice(written));
+      }
+
+      // Using `setImmediate` here appears to corrupt the data, this may be what
+      // the interleaving/dropped data comment is about in Node.js' tty module:
+      // https://github.com/nodejs/node/blob/4cac2b94bed4bf02810be054e8f63c0048c66564/lib/tty.js#L106C1-L111C34
+      //
+      // Yielding via `setImmediate` also doesn't seem to drain the buffer much
+      // anyway, so use a short timeout when this happens instead. Note that the `drain`
+      // event does not appear to happen on `net.Socket`/`tty.WriteStream` when
+      // writing to ptys.
+      this._writeTimeout = setTimeout(() => this._processWriteQueue(), 5);
+    });
   }
 
   /* Accessors */
@@ -238,6 +306,8 @@ export class UnixTerminal extends Terminal {
     });
 
     this._socket.destroy();
+
+    clearTimeout(this._writeTimeout);
   }
 
   public kill(signal?: string): void {