From bf3709f4c5b34614b1e25348ffb21c0df0ca04a4 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 23 Jul 2018 15:29:45 -0700 Subject: [PATCH 1/4] Fix NPE in InputHandler.parse Fixes #1567 --- src/InputHandler.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 3fda521f..4def4f75 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -296,6 +296,11 @@ export class InputHandler extends Disposable implements IInputHandler { } public parse(data: string): void { + // Ensure the terminal is not disposed + if (!this._terminal) { + return; + } + let buffer = this._terminal.buffer; const cursorStartX = buffer.x; const cursorStartY = buffer.y; From 20d4c64ffe91f345740fb42cece371c2cd9c20c4 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 23 Jul 2018 15:37:04 -0700 Subject: [PATCH 2/4] Make write/_innerWrite aware of terminal disposal --- src/Terminal.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Terminal.ts b/src/Terminal.ts index e8366290..2d247080 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -193,6 +193,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Store if user went browsing history in scrollback private _userScrolling: boolean; + private _isDisposed: boolean = false; private _inputHandler: InputHandler; public soundManager: SoundManager; @@ -232,6 +233,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } public dispose(): void { + this._isDisposed = true; super.dispose(); this._customKeyEventHandler = null; removeTerminalFromCache(this); @@ -1266,6 +1268,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II * @param {string} 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; @@ -1294,6 +1301,11 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } protected _innerWrite(): void { + // Ensure the terminal isn't disposed + if (this._isDisposed) { + this.writeBuffer = []; + } + const writeBatch = this.writeBuffer.splice(0, WRITE_BATCH_SIZE); while (writeBatch.length > 0) { const data = writeBatch.shift(); From cbfb99a5cfe02a872f5c16d827cd3571b3d0fc53 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 6 Aug 2018 07:16:51 -0700 Subject: [PATCH 3/4] Move _isDisposed to Disposable class --- src/Terminal.ts | 2 -- src/common/Lifecycle.ts | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index de753431..8f7d27b3 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -193,7 +193,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // Store if user went browsing history in scrollback private _userScrolling: boolean; - private _isDisposed: boolean = false; private _inputHandler: InputHandler; public soundManager: SoundManager; @@ -234,7 +233,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II } public dispose(): void { - this._isDisposed = true; super.dispose(); this._customKeyEventHandler = null; removeTerminalFromCache(this); diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts index 46828521..209a3e2a 100644 --- a/src/common/Lifecycle.ts +++ b/src/common/Lifecycle.ts @@ -11,6 +11,7 @@ import { IDisposable } from 'xterm'; */ export abstract class Disposable implements IDisposable { protected _disposables: IDisposable[] = []; + protected _isDisposed: boolean = false; constructor() { } @@ -19,6 +20,7 @@ export abstract class Disposable implements IDisposable { * Disposes the object, triggering the `dispose` method on all registered IDisposables. */ public dispose(): void { + this._isDisposed = true; this._disposables.forEach(d => d.dispose()); this._disposables.length = 0; } From 6a770296f9c8c1c2b5f2893ed7ff241dbaa57fdf Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 6 Aug 2018 07:22:32 -0700 Subject: [PATCH 4/4] Add tests for Dispoable --- src/common/Lifecycle.test.ts | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/common/Lifecycle.test.ts diff --git a/src/common/Lifecycle.test.ts b/src/common/Lifecycle.test.ts new file mode 100644 index 00000000..4b696fa5 --- /dev/null +++ b/src/common/Lifecycle.test.ts @@ -0,0 +1,45 @@ +/** + * Copyright (c) 2018 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { Disposable } from './Lifecycle'; + +class TestDisposable extends Disposable { + public get isDisposed(): boolean { + return this._isDisposed; + } +} + +describe('Disposable', () => { + describe('register', () => { + it('should register disposables', () => { + const d = new TestDisposable(); + const d2 = { + dispose: () => { throw new Error(); } + }; + d.register(d2); + assert.throws(() => d.dispose()); + }); + }); + describe('unregister', () => { + it('should unregister disposables', () => { + const d = new TestDisposable(); + const d2 = { + dispose: () => { throw new Error(); } + }; + d.register(d2); + d.unregister(d2); + assert.doesNotThrow(() => d.dispose()); + }); + }); + describe('dispose', () => { + it('should set is disposed flag', () => { + const d = new TestDisposable(); + assert.isFalse(d.isDisposed); + d.dispose(); + assert.isTrue(d.isDisposed); + }); + }); +});