Merge pull request #1372 from Tyriar/1341_memory_leak

Remove terminal references on dispose

Author: Tyriar

src/EventEmitter.ts

@@ -6,7 +6,7 @@
 import { XtermListener } from './Types';
 import { IEventEmitter, IDisposable } from 'xterm';
 
-export class EventEmitter implements IEventEmitter {
+export class EventEmitter implements IEventEmitter, IDisposable {
   private _events: {[type: string]: XtermListener[]};
 
   constructor() {
@@ -75,7 +75,7 @@ export class EventEmitter implements IEventEmitter {
     return this._events[type] || [];
   }
 
-  protected destroy(): void {
+  public dispose(): void {
     this._events = {};
   }
 }

src/Terminal.ts

@@ -38,6 +38,7 @@ import { Linkifier } from './Linkifier';
 import { SelectionManager } from './SelectionManager';
 import { CharMeasure } from './utils/CharMeasure';
 import * as Browser from './shared/utils/Browser';
+import * as Dom from './utils/Dom';
 import * as Strings from './Strings';
 import { MouseHelper } from './utils/MouseHelper';
 import { clone } from './utils/Clone';
@@ -46,7 +47,8 @@ import { DEFAULT_ANSI_COLORS } from './renderer/ColorManager';
 import { MouseZoneManager } from './input/MouseZoneManager';
 import { AccessibilityManager } from './AccessibilityManager';
 import { ScreenDprMonitor } from './utils/ScreenDprMonitor';
-import { ITheme, ILocalizableStrings, IMarker } from 'xterm';
+import { ITheme, ILocalizableStrings, IMarker, IDisposable } from 'xterm';
+import { removeTerminalFromCache } from './renderer/atlas/CharAtlas';
 
 // reg + shift key mappings for digits and special chars
 const KEYCODE_KEY_MAPPINGS = {
@@ -122,11 +124,13 @@ const DEFAULT_OPTIONS: ITerminalOptions = {
   rightClickSelectsWord: Browser.isMac
 };
 
-export class Terminal extends EventEmitter implements ITerminal, IInputHandlingTerminal {
+export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal {
   public textarea: HTMLTextAreaElement;
   public element: HTMLElement;
   public screenElement: HTMLElement;
 
+  private _disposables: IDisposable[];
+
   /**
    * The HTMLElement that the terminal is created in, set by Terminal.open.
    */
@@ -248,7 +252,28 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     this._setup();
   }
 
+  public dispose(): void {
+    super.dispose();
+    this._disposables.forEach(d => d.dispose());
+    this._disposables.length = 0;
+    removeTerminalFromCache(this);
+    this.handler = () => {};
+    this.write = () => {};
+    if (this.element && this.element.parentNode) {
+      this.element.parentNode.removeChild(this.element);
+    }
+  }
+
+  /**
+   * @deprecated Use dispose instead.
+   */
+  public destroy(): void {
+    this.dispose();
+  }
+
   private _setup(): void {
+    this._disposables = [];
+
     Object.keys(DEFAULT_OPTIONS).forEach((key) => {
       if (this.options[key] == null) {
         this.options[key] = DEFAULT_OPTIONS[key];
@@ -690,7 +715,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     this.on('dprchange', () => this.renderer.onWindowResize(window.devicePixelRatio));
     // dprchange should handle this case, we need this as well for browsers that don't support the
     // matchMedia query.
-    window.addEventListener('resize', () => this.renderer.onWindowResize(window.devicePixelRatio));
+    this._disposables.push(Dom.addDisposableListener(window, 'resize', () => this.renderer.onWindowResize(window.devicePixelRatio)));
     this.charMeasure.on('charsizechanged', () => this.renderer.onResize(this.cols, this.rows));
     this.renderer.on('resize', (dimensions) => this.viewport.syncScrollArea());
 
@@ -1083,19 +1108,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
     });
   }
 
-  /**
-   * Destroys the terminal.
-   */
-  public destroy(): void {
-    super.destroy();
-    this.handler = () => {};
-    this.write = () => {};
-    if (this.element && this.element.parentNode) {
-      this.element.parentNode.removeChild(this.element);
-    }
-    // this.emit('close');
-  }
-
   /**
    * Tells the renderer to refresh terminal content between two rows (inclusive) at the next
    * opportunity.

src/renderer/atlas/CharAtlas.ts

@@ -26,6 +26,8 @@ let charAtlasCache: ICharAtlasCacheEntry[] = [];
 export function acquireCharAtlas(terminal: ITerminal, colors: IColorSet, scaledCharWidth: number, scaledCharHeight: number): HTMLCanvasElement | Promise<ImageBitmap> {
   const newConfig = generateConfig(scaledCharWidth, scaledCharHeight, terminal, colors);
 
+  // TODO: Currently if a terminal changes configs it will not free the entry reference (until it's disposed)
+
   // Check to see if the terminal already owns this config
   for (let i = 0; i < charAtlasCache.length; i++) {
     const entry = charAtlasCache[i];
@@ -70,3 +72,23 @@ export function acquireCharAtlas(terminal: ITerminal, colors: IColorSet, scaledC
   charAtlasCache.push(newEntry);
   return newEntry.bitmap;
 }
+
+/**
+ * Removes a terminal reference from the cache, allowing its memory to be freed.
+ * @param terminal The terminal to remove.
+ */
+export function removeTerminalFromCache(terminal: ITerminal): void {
+  for (let i = 0; i < charAtlasCache.length; i++) {
+    const index = charAtlasCache[i].ownedBy.indexOf(terminal);
+    if (index !== -1) {
+      if (charAtlasCache[i].ownedBy.length === 1) {
+        // Remove the cache entry if it's the only terminal
+        charAtlasCache.splice(i, 1);
+      } else {
+        // Remove the reference from the cache entry
+        charAtlasCache[i].ownedBy.splice(index, 1);
+      }
+      break;
+    }
+  }
+}

src/utils/TestUtils.test.ts

@@ -63,6 +63,9 @@ export class MockTerminal implements ITerminal {
   selectAll(): void {
     throw new Error('Method not implemented.');
   }
+  dispose(): void {
+    throw new Error('Method not implemented.');
+  }
   destroy(): void {
     throw new Error('Method not implemented.');
   }

typings/xterm.d.ts

@@ -251,7 +251,7 @@ declare module 'xterm' {
   /**
    * The class that represents an xterm.js terminal.
    */
-  export class Terminal implements IEventEmitter {
+  export class Terminal implements IEventEmitter, IDisposable {
     /**
      * The element containing the terminal.
      */
@@ -451,8 +451,16 @@ declare module 'xterm' {
      */
     selectLines(start: number, end: number): void;
 
+    /*
+     * Disposes of the terminal, detaching it from the DOM and removing any
+     * active listeners.
+     */
+    dispose(): void;
+
     /**
      * Destroys the terminal and detaches it from the DOM.
+     *
+     * @deprecated Use dispose() instead.
      */
     destroy(): void;