Skip to content

Commit

Permalink
Fix API facade memory leaks
Browse files Browse the repository at this point in the history
We need to register/dispose any event created in the facades, otherwise
embedders need to dispose of the listeners manually.

Fixes #4645
  • Loading branch information
Tyriar committed Aug 9, 2023
1 parent 0224317 commit 6205552
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 16 deletions.
18 changes: 10 additions & 8 deletions src/browser/public/Terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<ITerminalOptions>;

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 => {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/common/public/BufferNamespaceApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IBufferApi>();
private readonly _onBufferChange = this.register(new EventEmitter<IBufferApi>());
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));
Expand Down
14 changes: 8 additions & 6 deletions src/headless/public/Terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,24 @@ 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;
private _buffer: BufferNamespaceApi | undefined;
private _publicOptions: Required<ITerminalOptions>;

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 => {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 6205552

Please sign in to comment.