xtermjs / xterm.js

A terminal for the web
https://xtermjs.org/
MIT License
17.33k stars 1.61k forks source link

Memory leak in CoreBrowserService #4935

Closed SimonSiefke closed 4 months ago

SimonSiefke commented 7 months ago

Details

Steps to reproduce

  1. In VSCode, create a terminal and then kill the terminal
  2. Notice that a resize, change, blur and focus have been added in CoreBrowserService.ts but not been removed:
{
  "eventListeners": [
    {
      "type": "resize",
      "description": "()=>this._setDprAndFireIfDiffers()",
      "stack": ["listener (vscode/node_modules/@xterm/xterm/lib/xterm.js:0:104883)"],
      "count": 1,
      "originalStack": ["webpack://@xterm/xterm/src/browser/services/CoreBrowserService.ts:105:94"],
      "originalName": null
    },
    {
      "type": "change",
      "description": "()=>this._setDprAndFireIfDiffers()",
      "stack": ["listener (vscode/node_modules/@xterm/xterm/lib/xterm.js:0:104453)"],
      "count": 1,
      "originalStack": ["webpack://@xterm/xterm/src/browser/services/CoreBrowserService.ts:86:26"],
      "originalName": null
    },
    {
      "type": "blur",
      "description": "()=>this._isFocused=!1",
      "stack": ["listener (vscode/node_modules/@xterm/xterm/lib/xterm.js:0:103775)"],
      "count": 1,
      "originalStack": ["webpack://@xterm/xterm/src/browser/services/CoreBrowserService.ts:35:44"],
      "originalName": null
    },
    {
      "type": "focus",
      "description": "()=>this._isFocused=!0",
      "stack": ["listener (vscode/node_modules/@xterm/xterm/lib/xterm.js:0:103710)"],
      "count": 1,
      "originalStack": ["webpack://@xterm/xterm/src/browser/services/CoreBrowserService.ts:34:45"],
      "originalName": null
    }
  ],
  "isLeak": true
}

Test script

(Note: the test script might not work on windows unfortunately):

git clone git@github.com:SimonSiefke/vscode-memory-leak-finder.git &&
cd vscode-memory-leak-finder &&
git checkout v5.35.0 &&
npm ci &&
node packages/cli/bin/test.js --cwd packages/e2e --check-leaks --measure event-listeners --measure-after --runs 1 --only terminal.create &&
cat .vscode-memory-leak-finder-results/event-listeners/terminal.create.json

Additional information

It seems that in CoreBrowserService.ts, ScreenDprMonitor is not registered like other disposables, which could cause leaking event listeners:

export class CoreBrowserService extends Disposable implements ICoreBrowserService {
  public serviceBrand: undefined;

  private _isFocused = false;
  private _cachedIsFocused: boolean | undefined = undefined;
  private _screenDprMonitor = new ScreenDprMonitor(this._window); // not registered

The blur and focus textarea event listeners could be disposed with addDisposableDomListener:

this.register(addDisposableDomListener(this._textarea, 'focus', () => (this._isFocused = true)));
this.register(addDisposableDomListener(this._textarea, 'blur', () => (this._isFocused = false)));