diff --git a/demo/client.ts b/demo/client.ts index 8bcdc9718d..e07d68d0b4 100644 --- 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 index c7c9fc9b26..3e9bb81567 100644 --- 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 new file mode 100644 index 0000000000..50f3760742 --- /dev/null +++ b/fast_producer.c @@ -0,0 +1,8 @@ +#include +#include + +int main(int argc, char **argv) { + while (1) { + putchar('#'); + } +} diff --git a/package.json b/package.json index c3f21ae7f3..34f1ce838c 100644 --- 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 index ff28ab02fc..ae0e3d693d 100644 --- 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 index 3962373c55..7a863a073e 100644 --- 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"