From 2e7e774f93a914e7338789b59fc81a63cc716492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 24 May 2019 02:36:13 +0200 Subject: [PATCH 01/10] custom flow control and discard limit: - adding watermark for write buffers - low and high watermark to toggle flow control - sanity watermark to discard data --- demo/server.js | 5 ++- package.json | 2 +- src/Terminal.ts | 111 ++++++++++++++++++++++++++++-------------------- yarn.lock | 17 +++++--- 4 files changed, 81 insertions(+), 54 deletions(-) diff --git a/demo/server.js b/demo/server.js index c7c9fc9b26..bb410a9d7d 100644 --- a/demo/server.js +++ b/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); diff --git a/package.json b/package.json index 043b76cba5..60b2df006b 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 8dd9f461f2..bb9de85864 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. + * 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 diff --git a/yarn.lock b/yarn.lock index 321fe6f390..0b1a2b6668 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" From d7ccd9f2a8923d1b0e5c549dbabde34d90df3971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Fri, 24 May 2019 03:26:56 +0200 Subject: [PATCH 02/10] make linter happy --- src/Terminal.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index bb9de85864..74bd8d2fb5 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -69,7 +69,7 @@ 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; @@ -77,7 +77,7 @@ const HIGH_WATERMARK = 300000; /** * Flow control PAUSE/RESUME messages. - * + * * TODO: make this configurable */ const FLOW_CONTROL_PAUSE = '\x1b^p\x1b\\'; // PM p ST From 97767f199fbae40c167fe7abf1c40246b00abf74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 26 May 2019 03:33:37 +0200 Subject: [PATCH 03/10] fining tuning watermarks --- demo/server.js | 30 +++++++++++++++++++++--------- fast_producer.c | 8 ++++++++ src/Terminal.ts | 5 +++-- 3 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 fast_producer.c diff --git a/demo/server.js b/demo/server.js index bb410a9d7d..892c83b84c 100644 --- a/demo/server.js +++ b/demo/server.js @@ -54,9 +54,9 @@ function startServer() { console.log('Created terminal with PID: ' + term.pid); terminals[term.pid] = term; logs[term.pid] = ''; - term.on('data', function(data) { - logs[term.pid] += data; - }); + //term.on('data', function(data) { + // logs[term.pid] += data; + //}); res.send(term.pid.toString()); res.end(); }); @@ -75,15 +75,20 @@ 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]); + //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 = ''; @@ -93,14 +98,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 = []; @@ -110,10 +121,11 @@ function startServer() { } }; } - const send = USE_BINARY_UTF8 ? bufferUtf8(ws, 5) : buffer(ws, 5); + const send = USE_BINARY_UTF8 ? bufferUtf8(ws, 5, 16384) : buffer(ws, 5, 16384); term.on('data', function(data) { try { + console.log(data.length); send(data); } catch (ex) { // The WebSocket is not open, ignore 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/src/Terminal.ts b/src/Terminal.ts index 74bd8d2fb5..f01331e529 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -72,8 +72,8 @@ const DISCARD_WATERMARK = 10000000; // FIXME: should this be bigger? * * TODO: make this configurable */ -const LOW_WATERMARK = 100000; -const HIGH_WATERMARK = 300000; +const LOW_WATERMARK = 32768; +const HIGH_WATERMARK = 131072; /** * Flow control PAUSE/RESUME messages. @@ -1467,6 +1467,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._watermark/1000).toFixed(2), data.length); // Ensure the terminal isn't disposed if (this._isDisposed) { return; From a4ec9b3eec0e45f99673946b4a69c2d8c6607d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 26 May 2019 03:37:51 +0200 Subject: [PATCH 04/10] make linter happy --- src/Terminal.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 439c5f3d69..a893576f6d 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -63,25 +63,21 @@ const document = (typeof window !== 'undefined') ? window.document : null; * The actual watermark is calculated as sum of * unhandled chunk sizes in both write buffers. */ -const DISCARD_WATERMARK = 10000000; // FIXME: should this be bigger? +const DISCARD_WATERMARK = 10000000; /** * Flow control watermarks for the write buffer. * low: send resume to pty * high: send pause to pty - * - * TODO: make this configurable */ const LOW_WATERMARK = 32768; const HIGH_WATERMARK = 131072; /** * 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 +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 @@ -1467,7 +1463,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._watermark/1000).toFixed(2), data.length); + console.log((this._watermark / 1000).toFixed(2), data.length); // Ensure the terminal isn't disposed if (this._isDisposed) { return; From ca3e47da975165f1134a35c5c41cf6da0b2b80b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Sun, 26 May 2019 03:43:53 +0200 Subject: [PATCH 05/10] fix PAUSE/RESUME in server.js --- demo/server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/demo/server.js b/demo/server.js index 892c83b84c..6408ee55b1 100644 --- a/demo/server.js +++ b/demo/server.js @@ -47,8 +47,8 @@ function startServer() { env: process.env, encoding: USE_BINARY_UTF8 ? null : 'utf8', handleFlowControl: true, - flowControlPause: '\x1b^p\x1b\\', - flowControlResume: '\x1b^r\x1b\\' + flowControlPause: '\x1b^pause\x1b\\', + flowControlResume: '\x1b^resume\x1b\\' }); console.log('Created terminal with PID: ' + term.pid); From 34c3a777e99899c6f81a60de0faed2e74739377b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 26 May 2019 20:48:54 -0700 Subject: [PATCH 06/10] Add useFlowControl as an option in demo --- demo/client.ts | 1 - 1 file changed, 1 deletion(-) 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' ]; From 43e2fb33cdb499cc123c784a403eb7959b86b548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 30 May 2019 15:50:50 +0200 Subject: [PATCH 07/10] comment discard watermark --- src/Terminal.ts | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index a893576f6d..4bfa64c7f9 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -59,11 +59,15 @@ import { RenderCoordinator } from './renderer/RenderCoordinator'; const document = (typeof window !== 'undefined') ? window.document : null; /** - * Safety watermark to avoid memory exhaustion. - * The actual watermark is calculated as sum of - * unhandled chunk sizes in both write buffers. + * 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 DISCARD_WATERMARK = 10000000; +const DISCARD_WATERMARK = 50000000; // ~50 MB /** * Flow control watermarks for the write buffer. @@ -201,7 +205,13 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public writeBuffer: string[]; public writeBufferUtf8: Uint8Array[]; private _writeInProgress: boolean; - private _watermark: number = 0; + + /** + * 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. @@ -1388,15 +1398,15 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // safety measure: dont allow the backend to crash // the terminal by writing to much data to fast. - if (this._watermark > DISCARD_WATERMARK) { + if (this._writeBuffersPendingSize > DISCARD_WATERMARK) { // FIXME: do something more useful console.error('write data discarded, use flow control to avoid losing data'); return; } // flow control: pause pty (like XOFF) - this._watermark += data.length; - if (this.options.useFlowControl && this._watermark > HIGH_WATERMARK) { + this._writeBuffersPendingSize += data.length; + if (this.options.useFlowControl && this._writeBuffersPendingSize > HIGH_WATERMARK) { this.handler(FLOW_CONTROL_PAUSE); this._xoffSentToCatchUp = true; } @@ -1428,7 +1438,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._refreshEnd = this.buffer.y; this._inputHandler.parseUtf8(data); - this._watermark -= data.length; + this._writeBuffersPendingSize -= data.length; this.updateRange(this.buffer.y); this.refresh(this._refreshStart, this._refreshEnd); @@ -1439,7 +1449,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } // flow control: resume pty (like XON) - if (this._xoffSentToCatchUp && this._watermark < LOW_WATERMARK) { + if (this._xoffSentToCatchUp && this._writeBuffersPendingSize < LOW_WATERMARK) { this.handler(FLOW_CONTROL_RESUME); this._xoffSentToCatchUp = false; } @@ -1463,7 +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._watermark / 1000).toFixed(2), data.length); + console.log((this._writeBuffersPendingSize / 1000).toFixed(2), data.length); // Ensure the terminal isn't disposed if (this._isDisposed) { return; @@ -1476,15 +1486,15 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // safety measure: dont allow the backend to crash // the terminal by writing to much data to fast. - if (this._watermark > DISCARD_WATERMARK) { + if (this._writeBuffersPendingSize > DISCARD_WATERMARK) { // FIXME: do something more useful console.error('write data discarded, use flow control to avoid losing data'); return; } // flow control: pause pty (like XOFF) - this._watermark += data.length; - if (this.options.useFlowControl && this._watermark > HIGH_WATERMARK) { + this._writeBuffersPendingSize += data.length; + if (this.options.useFlowControl && this._writeBuffersPendingSize > HIGH_WATERMARK) { this.handler(FLOW_CONTROL_PAUSE); this._xoffSentToCatchUp = true; } @@ -1516,7 +1526,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II this._refreshEnd = this.buffer.y; this._inputHandler.parse(data); - this._watermark -= data.length; + this._writeBuffersPendingSize -= data.length; this.updateRange(this.buffer.y); this.refresh(this._refreshStart, this._refreshEnd); @@ -1527,7 +1537,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } // flow control: resume pty (like XON) - if (this._xoffSentToCatchUp && this._watermark < LOW_WATERMARK) { + if (this._xoffSentToCatchUp && this._writeBuffersPendingSize < LOW_WATERMARK) { this.handler(FLOW_CONTROL_RESUME); this._xoffSentToCatchUp = false; } From e366e1b25340f70c442a61b2f5a7cebee02261f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 30 May 2019 15:56:23 +0200 Subject: [PATCH 08/10] remove logs from demo --- demo/server.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/demo/server.js b/demo/server.js index 6408ee55b1..74410c5c9e 100644 --- a/demo/server.js +++ b/demo/server.js @@ -14,8 +14,7 @@ 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')); @@ -53,10 +52,6 @@ function startServer() { 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(); }); @@ -75,7 +70,6 @@ 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, limit) { @@ -125,7 +119,6 @@ function startServer() { term.on('data', function(data) { try { - console.log(data.length); send(data); } catch (ex) { // The WebSocket is not open, ignore @@ -139,7 +132,6 @@ function startServer() { console.log('Closed terminal ' + term.pid); // Clean things up delete terminals[term.pid]; - delete logs[term.pid]; }); }); From adf04885c62454db723697bc5aab75e854a8b830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 30 May 2019 15:59:22 +0200 Subject: [PATCH 09/10] remove magic numbers --- demo/server.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/demo/server.js b/demo/server.js index 74410c5c9e..3e9bb81567 100644 --- a/demo/server.js +++ b/demo/server.js @@ -9,6 +9,10 @@ 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(); @@ -115,7 +119,9 @@ function startServer() { } }; } - const send = USE_BINARY_UTF8 ? bufferUtf8(ws, 5, 16384) : buffer(ws, 5, 16384); + 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 { From cdf9753b217b4625a1506805f7483a065bf641f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 30 May 2019 16:10:46 +0200 Subject: [PATCH 10/10] throw error in discard limit --- src/Terminal.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 4bfa64c7f9..ae0e3d693d 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1398,10 +1398,10 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // 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) { - // FIXME: do something more useful - console.error('write data discarded, use flow control to avoid losing data'); - return; + throw new Error('write data discarded, use flow control to avoid losing data'); } // flow control: pause pty (like XOFF) @@ -1486,10 +1486,10 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // 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) { - // FIXME: do something more useful - console.error('write data discarded, use flow control to avoid losing data'); - return; + throw new Error('write data discarded, use flow control to avoid losing data'); } // flow control: pause pty (like XOFF)