From b75e42a56c4eed39fb5d9b0071c0938f6f8aecaf Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 3 May 2020 12:46:37 -0700 Subject: [PATCH] Fix terminal object leaking --- demo/client.ts | 23 ++++++++++++++++--- src/browser/Linkifier2.ts | 9 ++++---- src/browser/Terminal.ts | 20 ++++++++-------- src/browser/selection/Types.d.ts | 5 ++++ src/browser/services/SelectionService.test.ts | 2 +- src/browser/services/SelectionService.ts | 22 ++++++++++-------- src/browser/services/Services.ts | 5 ++-- src/common/CoreTerminal.ts | 2 +- src/common/Lifecycle.ts | 4 +++- src/common/services/CoreService.ts | 19 ++++++++++----- 10 files changed, 74 insertions(+), 37 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index eaa6f8c1..5fc8b131 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -31,6 +31,17 @@ import { Unicode11Addon } from '../addons/xterm-addon-unicode11/out/Unicode11Add // little weird here as we're importing "this" module import { Terminal as TerminalType, ITerminalOptions } from 'xterm'; +interface IDisposable { + dispose(): void; +} + +function addDisposableListener(element: HTMLElement, type: string, listener: (event: any) => void): IDisposable { + element.addEventListener(type, listener); + return { + dispose: () => element.removeEventListener(type, listener) + }; +} + export interface IWindowWithTerminal extends Window { term: TerminalType; Terminal?: typeof TerminalType; @@ -112,9 +123,15 @@ const disposeRecreateButtonHandler = () => { term = null; window.term = null; socket = null; + addons.attach.instance = undefined; + addons.fit.instance = undefined; + addons.search.instance = undefined; + addons.serialize.instance = undefined; + addons.unicode11.instance = undefined; + addons['web-links'].instance = undefined; + addons.webgl.instance = undefined; document.getElementById('dispose').innerHTML = 'Recreate Terminal'; - } - else { + } else { createTerminal(); document.getElementById('dispose').innerHTML = 'Dispose terminal'; } @@ -357,7 +374,7 @@ function initAddons(term: TerminalType): void { if (!addon.canChange) { checkbox.disabled = true; } - checkbox.addEventListener('change', () => { + addDomListener(checkbox, 'change', () => { if (checkbox.checked) { addon.instance = new addon.ctor(); term.loadAddon(addon.instance); diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier2.ts index 458b1bff..647b9882 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier2.ts @@ -8,13 +8,14 @@ import { IDisposable } from 'common/Types'; import { IMouseService, IRenderService } from './services/Services'; import { IBufferService } from 'common/services/Services'; import { EventEmitter, IEvent } from 'common/EventEmitter'; +import { Disposable } from 'common/Lifecycle'; interface ILinkState { decorations: ILinkDecorations; isHovered: boolean; } -export class Linkifier2 implements ILinkifier2 { +export class Linkifier2 extends Disposable implements ILinkifier2 { private _element: HTMLElement | undefined; private _mouseService: IMouseService | undefined; private _renderService: IRenderService | undefined; @@ -26,15 +27,15 @@ export class Linkifier2 implements ILinkifier2 { private _lastBufferCell: IBufferCellPosition | undefined; private _isMouseOut: boolean = true; - private _onShowLinkUnderline = new EventEmitter(); + private _onShowLinkUnderline = this.register(new EventEmitter()); public get onShowLinkUnderline(): IEvent { return this._onShowLinkUnderline.event; } - private _onHideLinkUnderline = new EventEmitter(); + private _onHideLinkUnderline = this.register(new EventEmitter()); public get onHideLinkUnderline(): IEvent { return this._onHideLinkUnderline.event; } constructor( @IBufferService private readonly _bufferService: IBufferService ) { - + super(); } public registerLinkProvider(linkProvider: ILinkProvider): IDisposable { diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 7f3ead61..c38e9bb4 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -140,7 +140,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this._setup(); this.linkifier = this._instantiationService.createInstance(Linkifier); - this.linkifier2 = this._instantiationService.createInstance(Linkifier2); + this.linkifier2 = this.register(this._instantiationService.createInstance(Linkifier2)); // Setup InputHandler listeners this.register(this._inputHandler.onRequestBell(() => this.bell())); @@ -154,7 +154,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this.register(forwardEvent(this._inputHandler.onA11yTab, this._onA11yTabEmitter)); // Setup listeners - this._bufferService.onResize(e => this._afterResize(e.cols, e.rows)); + this.register(this._bufferService.onResize(e => this._afterResize(e.cols, e.rows))); } public dispose(): void { @@ -413,13 +413,13 @@ export class Terminal extends CoreTerminal implements ITerminal { this._theme = this.options.theme || this._theme; this._colorManager = new ColorManager(document, this.options.allowTransparency); - this.optionsService.onOptionChange(e => this._colorManager!.onOptionsChange(e)); + this.register(this.optionsService.onOptionChange(e => this._colorManager!.onOptionsChange(e))); this._colorManager.setTheme(this._theme); const renderer = this._createRenderer(); this._renderService = this._instantiationService.createInstance(RenderService, renderer, this.rows, this.screenElement); this._instantiationService.setService(IRenderService, this._renderService); - this._renderService.onRenderedBufferChange(e => this._onRender.fire(e)); + this.register(this._renderService.onRenderedBufferChange(e => this._onRender.fire(e))); this.onResize(e => this._renderService!.resize(e.cols, e.rows)); this._soundService = this._instantiationService.createInstance(SoundService); @@ -442,13 +442,13 @@ export class Terminal extends CoreTerminal implements ITerminal { this.register(this.onFocus(() => this._renderService!.onFocus())); this.register(this._renderService.onDimensionsChange(() => this.viewport!.syncScrollArea())); - this._selectionService = this._instantiationService.createInstance(SelectionService, - (amount: number, suppressEvent: boolean) => this.scrollLines(amount, suppressEvent), + this._selectionService = this.register(this._instantiationService.createInstance(SelectionService, this.element, - this.screenElement); + this.screenElement)); this._instantiationService.setService(ISelectionService, this._selectionService); + this.register(this._selectionService.onRequestScrollLines(e => this.scrollLines(e.amount, e.suppressScrollEvent))); this.register(this._selectionService.onSelectionChange(() => this._onSelectionChange.fire())); - this.register(this._selectionService.onRedrawRequest(e => this._renderService!.onSelectionChanged(e.start, e.end, e.columnSelectMode))); + this.register(this._selectionService.onRequestRedraw(e => this._renderService!.onSelectionChanged(e.start, e.end, e.columnSelectMode))); this.register(this._selectionService.onLinuxMouseSelection(text => { // If there's a new selection, put it into the textarea, focus and select it // in order to register it as a selection on the OS. This event is fired @@ -646,7 +646,7 @@ export class Terminal extends CoreTerminal implements ITerminal { } } }; - this._coreMouseService.onProtocolChange(events => { + this.register(this._coreMouseService.onProtocolChange(events => { // apply global changes on events if (events) { if (this.optionsService.options.logLevel === 'debug') { @@ -691,7 +691,7 @@ export class Terminal extends CoreTerminal implements ITerminal { } else if (!requestedEvents.mousedrag) { requestedEvents.mousedrag = eventListeners.mousedrag; } - }); + })); // force initial onProtocolChange so we dont miss early mouse requests this._coreMouseService.activeProtocol = this._coreMouseService.activeProtocol; diff --git a/src/browser/selection/Types.d.ts b/src/browser/selection/Types.d.ts index d5a03ecc..8adfc17c 100644 --- a/src/browser/selection/Types.d.ts +++ b/src/browser/selection/Types.d.ts @@ -8,3 +8,8 @@ export interface ISelectionRedrawRequestEvent { end: [number, number] | undefined; columnSelectMode: boolean; } + +export interface ISelectionRequestScrollLinesEvent { + amount: number; + suppressScrollEvent: boolean; +} diff --git a/src/browser/services/SelectionService.test.ts b/src/browser/services/SelectionService.test.ts index cd787531..eb90ba44 100644 --- a/src/browser/services/SelectionService.test.ts +++ b/src/browser/services/SelectionService.test.ts @@ -21,7 +21,7 @@ class TestSelectionService extends SelectionService { optionsService: IOptionsService, renderService: IRenderService ) { - super(() => {}, null!, null!, bufferService, new MockCoreService(), new MockMouseService(), optionsService, renderService); + super(null!, null!, bufferService, new MockCoreService(), new MockMouseService(), optionsService, renderService); } public get model(): SelectionModel { return this._model; } diff --git a/src/browser/services/SelectionService.ts b/src/browser/services/SelectionService.ts index d70cfa11..4b8650b4 100644 --- a/src/browser/services/SelectionService.ts +++ b/src/browser/services/SelectionService.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { ISelectionRedrawRequestEvent } from 'browser/selection/Types'; +import { ISelectionRedrawRequestEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types'; import { IBuffer } from 'common/buffer/Types'; import { IBufferLine, IDisposable } from 'common/Types'; import * as Browser from 'common/Platform'; @@ -14,6 +14,7 @@ import { ICharSizeService, IMouseService, ISelectionService, IRenderService } fr import { IBufferService, IOptionsService, ICoreService } from 'common/services/Services'; import { getCoordsRelativeToElement } from 'browser/input/Mouse'; import { moveToCellSequence } from 'browser/input/MoveToCell'; +import { Disposable } from 'common/Lifecycle'; /** * The number of pixels the mouse needs to be above or below the viewport in @@ -66,7 +67,7 @@ export const enum SelectionMode { * not handled by the SelectionService but the onRedrawRequest event is fired * when the selection is ready to be redrawn (on an animation frame). */ -export class SelectionService implements ISelectionService { +export class SelectionService extends Disposable implements ISelectionService { public serviceBrand: undefined; protected _model: SelectionModel; @@ -105,15 +106,16 @@ export class SelectionService implements ISelectionService { private _mouseDownTimeStamp: number = 0; - private _onLinuxMouseSelection = new EventEmitter(); + private _onLinuxMouseSelection = this.register(new EventEmitter()); public get onLinuxMouseSelection(): IEvent { return this._onLinuxMouseSelection.event; } - private _onRedrawRequest = new EventEmitter(); - public get onRedrawRequest(): IEvent { return this._onRedrawRequest.event; } - private _onSelectionChange = new EventEmitter(); + private _onRedrawRequest = this.register(new EventEmitter()); + public get onRequestRedraw(): IEvent { return this._onRedrawRequest.event; } + private _onSelectionChange = this.register(new EventEmitter()); public get onSelectionChange(): IEvent { return this._onSelectionChange.event; } + private _onRequestScrollLines = this.register(new EventEmitter()); + public get onRequestScrollLines(): IEvent { return this._onRequestScrollLines.event; } constructor( - private readonly _scrollLines: (amount: number, suppressEvent: boolean) => void, private readonly _element: HTMLElement, private readonly _screenElement: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @@ -122,6 +124,8 @@ export class SelectionService implements ISelectionService { @IOptionsService private readonly _optionsService: IOptionsService, @IRenderService private readonly _renderService: IRenderService ) { + super(); + // Init listeners this._mouseMoveListener = event => this._onMouseMove(event); this._mouseUpListener = event => this._onMouseUp(event); @@ -131,7 +135,7 @@ export class SelectionService implements ISelectionService { } }); this._trimListener = this._bufferService.buffer.lines.onTrim(amount => this._onTrim(amount)); - this._bufferService.buffers.onBufferActivate(e => this._onBufferActivate(e)); + this.register(this._bufferService.buffers.onBufferActivate(e => this._onBufferActivate(e))); this.enable(); @@ -633,7 +637,7 @@ export class SelectionService implements ISelectionService { return; } if (this._dragScrollAmount) { - this._scrollLines(this._dragScrollAmount, false); + this._onRequestScrollLines.fire({ amount: this._dragScrollAmount, suppressScrollEvent: false }); // Re-evaluate selection // If the cursor was above or below the viewport, make sure it's at the // start or end of the viewport respectively. This should only happen when diff --git a/src/browser/services/Services.ts b/src/browser/services/Services.ts index 445cab05..1c71d387 100644 --- a/src/browser/services/Services.ts +++ b/src/browser/services/Services.ts @@ -6,7 +6,7 @@ import { IEvent } from 'common/EventEmitter'; import { IRenderDimensions, IRenderer, CharacterJoinerHandler } from 'browser/renderer/Types'; import { IColorSet } from 'browser/Types'; -import { ISelectionRedrawRequestEvent } from 'browser/selection/Types'; +import { ISelectionRedrawRequestEvent as ISelectionRequestRedrawEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; import { IDisposable } from 'common/Types'; @@ -80,7 +80,8 @@ export interface ISelectionService { readonly selectionEnd: [number, number] | undefined; readonly onLinuxMouseSelection: IEvent; - readonly onRedrawRequest: IEvent; + readonly onRequestRedraw: IEvent; + readonly onRequestScrollLines: IEvent; readonly onSelectionChange: IEvent; disable(): void; diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 3793ae25..65768c51 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -86,7 +86,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this._instantiationService.setService(IBufferService, this._bufferService); this._logService = this._instantiationService.createInstance(LogService); this._instantiationService.setService(ILogService, this._logService); - this._coreService = this._instantiationService.createInstance(CoreService, () => this.scrollToBottom()); + this._coreService = this.register(this._instantiationService.createInstance(CoreService, () => this.scrollToBottom())); this._instantiationService.setService(ICoreService, this._coreService); this._coreMouseService = this._instantiationService.createInstance(CoreMouseService); this._instantiationService.setService(ICoreMouseService, this._coreMouseService); diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts index ad1afa1d..910862c4 100644 --- a/src/common/Lifecycle.ts +++ b/src/common/Lifecycle.ts @@ -28,9 +28,11 @@ export abstract class Disposable implements IDisposable { /** * Registers a disposable object. * @param d The disposable to register. + * @returns The disposable. */ - public register(d: T): void { + public register(d: T): T { this._disposables.push(d); + return d; } /** diff --git a/src/common/services/CoreService.ts b/src/common/services/CoreService.ts index 3ea4f29c..43b89a9c 100644 --- a/src/common/services/CoreService.ts +++ b/src/common/services/CoreService.ts @@ -7,6 +7,7 @@ import { ICoreService, ILogService, IOptionsService, IBufferService } from 'comm import { EventEmitter, IEvent } from 'common/EventEmitter'; import { IDecPrivateModes, IModes } from 'common/Types'; import { clone } from 'common/Clone'; +import { Disposable } from 'common/Lifecycle'; const DEFAULT_MODES: IModes = Object.freeze({ insertMode: false @@ -22,7 +23,7 @@ const DEFAULT_DEC_PRIVATE_MODES: IDecPrivateModes = Object.freeze({ wraparound: true // defaults: xterm - true, vt100 - false }); -export class CoreService implements ICoreService { +export class CoreService extends Disposable implements ICoreService { public serviceBrand: any; public isCursorInitialized: boolean = false; @@ -30,20 +31,26 @@ export class CoreService implements ICoreService { public modes: IModes; public decPrivateModes: IDecPrivateModes; - private _onData = new EventEmitter(); + // Circular dependency, this must be unset or memory will leak after Terminal.dispose + private _scrollToBottom: (() => void) | undefined; + + private _onData = this.register(new EventEmitter()); public get onData(): IEvent { return this._onData.event; } - private _onUserInput = new EventEmitter(); + private _onUserInput = this.register(new EventEmitter()); public get onUserInput(): IEvent { return this._onUserInput.event; } - private _onBinary = new EventEmitter(); + private _onBinary = this.register(new EventEmitter()); public get onBinary(): IEvent { return this._onBinary.event; } constructor( // TODO: Move this into a service - private readonly _scrollToBottom: () => void, + scrollToBottom: () => void, @IBufferService private readonly _bufferService: IBufferService, @ILogService private readonly _logService: ILogService, @IOptionsService private readonly _optionsService: IOptionsService ) { + super(); + this._scrollToBottom = scrollToBottom; + this.register({ dispose: () => this._scrollToBottom = undefined }); this.modes = clone(DEFAULT_MODES); this.decPrivateModes = clone(DEFAULT_DEC_PRIVATE_MODES); } @@ -62,7 +69,7 @@ export class CoreService implements ICoreService { // Input is being sent to the terminal, the terminal should focus the prompt. const buffer = this._bufferService.buffer; if (buffer.ybase !== buffer.ydisp) { - this._scrollToBottom(); + this._scrollToBottom!(); } // Fire onUserInput so listeners can react as well (eg. clear selection)