custom flow control and discard limit

demo/server.js

@@ -45,7 +45,10 @@ function startServer() {
           rows: rows || 24,
           cwd: process.env.PWD,
           env: process.env,
-          encoding: USE_BINARY_UTF8 ? null : 'utf8'
+          encoding: USE_BINARY_UTF8 ? null : 'utf8',
+          handleFlowControl: true,
+          flowControlPause: '\x1b^p\x1b\\',
+          flowControlResume: '\x1b^r\x1b\\'
         });
 
     console.log('Created terminal with PID: ' + term.pid);

package.json

@@ -31,7 +31,7 @@
     "gulp-util": "3.0.8",
     "jsdom": "^11.11.0",
     "merge-stream": "^1.0.1",
-    "node-pty": "0.7.6",
+    "node-pty": "0.9.0-beta10",
     "nodemon": "1.10.2",
     "nyc": "^11.8.0",
     "puppeteer": "^1.15.0",

src/Terminal.ts

@@ -59,11 +59,29 @@ import { RenderCoordinator } from './renderer/RenderCoordinator';
 const document = (typeof window !== 'undefined') ? window.document : null;
 
 /**
- * The amount of write requests to queue before sending an XOFF signal to the
- * pty process. This number must be small in order for ^C and similar sequences
- * to be responsive.
+ * Safety watermark to avoid memory exhaustion.
+ * The actual watermark is calculated as sum of
+ * unhandled chunk sizes in both write buffers.
  */
