Merge pull request #2899 from Tyriar/memoryleak

Fix listener leaks causing Terminal memory from not getting freed

Author: Tyriar

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;