Merge cdf9753 into 9643de4

xtermjs · May 30, 2019 · 955218b · 955218b
2 parents 9643de4 + cdf9753
commit 955218b
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 68 deletions.
diff --git a/demo/client.ts b/demo/client.ts
@@ -203,7 +203,6 @@ function initOptions(term: TerminalType): void {
     'handler',
     'screenKeys',
     'termName',
-    'useFlowControl',
     // Complex option
     'theme'
   ];

diff --git a/demo/server.js b/demo/server.js
@@ -9,13 +9,16 @@ var pty = require('node-pty');
  */
 const USE_BINARY_UTF8 = false;
 
+// buffering
+const MAX_SEND_INTERVAL = 5;
+const MAX_CHUNK_SIZE = 16384;
+
 
 function startServer() {
   var app = express();
   expressWs(app);
 
-  var terminals = {},
-      logs = {};
+  var terminals = {};
 
   app.use('/src', express.static(__dirname + '/../src'));
   app.get('/logo.png', (req, res) => res.sendFile(__dirname + '/logo.png'));
@@ -45,15 +48,14 @@ 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^pause\x1b\\',
+          flowControlResume: '\x1b^resume\x1b\\'
         });
 
     console.log('Created terminal with PID: ' + term.pid);
     terminals[term.pid] = term;
-    logs[term.pid] = '';
-    term.on('data', function(data) {
-      logs[term.pid] += data;
-    });
     res.send(term.pid.toString());
     res.end();
   });
@@ -72,15 +74,19 @@ function startServer() {
   app.ws('/terminals/:pid', function (ws, req) {
     var term = terminals[parseInt(req.params.pid)];
     console.log('Connected to terminal ' + term.pid);
-    ws.send(logs[term.pid]);
 
     // string message buffering
-    function buffer(socket, timeout) {
+    function buffer(socket, timeout, limit) {
       let s = '';
       let sender = null;
       return (data) => {
         s += data;
-        if (!sender) {
+        if (s.length > limit) {
+          clearTimeout(sender);
+          socket.send(s);
+          s = '';
+          sender = null;
+        } else if (!sender) {
           sender = setTimeout(() => {
             socket.send(s);
             s = '';
@@ -90,14 +96,20 @@ function startServer() {
       };
     }
     // binary message buffering
-    function bufferUtf8(socket, timeout) {
+    function bufferUtf8(socket, timeout, limit) {
       let buffer = [];
       let sender = null;
       let length = 0;
       return (data) => {
         buffer.push(data);
         length += data.length;
-        if (!sender) {
+        if (length > limit) {
+          clearTimeout(sender);
+          socket.send(Buffer.concat(buffer, length));
+          buffer = [];
+          sender = null;
+          length = 0;
+        } else if (!sender) {
           sender = setTimeout(() => {
             socket.send(Buffer.concat(buffer, length));
             buffer = [];
@@ -107,7 +119,9 @@ function startServer() {
         }
       };
     }
-    const send = USE_BINARY_UTF8 ? bufferUtf8(ws, 5) : buffer(ws, 5);
+    const send = USE_BINARY_UTF8
+      ? bufferUtf8(ws, MAX_SEND_INTERVAL, MAX_CHUNK_SIZE)
+      : buffer(ws, MAX_SEND_INTERVAL, MAX_CHUNK_SIZE);
 
     term.on('data', function(data) {
       try {
@@ -124,7 +138,6 @@ function startServer() {
       console.log('Closed terminal ' + term.pid);
       // Clean things up
       delete terminals[term.pid];
-      delete logs[term.pid];
     });
   });
 

diff --git a/fast_producer.c b/fast_producer.c
@@ -0,0 +1,8 @@
+#include <stdio.h>
+#include <unistd.h>
+
+int main(int argc, char **argv) {
+  while (1) {
+    putchar('#');
+  }
+}
diff --git a/package.json b/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",

diff --git a/src/Terminal.ts b/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 and browser engine crash on fast data input.
+ * Once hit the terminal will stop working. Enable flow control to avoid this limit
+ * and make sure that your backend correctly propagates this to the underlying pty.
+ * (see docs for further instructions)
+ * Since this limit is meant as a safety parachute to prevent browser crashs,
+ * it is set to a very high number. Typically xterm.js gets unresponsive with
+ * a much lower number (>500 kB).
  */
-const WRITE_BUFFER_PAUSE_THRESHOLD = 5;
+const DISCARD_WATERMARK = 50000000;  // ~50 MB
+
+/**
+ * Flow control watermarks for the write buffer.
+ * low: send resume to pty
+ * high: send pause to pty
+ */
+const LOW_WATERMARK = 32768;
+const HIGH_WATERMARK = 131072;
+
+/**
+ * Flow control PAUSE/RESUME messages.
+ */
+const FLOW_CONTROL_PAUSE  = '\x1b^pause\x1b\\';  // PM pause ST
+const FLOW_CONTROL_RESUME = '\x1b^resume\x1b\\'; // PM resume ST
 
 /**
  * The max number of ms to spend on writes before allowing the renderer to
@@ -188,6 +206,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
   public writeBufferUtf8: Uint8Array[];
   private _writeInProgress: boolean;
 
+  /**
+   * Sum of length of pending chunks in all write buffers.
+   * Note: For the string chunks the actual memory usage is
+   * doubled (JSString char takes 2 bytes).
+   */
+  private _writeBuffersPendingSize: number = 0;
+
   /**
    * Whether _xterm.js_ sent XOFF in order to catch up with the pty process.
    * This is a distinct state from writeStopped so that if the user requested
@@ -1371,18 +1396,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 we hit this, the terminal cant keep up with data written
+    // and will start to degenerate.
+    if (this._writeBuffersPendingSize > DISCARD_WATERMARK) {
+      throw new Error('write data discarded, use flow control to avoid losing data');
+    }
 
-    // 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._writeBuffersPendingSize += data.length;
+    if (this.options.useFlowControl && this._writeBuffersPendingSize > 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 +1434,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._writeBuffersPendingSize -= data.length;
 
       this.updateRange(this.buffer.y);
       this.refresh(this._refreshStart, this._refreshEnd);
@@ -1429,6 +1447,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
         break;
       }
     }
+
+    // flow control: resume pty (like XON)
+    if (this._xoffSentToCatchUp && this._writeBuffersPendingSize < 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
@@ -1448,6 +1473,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
    * @param data The text to write to the terminal.
    */
   public write(data: string): void {
+    console.log((this._writeBuffersPendingSize / 1000).toFixed(2), data.length);
     // Ensure the terminal isn't disposed
     if (this._isDisposed) {
       return;
@@ -1458,18 +1484,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 we hit this, the terminal cant keep up with data written
+    // and will start to degenerate.
+    if (this._writeBuffersPendingSize > DISCARD_WATERMARK) {
+      throw new Error('write data discarded, use flow control to avoid losing data');
+    }
 
-    // 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._writeBuffersPendingSize += data.length;
+    if (this.options.useFlowControl && this._writeBuffersPendingSize > 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 +1522,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._writeBuffersPendingSize -= data.length;
 
       this.updateRange(this.buffer.y);
       this.refresh(this._refreshStart, this._refreshEnd);
@@ -1516,6 +1535,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
         break;
       }
     }
+
+    // flow control: resume pty (like XON)
+    if (this._xoffSentToCatchUp && this._writeBuffersPendingSize < 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

diff --git a/yarn.lock b/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"