-const WRITE_BUFFER_PAUSE_THRESHOLD = 5;
+const DISCARD_WATERMARK = 10000000; // FIXME: should this be bigger?
+
+/**
+ * Flow control watermarks for the write buffer.
+ * low: send resume to pty
+ * high: send pause to pty
+ *
+ * TODO: make this configurable
+ */
+const LOW_WATERMARK = 100000;
+const HIGH_WATERMARK = 300000;
+
+/**
+ * Flow control PAUSE/RESUME messages.
+ *
+ * TODO: make this configurable
+ */
+const FLOW_CONTROL_PAUSE  = '\x1b^p\x1b\\'; // PM p ST
+const FLOW_CONTROL_RESUME = '\x1b^r\x1b\\'; // PM r ST
 
 /**
  * The max number of ms to spend on writes before allowing the renderer to
@@ -187,6 +205,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
   public writeBuffer: string[];
   public writeBufferUtf8: Uint8Array[];
   private _writeInProgress: boolean;
+  private _watermark: number = 0;
 
   /**
    * Whether _xterm.js_ sent XOFF in order to catch up with the pty process.
@@ -1371,18 +1390,23 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
       return;
     }
 
-    this.writeBufferUtf8.push(data);
+    // safety measure: dont allow the backend to crash
+    // the terminal by writing to much data to fast.
+    if (this._watermark > DISCARD_WATERMARK) {
+      // FIXME: do something more useful
+      console.error('write data discarded, use flow control to avoid losing data');
+      return;
+    }
 
-    // Send XOFF to pause the pty process if the write buffer becomes too large so
-    // xterm.js can catch up before more data is sent. This is necessary in order
-    // to keep signals such as ^C responsive.
-    if (this.options.useFlowControl && !this._xoffSentToCatchUp && this.writeBufferUtf8.length >= WRITE_BUFFER_PAUSE_THRESHOLD) {
-      // XOFF - stop pty pipe
-      // XON will be triggered by emulator before processing data chunk
-      this.handler(C0.DC3);
+    // flow control: pause pty (like XOFF)
+    this._watermark += data.length;
+    if (this.options.useFlowControl && this._watermark > HIGH_WATERMARK) {
+      this.handler(FLOW_CONTROL_PAUSE);
       this._xoffSentToCatchUp = true;
     }
 
+    this.writeBufferUtf8.push(data);
+
     if (!this._writeInProgress && this.writeBufferUtf8.length > 0) {
       // Kick off a write which will write all data in sequence recursively
       this._writeInProgress = true;
@@ -1404,23 +1428,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
       const data = this.writeBufferUtf8[bufferOffset];
       bufferOffset++;
 
-      // If XOFF was sent in order to catch up with the pty process, resume it if
-      // we reached the end of the writeBuffer to allow more data to come in.
-      if (this._xoffSentToCatchUp && this.writeBufferUtf8.length === bufferOffset) {
-        this.handler(C0.DC1);
-        this._xoffSentToCatchUp = false;
-      }
-
       this._refreshStart = this.buffer.y;
       this._refreshEnd = this.buffer.y;
 
-      // HACK: Set the parser state based on it's state at the time of return.
-      // This works around the bug #662 which saw the parser state reset in the
-      // middle of parsing escape sequence in two chunks. For some reason the
-      // state of the parser resets to 0 after exiting parser.parse. This change
-      // just sets the state back based on the correct return statement.
-
       this._inputHandler.parseUtf8(data);
+      this._watermark -= data.length;
 
       this.updateRange(this.buffer.y);
       this.refresh(this._refreshStart, this._refreshEnd);
@@ -1429,6 +1441,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
         break;
       }
     }
+
+    // flow control: resume pty (like XON)
+    if (this._xoffSentToCatchUp && this._watermark < LOW_WATERMARK) {
+      this.handler(FLOW_CONTROL_RESUME);
+      this._xoffSentToCatchUp = false;
+    }
+
     if (this.writeBufferUtf8.length > bufferOffset) {
       // Allow renderer to catch up before processing the next batch
       // trim already processed chunks if we are above threshold
@@ -1458,18 +1477,23 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
       return;
     }
 
-    this.writeBuffer.push(data);
+    // safety measure: dont allow the backend to crash
+    // the terminal by writing to much data to fast.
+    if (this._watermark > DISCARD_WATERMARK) {
+      // FIXME: do something more useful
+      console.error('write data discarded, use flow control to avoid losing data');
+      return;
+    }
 
-    // Send XOFF to pause the pty process if the write buffer becomes too large so
-    // xterm.js can catch up before more data is sent. This is necessary in order
-    // to keep signals such as ^C responsive.
-    if (this.options.useFlowControl && !this._xoffSentToCatchUp && this.writeBuffer.length >= WRITE_BUFFER_PAUSE_THRESHOLD) {
-      // XOFF - stop pty pipe
-      // XON will be triggered by emulator before processing data chunk
-      this.handler(C0.DC3);
+    // flow control: pause pty (like XOFF)
+    this._watermark += data.length;
+    if (this.options.useFlowControl && this._watermark > HIGH_WATERMARK) {
+      this.handler(FLOW_CONTROL_PAUSE);
       this._xoffSentToCatchUp = true;
     }
 
+    this.writeBuffer.push(data);
+
     if (!this._writeInProgress && this.writeBuffer.length > 0) {
       // Kick off a write which will write all data in sequence recursively
       this._writeInProgress = true;
@@ -1491,23 +1515,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
       const data = this.writeBuffer[bufferOffset];
       bufferOffset++;
 
-      // If XOFF was sent in order to catch up with the pty process, resume it if
-      // we reached the end of the writeBuffer to allow more data to come in.
-      if (this._xoffSentToCatchUp && this.writeBuffer.length === bufferOffset) {
-        this.handler(C0.DC1);
-        this._xoffSentToCatchUp = false;
-      }
-
       this._refreshStart = this.buffer.y;
       this._refreshEnd = this.buffer.y;
 
-      // HACK: Set the parser state based on it's state at the time of return.
-      // This works around the bug #662 which saw the parser state reset in the
-      // middle of parsing escape sequence in two chunks. For some reason the
-      // state of the parser resets to 0 after exiting parser.parse. This change
-      // just sets the state back based on the correct return statement.
-
       this._inputHandler.parse(data);
+      this._watermark -= data.length;
 
       this.updateRange(this.buffer.y);
       this.refresh(this._refreshStart, this._refreshEnd);
@@ -1516,6 +1528,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
         break;
       }
     }
+
+    // flow control: resume pty (like XON)
+    if (this._xoffSentToCatchUp && this._watermark < LOW_WATERMARK) {
+      this.handler(FLOW_CONTROL_RESUME);
+      this._xoffSentToCatchUp = false;
+    }
+
     if (this.writeBuffer.length > bufferOffset) {
       // Allow renderer to catch up before processing the next batch
       // trim already processed chunks if we are above threshold

yarn.lock

@@ -4484,7 +4484,12 @@ mute-stream@0.0.7:
   resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-0.0.7.tgz#3075ce93bc21b8fab43e1bc4da7e8115ed1e7bab"
   integrity sha1-MHXOk7whuPq0PhvE2n6BFe0ee6s=
 
-nan@2.10.0, nan@^2.9.2:
+nan@^2.13.2:
+  version "2.14.0"
+  resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.0.tgz#7818f722027b2459a86f0295d434d1fc2336c52c"
+  integrity sha512-INOFj37C7k3AfaNTtX8RhsTw7qRy7eLET14cROi9+5HAVbbHuIWUHEauBv5qT4Av2tWasiTY1Jw6puUNqRJXQg==
+
+nan@^2.9.2:
   version "2.10.0"
   resolved "https://registry.yarnpkg.com/nan/-/nan-2.10.0.tgz#96d0cd610ebd58d4b4de9cc0c6828cda99c7548f"
   integrity sha512-bAdJv7fBLhWC+/Bls0Oza+mvTaNQtP+1RyhhhvD95pgUJz6XM5IzgmxOkItJ9tkoCiplvAnXI1tNmmUD/eScyA==
@@ -4592,12 +4597,12 @@ node-pre-gyp@^0.10.0:
     semver "^5.3.0"
     tar "^4"
 
-node-pty@0.7.6:
-  version "0.7.6"
-  resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-0.7.6.tgz#bff6148c9c5836ca7e73c7aaaec067dcbdac2f7b"
-  integrity sha512-ECzKUB7KkAFZ0cjyjMXp5WLJ+7YIZ1xnNmiiegOI6WdDaKABUNV5NbB1Dw9MXD4KrZipWII0wQ7RGZ6StU/7jA==
+node-pty@0.9.0-beta10:
+  version "0.9.0-beta10"
+  resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-0.9.0-beta10.tgz#058850d6971b04fefaa5ffe00a8816cd892f1419"
+  integrity sha512-I+wvK1FCiaAkIhlW7zA7V2FkJSX2JjOto5R9DXLGQGWMIXo+n2f0vXu7YLMbGaR5eR6NIm6KP0UhsFKKCn/bwg==
   dependencies:
-    nan "2.10.0"
+    nan "^2.13.2"
 
 nodemon@1.10.2:
   version "1.10.2"