From 75ba717bd7519056156df26dbdbe2be25490cf1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 12 Sep 2019 13:36:58 +0200 Subject: [PATCH] merge write methods, optional callback --- src/InputHandler.ts | 26 +--- src/Terminal.test.ts | 2 +- src/Terminal.ts | 205 +++++++-------------------- src/TestUtils.test.ts | 2 +- src/Types.d.ts | 9 +- src/public/Terminal.ts | 14 +- test/benchmark/Terminal.benchmark.ts | 6 +- typings/xterm.d.ts | 24 ++-- 8 files changed, 85 insertions(+), 203 deletions(-) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 56b2d8be..f3ed0454 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -324,7 +324,7 @@ export class InputHandler extends Disposable implements IInputHandler { super.dispose(); } - public parse(data: string): void { + public parse(data: string | Uint8Array): void { let buffer = this._bufferService.buffer; const cursorStartX = buffer.x; const cursorStartY = buffer.y; @@ -334,25 +334,11 @@ export class InputHandler extends Disposable implements IInputHandler { if (this._parseBuffer.length < data.length) { this._parseBuffer = new Uint32Array(data.length); } - this._parser.parse(this._parseBuffer, this._stringDecoder.decode(data, this._parseBuffer)); - - buffer = this._bufferService.buffer; - if (buffer.x !== cursorStartX || buffer.y !== cursorStartY) { - this._onCursorMove.fire(); - } - } - - public parseUtf8(data: Uint8Array): void { - let buffer = this._bufferService.buffer; - const cursorStartX = buffer.x; - const cursorStartY = buffer.y; - - this._logService.debug('parsing data', data); - - if (this._parseBuffer.length < data.length) { - this._parseBuffer = new Uint32Array(data.length); - } - this._parser.parse(this._parseBuffer, this._utf8Decoder.decode(data, this._parseBuffer)); + this._parser.parse(this._parseBuffer, + (typeof data === 'string') + ? this._stringDecoder.decode(data, this._parseBuffer) + : this._utf8Decoder.decode(data, this._parseBuffer) + ); buffer = this._bufferService.buffer; if (buffer.x !== cursorStartX || buffer.y !== cursorStartY) { diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index 59864f0a..9cf10018 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -31,7 +31,7 @@ describe('Terminal', () => { (term)._compositionHelper = new MockCompositionHelper(); // Force synchronous writes term.write = (data) => { - term.writeBuffer.push(data); + (term)._writeBuffer.push(data); (term)._innerWrite(); }; (term).element = { diff --git a/src/Terminal.ts b/src/Terminal.ts index d793abfc..8b26069c 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -67,11 +67,14 @@ import { CoreMouseService } from 'common/services/CoreMouseService'; 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. + * 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 100 times lower number (>500 kB). */ -const WRITE_BUFFER_PAUSE_THRESHOLD = 5; +const DISCARD_WATERMARK = 50000000; // ~50 MB /** * The max number of ms to spend on writes before allowing the renderer to @@ -80,8 +83,15 @@ const WRITE_BUFFER_PAUSE_THRESHOLD = 5; * depends on the time it takes for the renderer to draw the frame. */ const WRITE_TIMEOUT_MS = 12; + +/** + * Threshold of max held chunks in the write buffer, that were already processed. + * This is a tradeoff between extensive write buffer shifts (bad runtime) and high + * memory consumption by data thats not used anymore. + */ const WRITE_BUFFER_LENGTH_THRESHOLD = 50; + export class Terminal extends Disposable implements ITerminal, IDisposable, IInputHandlingTerminal { public textarea: HTMLTextAreaElement; public element: HTMLElement; @@ -153,21 +163,11 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp public params: (string | number)[]; public currentParam: string | number; - // user input states - public writeBuffer: string[]; - public writeBufferUtf8: Uint8Array[]; - private _writeInProgress: boolean; - - /** - * 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 - * XOFF via ^S that it will not automatically resume when the writeBuffer goes - * below threshold. - */ - private _xoffSentToCatchUp: boolean; - - /** Whether writing has been stopped as a result of XOFF */ - // private _writeStopped: boolean; + // write data related containers + protected _writeBuffer: (Uint8Array | string)[] = []; + private _pendingWriteDataSize: number = 0; + private _writeChunkCallbacks: ((() => void) | undefined)[] = []; + private _writeInProgress = false; // Store if user went browsing history in scrollback private _userScrolling: boolean; @@ -306,13 +306,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp this.params = []; this.currentParam = 0; - // user input states - this.writeBuffer = []; - this.writeBufferUtf8 = []; - this._writeInProgress = false; - - this._xoffSentToCatchUp = false; - // this._writeStopped = false; this._userScrolling = false; // Register input handler and refire/handle events @@ -1144,139 +1137,37 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp } } - /** - * Writes raw utf8 bytes to the terminal. - * @param data UintArray with UTF8 bytes to write to the terminal. - */ - public writeUtf8(data: Uint8Array): void { + public write(data: string | Uint8Array, callback?: () => void): void { // Ensure the terminal isn't disposed - if (this._isDisposed) { + // NOOP on empty data + if (this._isDisposed || !data.length) { return; } - // Ignore falsy data values - if (!data) { - return; + if (this._pendingWriteDataSize > DISCARD_WATERMARK) { + throw new Error('write data discarded, use flow control to avoid losing data'); } - this.writeBufferUtf8.push(data); + this._pendingWriteDataSize += data.length; + this._writeBuffer.push(data); + this._writeChunkCallbacks.push(callback); - // 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._coreService.triggerDataEvent(C0.DC3); - this._xoffSentToCatchUp = true; - } - - if (!this._writeInProgress && this.writeBufferUtf8.length > 0) { - // Kick off a write which will write all data in sequence recursively + if (!this._writeInProgress) { this._writeInProgress = true; - // Kick off an async innerWrite so more writes can come in while processing data - setTimeout(() => { - this._innerWriteUtf8(); - }); - } - } - - protected _innerWriteUtf8(bufferOffset: number = 0): void { - // Ensure the terminal isn't disposed - if (this._isDisposed) { - this.writeBufferUtf8 = []; - } - - const startTime = Date.now(); - while (this.writeBufferUtf8.length > bufferOffset) { - 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._coreService.triggerDataEvent(C0.DC1); - this._xoffSentToCatchUp = false; - } - - this._inputHandler.parseUtf8(data); - - this.refresh(this._dirtyRowService.start, this._dirtyRowService.end); - - if (Date.now() - startTime >= WRITE_TIMEOUT_MS) { - break; - } - } - if (this.writeBufferUtf8.length > bufferOffset) { - // Allow renderer to catch up before processing the next batch - // trim already processed chunks if we are above threshold - if (bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) { - this.writeBufferUtf8 = this.writeBufferUtf8.slice(bufferOffset); - bufferOffset = 0; - } - setTimeout(() => this._innerWriteUtf8(bufferOffset), 0); - } else { - this._writeInProgress = false; - this.writeBufferUtf8 = []; - } - } - - /** - * Writes text to the terminal. - * @param data The text to write to the terminal. - */ - public write(data: string): void { - // Ensure the terminal isn't disposed - if (this._isDisposed) { - return; - } - - // Ignore falsy data values (including the empty string) - if (!data) { - return; - } - - this.writeBuffer.push(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._coreService.triggerDataEvent(C0.DC3); - this._xoffSentToCatchUp = true; - } - - if (!this._writeInProgress && this.writeBuffer.length > 0) { - // Kick off a write which will write all data in sequence recursively - this._writeInProgress = true; - // Kick off an async innerWrite so more writes can come in while processing data - setTimeout(() => { - this._innerWrite(); - }); + setTimeout(() => this._innerWrite()); } } protected _innerWrite(bufferOffset: number = 0): void { - // Ensure the terminal isn't disposed - if (this._isDisposed) { - this.writeBuffer = []; - } - const startTime = Date.now(); - while (this.writeBuffer.length > bufferOffset) { - const data = this.writeBuffer[bufferOffset]; + while (this._writeBuffer.length > bufferOffset) { + const data = this._writeBuffer[bufferOffset]; + const cb = this._writeChunkCallbacks[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._coreService.triggerDataEvent(C0.DC1); - this._xoffSentToCatchUp = false; - } - this._inputHandler.parse(data); + this._pendingWriteDataSize -= data.length; + if (cb) cb(); this.refresh(this._dirtyRowService.start, this._dirtyRowService.end); @@ -1284,26 +1175,36 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp break; } } - if (this.writeBuffer.length > bufferOffset) { + if (this._writeBuffer.length > bufferOffset) { // Allow renderer to catch up before processing the next batch // trim already processed chunks if we are above threshold if (bufferOffset > WRITE_BUFFER_LENGTH_THRESHOLD) { - this.writeBuffer = this.writeBuffer.slice(bufferOffset); + this._writeBuffer = this._writeBuffer.slice(bufferOffset); + this._writeChunkCallbacks = this._writeChunkCallbacks.slice(bufferOffset); bufferOffset = 0; } setTimeout(() => this._innerWrite(bufferOffset), 0); } else { this._writeInProgress = false; - this.writeBuffer = []; + this._writeBuffer = []; + this._writeChunkCallbacks = []; } } + /** + * @deprecated use write instead + */ + public writeUtf8(data: Uint8Array, callback?: () => void): void { + this.write(data, callback); + } + /** * Writes text to the terminal, followed by a break line character (\n). * @param data The text to write to the terminal. */ - public writeln(data: string): void { - this.write(data + '\r\n'); + public writeln(data: string | Uint8Array, callback?: () => void): void { + this.write(data); + this.write('\r\n', callback); } public paste(data: string): void { @@ -1743,10 +1644,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp const customKeyEventHandler = this._customKeyEventHandler; const inputHandler = this._inputHandler; const cursorState = this.cursorState; - const writeBuffer = this.writeBuffer; - const writeBufferUtf8 = this.writeBufferUtf8; + const writeBuffer = this._writeBuffer; const writeInProgress = this._writeInProgress; - const xoffSentToCatchUp = this._xoffSentToCatchUp; const userScrolling = this._userScrolling; this._setup(); @@ -1761,10 +1660,8 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp this._customKeyEventHandler = customKeyEventHandler; this._inputHandler = inputHandler; this.cursorState = cursorState; - this.writeBuffer = writeBuffer; - this.writeBufferUtf8 = writeBufferUtf8; + this._writeBuffer = writeBuffer; this._writeInProgress = writeInProgress; - this._xoffSentToCatchUp = xoffSentToCatchUp; this._userScrolling = userScrolling; // do a full screen refresh diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index ad19f721..a30f60ce 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -20,7 +20,7 @@ import { ISelectionService } from 'browser/services/Services'; export class TestTerminal extends Terminal { writeSync(data: string): void { - this.writeBuffer.push(data); + this._writeBuffer.push(data); this._innerWrite(); } keyDown(ev: any): boolean { return this._keyDown(ev); } diff --git a/src/Types.d.ts b/src/Types.d.ts index 13c65a4d..026efe52 100644 --- a/src/Types.d.ts +++ b/src/Types.d.ts @@ -73,8 +73,7 @@ export interface ICompositionHelper { * Calls the parser and handles actions generated by the parser. */ export interface IInputHandler { - parse(data: string): void; - parseUtf8(data: Uint8Array): void; + parse(data: string | Uint8Array): void; print(data: Uint32Array, start: number, end: number): void; /** C0 BEL */ bell(): void; @@ -151,7 +150,6 @@ export interface IInputHandler { export interface ITerminal extends IPublicTerminal, IElementAccessor, IBufferAccessor, ILinkifierAccessor { screenElement: HTMLElement; browser: IBrowser; - writeBuffer: string[]; cursorHidden: boolean; cursorState: number; buffer: IBuffer; @@ -192,7 +190,6 @@ export interface IPublicTerminal extends IDisposable { blur(): void; focus(): void; resize(columns: number, rows: number): void; - writeln(data: string): void; open(parent: HTMLElement): void; attachCustomKeyEventHandler(customKeyEventHandler: (event: KeyboardEvent) => boolean): void; addCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean): IDisposable; @@ -218,8 +215,8 @@ export interface IPublicTerminal extends IDisposable { scrollToBottom(): void; scrollToLine(line: number): void; clear(): void; - write(data: string): void; - writeUtf8(data: Uint8Array): void; + write(data: string | Uint8Array, callback?: () => void): void; + writeln(data: string | Uint8Array, callback?: () => void): void; paste(data: string): void; refresh(start: number, end: number): void; reset(): void; diff --git a/src/public/Terminal.ts b/src/public/Terminal.ts index 6d6cedca..eb45623d 100644 --- a/src/public/Terminal.ts +++ b/src/public/Terminal.ts @@ -55,9 +55,6 @@ export class Terminal implements ITerminalApi { this._verifyIntegers(columns, rows); this._core.resize(columns, rows); } - public writeln(data: string): void { - this._core.writeln(data); - } public open(parent: HTMLElement): void { this._core.open(parent); } @@ -128,11 +125,14 @@ export class Terminal implements ITerminalApi { public clear(): void { this._core.clear(); } - public write(data: string): void { - this._core.write(data); + public write(data: string | Uint8Array, callback?: () => void): void { + this._core.write(data, callback); } - public writeUtf8(data: Uint8Array): void { - this._core.writeUtf8(data); + public writeUtf8(data: Uint8Array, callback?: () => void): void { + this._core.write(data, callback); + } + public writeln(data: string | Uint8Array, callback?: () => void): void { + this._core.writeln(data, callback); } public paste(data: string): void { this._core.paste(data); diff --git a/test/benchmark/Terminal.benchmark.ts b/test/benchmark/Terminal.benchmark.ts index a0b8fd29..676aa891 100644 --- a/test/benchmark/Terminal.benchmark.ts +++ b/test/benchmark/Terminal.benchmark.ts @@ -11,12 +11,12 @@ import { Terminal } from 'Terminal'; class TestTerminal extends Terminal { writeSync(data: string): void { - this.writeBuffer.push(data); + this._writeBuffer.push(data); this._innerWrite(); } writeSyncUtf8(data: Uint8Array): void { - this.writeBufferUtf8.push(data); - this._innerWriteUtf8(); + this._writeBuffer.push(data); + this._innerWrite(); } } diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index d0a33cba..8e279f8c 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -650,24 +650,26 @@ declare module 'xterm' { clear(): void; /** - * Writes text to the terminal. - * @param data The text to write to the terminal. + * Write data to the terminal. + * `data` can either be raw bytes given as Uint8Array from the pty or a string. + * Raw bytes will always be treated as UTF-8 encoded, string data as UTF-16. + * `callback` is an optional callback that gets called once the data + * chunk was processed by the parser. */ - write(data: string): void; + write(data: string | Uint8Array, callback?: () => void): void; /** - * Writes text to the terminal, followed by a break line character (\n). - * @param data The text to write to the terminal. + * Writes data to the terminal, followed by a break line character (\n). + * `callback` is an optional callback that gets called once the data + * chunk was processed by the parser. */ - writeln(data: string): void; + writeln(data: string | Uint8Array, callback?: () => void): void; /** - * Writes UTF8 data to the terminal. This has a slight performance advantage - * over the string based write method due to lesser data conversions needed - * on the way from the pty to xterm.js. - * @param data The data to write to the terminal. + * Write UTF8 data to the terminal. Deprecated, use `.write` instead. + * @deprecated */ - writeUtf8(data: Uint8Array): void; + writeUtf8(data: Uint8Array, callback?: () => void): void; /** * Writes text to the terminal, performing the necessary transformations for pasted text.