Merge pull request #2899 from Tyriar/memoryleak

Fix listener leaks causing Terminal memory from not getting freed

demo/client.ts

@@ -112,9 +112,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 +363,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);

src/browser/Linkifier2.ts

@@ -8,13 +8,15 @@ 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, getDisposeArrayDisposable } from 'common/Lifecycle';
+import { addDisposableDomListener } from 'browser/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 +28,16 @@ export class Linkifier2 implements ILinkifier2 {
   private _lastBufferCell: IBufferCellPosition | undefined;
   private _isMouseOut: boolean = true;
 
-  private _onShowLinkUnderline = new EventEmitter<ILinkifierEvent>();
+  private _onShowLinkUnderline = this.register(new EventEmitter<ILinkifierEvent>());
   public get onShowLinkUnderline(): IEvent<ILinkifierEvent> { return this._onShowLinkUnderline.event; }
-  private _onHideLinkUnderline = new EventEmitter<ILinkifierEvent>();
+  private _onHideLinkUnderline = this.register(new EventEmitter<ILinkifierEvent>());
   public get onHideLinkUnderline(): IEvent<ILinkifierEvent> { return this._onHideLinkUnderline.event; }
 
   constructor(
     @IBufferService private readonly _bufferService: IBufferService
   ) {
-
+    super();
+    this.register(getDisposeArrayDisposable(this._linkCacheDisposables));
   }
 
   public registerLinkProvider(linkProvider: ILinkProvider): IDisposable {
@@ -56,12 +59,12 @@ export class Linkifier2 implements ILinkifier2 {
     this._mouseService = mouseService;
     this._renderService = renderService;
 
-    this._element.addEventListener('mouseleave', () => {
+    this.register(addDisposableDomListener(this._element, 'mouseleave', () => {
       this._isMouseOut = true;
       this._clearCurrentLink();
-    });
-    this._element.addEventListener('mousemove', this._onMouseMove.bind(this));
-    this._element.addEventListener('click', this._onClick.bind(this));
+    }));
+    this.register(addDisposableDomListener(this._element, 'mousemove', this._onMouseMove.bind(this)));
+    this.register(addDisposableDomListener(this._element, 'click', this._onClick.bind(this)));
   }
 
   private _onMouseMove(event: MouseEvent): void {

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._renderService = this.register(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;
 

src/browser/renderer/BaseRenderLayer.ts

@@ -60,7 +60,7 @@ export abstract class BaseRenderLayer implements IRenderLayer {
   }
 
   public dispose(): void {
-    this._container.removeChild(this._canvas);
+    this._canvas.parentElement?.removeChild(this._canvas);
     this._charAtlas?.dispose();
   }
 

src/browser/renderer/Renderer.ts

@@ -71,8 +71,8 @@ export class Renderer extends Disposable implements IRenderer {
   }
 
   public dispose(): void {
-    super.dispose();
     this._renderLayers.forEach(l => l.dispose());
+    super.dispose();
     removeTerminalFromCache(this._id);
   }
 

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;
+}

src/browser/services/RenderService.ts

@@ -52,7 +52,7 @@ export class RenderService extends Disposable implements IRenderService {
     screenElement: HTMLElement,
     @IOptionsService optionsService: IOptionsService,
     @ICharSizeService charSizeService: ICharSizeService,
-    @IBufferService private readonly _bufferService: IBufferService
+    @IBufferService bufferService: IBufferService
   ) {
     super();
     this._renderDebouncer = new RenderDebouncer((start, end) => this._renderRows(start, end));
@@ -62,7 +62,7 @@ export class RenderService extends Disposable implements IRenderService {
     this._screenDprMonitor.setListener(() => this.onDevicePixelRatioChange());
     this.register(this._screenDprMonitor);
 
-    this.register(this._bufferService.onResize(e => this._fullRefresh()));
+    this.register(bufferService.onResize(e => this._fullRefresh()));
     this.register(optionsService.onOptionChange(() => this._renderer.onOptionsChanged()));
     this.register(charSizeService.onCharSizeChange(() => this.onCharSizeChanged()));
 

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; }

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<string>();
+  private _onLinuxMouseSelection = this.register(new EventEmitter<string>());
   public get onLinuxMouseSelection(): IEvent<string> { return this._onLinuxMouseSelection.event; }
-  private _onRedrawRequest = new EventEmitter<ISelectionRedrawRequestEvent>();
-  public get onRedrawRequest(): IEvent<ISelectionRedrawRequestEvent> { return this._onRedrawRequest.event; }
-  private _onSelectionChange = new EventEmitter<void>();
+  private _onRedrawRequest = this.register(new EventEmitter<ISelectionRedrawRequestEvent>());
+  public get onRequestRedraw(): IEvent<ISelectionRedrawRequestEvent> { return this._onRedrawRequest.event; }
+  private _onSelectionChange = this.register(new EventEmitter<void>());
   public get onSelectionChange(): IEvent<void> { return this._onSelectionChange.event; }
+  private _onRequestScrollLines = this.register(new EventEmitter<ISelectionRequestScrollLinesEvent>());
+  public get onRequestScrollLines(): IEvent<ISelectionRequestScrollLinesEvent> { 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(<MouseEvent>event);
     this._mouseUpListener = event => this._onMouseUp(<MouseEvent>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

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<string>;
-  readonly onRedrawRequest: IEvent<ISelectionRedrawRequestEvent>;
+  readonly onRequestRedraw: IEvent<ISelectionRequestRedrawEvent>;
+  readonly onRequestScrollLines: IEvent<ISelectionRequestScrollLinesEvent>;
   readonly onSelectionChange: IEvent<void>;
 
   disable(): void;

src/common/CoreTerminal.ts

@@ -82,11 +82,11 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     this._instantiationService = new InstantiationService();
     this.optionsService = new OptionsService(options);
     this._instantiationService.setService(IOptionsService, this.optionsService);
-    this._bufferService = this._instantiationService.createInstance(BufferService);
+    this._bufferService = this.register(this._instantiationService.createInstance(BufferService));
     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);

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<T extends IDisposable>(d: T): void {
+  public register<T extends IDisposable>(d: T): T {
     this._disposables.push(d);
+    return d;
   }
 
   /**
@@ -45,3 +47,18 @@ export abstract class Disposable implements IDisposable {
     }
   }
 }
+
+/**
+ * Dispose of all disposables in an array and set its length to 0.
+ */
+export function disposeArray(disposables: IDisposable[]): void {
+  disposables.forEach(d => d.dispose());
+  disposables.length = 0;
+}
+
+/**
+ * Creates a disposable that will dispose of an array of disposables when disposed.
+ */
+export function getDisposeArrayDisposable(array: IDisposable[]): IDisposable {
+  return { dispose: () => disposeArray(array) };
+}

src/common/buffer/BufferSet.ts

@@ -8,18 +8,19 @@ import { IAttributeData } from 'common/Types';
 import { Buffer } from 'common/buffer/Buffer';
 import { EventEmitter, IEvent } from 'common/EventEmitter';
 import { IOptionsService, IBufferService } from 'common/services/Services';
+import { Disposable } from 'common/Lifecycle';
 
 /**
  * The BufferSet represents the set of two buffers used by xterm terminals (normal and alt) and
  * provides also utilities for working with them.
  */
-export class BufferSet implements IBufferSet {
+export class BufferSet extends Disposable implements IBufferSet {
   private _normal: Buffer;
   private _alt: Buffer;
   private _activeBuffer: Buffer;
 
 
-  private _onBufferActivate = new EventEmitter<{activeBuffer: IBuffer, inactiveBuffer: IBuffer}>();
+  private _onBufferActivate = this.register(new EventEmitter<{activeBuffer: IBuffer, inactiveBuffer: IBuffer}>());
   public get onBufferActivate(): IEvent<{activeBuffer: IBuffer, inactiveBuffer: IBuffer}> { return this._onBufferActivate.event; }
 
   /**
@@ -30,6 +31,8 @@ export class BufferSet implements IBufferSet {
     optionsService: IOptionsService,
     bufferService: IBufferService
   ) {
+    super();
+
     this._normal = new Buffer(true, optionsService, bufferService);
     this._normal.fillViewportRows();
 

src/common/buffer/Types.d.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IAttributeData, ICircularList, IBufferLine, ICellData, IMarker, ICharset } from 'common/Types';
+import { IAttributeData, ICircularList, IBufferLine, ICellData, IMarker, ICharset, IDisposable } from 'common/Types';
 import { IEvent } from 'common/EventEmitter';
 
 // BufferIndex denotes a position in the buffer: [rowIndex, colIndex]
@@ -47,7 +47,7 @@ export interface IBuffer {
   addMarker(y: number): IMarker;
 }
 
-export interface IBufferSet {
+export interface IBufferSet extends IDisposable {
   alt: IBuffer;
   normal: IBuffer;
   active: IBuffer;

src/common/services/BufferService.ts

@@ -7,11 +7,12 @@ import { IBufferService, IOptionsService } from 'common/services/Services';
 import { BufferSet } from 'common/buffer/BufferSet';
 import { IBufferSet, IBuffer } from 'common/buffer/Types';
 import { EventEmitter, IEvent } from 'common/EventEmitter';
+import { Disposable } from 'common/Lifecycle';
 
 export const MINIMUM_COLS = 2; // Less than 2 can mess with wide chars
 export const MINIMUM_ROWS = 1;
 
-export class BufferService implements IBufferService {
+export class BufferService extends Disposable implements IBufferService {
   public serviceBrand: any;
 
   public cols: number;
@@ -28,11 +29,17 @@ export class BufferService implements IBufferService {
   constructor(
     @IOptionsService private _optionsService: IOptionsService
   ) {
+    super();
     this.cols = Math.max(_optionsService.options.cols, MINIMUM_COLS);
     this.rows = Math.max(_optionsService.options.rows, MINIMUM_ROWS);
     this.buffers = new BufferSet(_optionsService, this);
   }
 
+  public dispose(): void {
+    super.dispose();
+    this.buffers.dispose();
+  }
+
   public resize(cols: number, rows: number): void {
     this.cols = cols;
     this.rows = rows;
@@ -42,6 +49,7 @@ export class BufferService implements IBufferService {
   }
 
   public reset(): void {
+    this.buffers.dispose();
     this.buffers = new BufferSet(this._optionsService, this);
     this.isUserScrolling = false;
   }

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<string>();
+  // Circular dependency, this must be unset or memory will leak after Terminal.dispose
+  private _scrollToBottom: (() => void) | undefined;
+
+  private _onData = this.register(new EventEmitter<string>());
   public get onData(): IEvent<string> { return this._onData.event; }
-  private _onUserInput = new EventEmitter<void>();
+  private _onUserInput = this.register(new EventEmitter<void>());
   public get onUserInput(): IEvent<void> { return this._onUserInput.event; }
-  private _onBinary = new EventEmitter<string>();
+  private _onBinary = this.register(new EventEmitter<string>());
   public get onBinary(): IEvent<string> { 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)