From 62055522fb0590298da31c16d3eb0df6919d8bf8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:55:31 -0700 Subject: [PATCH] Fix API facade memory leaks We need to register/dispose any event created in the facades, otherwise embedders need to dispose of the listeners manually. Fixes #4645 --- src/browser/public/Terminal.ts | 18 ++++++++++-------- src/common/public/BufferNamespaceApi.ts | 6 ++++-- src/headless/public/Terminal.ts | 14 ++++++++------ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 3c9b6d3acb..7a94356bd1 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, IModes, IDecorationOptions, IDecoration, IBufferElementProvider } from 'xterm'; +import { Terminal as ITerminalApi, IMarker, IDisposable, ILocalizableStrings, ITerminalAddon, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, IModes, IDecorationOptions, IDecoration, IBufferElementProvider, ITerminalInitOnlyOptions } from 'xterm'; import { IBufferRange, ITerminal } from 'browser/Types'; import { Terminal as TerminalCore } from 'browser/Terminal'; import * as Strings from 'browser/LocalizableStrings'; @@ -13,22 +13,25 @@ import { UnicodeApi } from 'common/public/UnicodeApi'; import { AddonManager } from 'common/public/AddonManager'; import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi'; import { ITerminalOptions } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; /** * The set of options that only have an effect when set in the Terminal constructor. */ const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; -export class Terminal implements ITerminalApi { +export class Terminal extends Disposable implements ITerminalApi { private _core: ITerminal; private _addonManager: AddonManager; private _parser: IParser | undefined; private _buffer: BufferNamespaceApi | undefined; private _publicOptions: Required; - constructor(options?: ITerminalOptions) { - this._core = new TerminalCore(options); - this._addonManager = new AddonManager(); + constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions) { + super(); + + this._core = this.register(new TerminalCore(options)); + this._addonManager = this.register(new AddonManager()); this._publicOptions = { ... this._core.options }; const getter = (propName: string): any => { @@ -92,7 +95,7 @@ export class Terminal implements ITerminalApi { public get cols(): number { return this._core.cols; } public get buffer(): IBufferNamespaceApi { if (!this._buffer) { - this._buffer = new BufferNamespaceApi(this._core); + this._buffer = this.register(new BufferNamespaceApi(this._core)); } return this._buffer; } @@ -189,8 +192,7 @@ export class Terminal implements ITerminalApi { this._core.selectLines(start, end); } public dispose(): void { - this._addonManager.dispose(); - this._core.dispose(); + super.dispose(); } public scrollLines(amount: number): void { this._verifyIntegers(amount); diff --git a/src/common/public/BufferNamespaceApi.ts b/src/common/public/BufferNamespaceApi.ts index 9e49ce2b4f..aeaa4ac841 100644 --- a/src/common/public/BufferNamespaceApi.ts +++ b/src/common/public/BufferNamespaceApi.ts @@ -7,15 +7,17 @@ import { IBuffer as IBufferApi, IBufferNamespace as IBufferNamespaceApi } from ' import { BufferApiView } from 'common/public/BufferApiView'; import { EventEmitter } from 'common/EventEmitter'; import { ICoreTerminal } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; -export class BufferNamespaceApi implements IBufferNamespaceApi { +export class BufferNamespaceApi extends Disposable implements IBufferNamespaceApi { private _normal: BufferApiView; private _alternate: BufferApiView; - private readonly _onBufferChange = new EventEmitter(); + private readonly _onBufferChange = this.register(new EventEmitter()); public readonly onBufferChange = this._onBufferChange.event; constructor(private _core: ICoreTerminal) { + super(); this._normal = new BufferApiView(this._core.buffers.normal, 'normal'); this._alternate = new BufferApiView(this._core.buffers.alt, 'alternate'); this._core.buffers.onBufferActivate(() => this._onBufferChange.fire(this.active)); diff --git a/src/headless/public/Terminal.ts b/src/headless/public/Terminal.ts index d4360c7bbc..5eeeb361ab 100644 --- a/src/headless/public/Terminal.ts +++ b/src/headless/public/Terminal.ts @@ -11,12 +11,13 @@ import { IBufferNamespace as IBufferNamespaceApi, IMarker, IModes, IParser, ITer import { Terminal as TerminalCore } from 'headless/Terminal'; import { AddonManager } from 'common/public/AddonManager'; import { ITerminalOptions } from 'common/Types'; +import { Disposable } from 'common/Lifecycle'; /** * The set of options that only have an effect when set in the Terminal constructor. */ const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; -export class Terminal implements ITerminalApi { +export class Terminal extends Disposable implements ITerminalApi { private _core: TerminalCore; private _addonManager: AddonManager; private _parser: IParser | undefined; @@ -24,8 +25,10 @@ export class Terminal implements ITerminalApi { private _publicOptions: Required; constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions) { - this._core = new TerminalCore(options); - this._addonManager = new AddonManager(); + super(); + + this._core = this.register(new TerminalCore(options)); + this._addonManager = this.register(new AddonManager()); this._publicOptions = { ... this._core.options }; const getter = (propName: string): any => { @@ -94,7 +97,7 @@ export class Terminal implements ITerminalApi { public get buffer(): IBufferNamespaceApi { this._checkProposedApi(); if (!this._buffer) { - this._buffer = new BufferNamespaceApi(this._core); + this._buffer = this.register(new BufferNamespaceApi(this._core)); } return this._buffer; } @@ -144,8 +147,7 @@ export class Terminal implements ITerminalApi { return this.registerMarker(cursorYOffset); } public dispose(): void { - this._addonManager.dispose(); - this._core.dispose(); + super.dispose(); } public scrollLines(amount: number): void { this._verifyIntegers(amount);