Skip to content

Conversation

@SimonSiefke
Copy link
Contributor

Fixes a memory leak in task problem monitor.

Overview

It seems a problem matcher can be added to a terminal again, e.g. when re-running a task, this function would be called again with the same terminal since terminals are reused between tasks

addTerminal(terminal: ITerminalInstance, problemMatcher: AbstractProblemCollector) {
	this.terminalMarkerMap.set(terminal.instanceId, {
		resources: new Map<string, URI>(),
		markers: new Map<string, Map<string, IMarkerData>>()
	});

	const store = new DisposableStore();
	this.terminalDisposables.set(terminal.instanceId, store); 

This line this.terminalDisposables.set(terminal.instanceId, store) registers the store for the terminal, but doesn't dispose the previous registered store if there is one.

Change

The change is to use a DisposableMap which ensures the already registered disposable gets disposed if there is one.

Before

When running a task 97 times, the number of functions grows by each time in several places, one of which is task problem monitor.

{
  "namedFunctionCount3": [
	/* some more data omitted */
  {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3019:8513",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts:75:65",
      "originalName": "TaskTerminalStatus.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3019:11261",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskProblemMonitor.ts:35:32",
      "originalName": "TaskProblemMonitor.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3021:11006",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts:1051:60",
      "originalName": "TerminalTaskSystem._executeInTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3019:11385",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskProblemMonitor.ts:41:44",
      "originalName": "TaskProblemMonitor.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3019:11732",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskProblemMonitor.ts:64:60",
      "originalName": "TaskProblemMonitor.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/workbench.desktop.main.js:3021:11503",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts:1073:39",
      "originalName": "TerminalTaskSystem._executeInTerminal"
    }
  ],
  "isLeak": true
}

After

Still various possible memory leaks are detected. But not in taskProblemMonitor anymore.

{
  "namedFunctionCount3": [
	/* some more data omitted */
  {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/tasks/browser/taskTerminalStatus.js:66:50",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts:70:48",
      "originalName": "TaskTerminalStatus.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/tasks/browser/taskTerminalStatus.js:60:54",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts:64:52",
      "originalName": "TaskTerminalStatus.addTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/tasks/browser/terminalTaskSystem.js:862:41",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts:1074:39",
      "originalName": "TerminalTaskSystem._executeInTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/tasks/browser/terminalTaskSystem.js:842:62",
      "originalLocation": "src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts:1052:60",
      "originalName": "TerminalTaskSystem._executeInTerminal"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/terminal/browser/terminalProcessManager.js:388:77",
      "originalLocation": "src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts:465:74",
      "originalName": "TerminalProcessManager._resolveEnvironment"
    },
    {
      "count": 99,
      "delta": 97,
      "name": "anonymous",
      "sourceLocation": "out/vs/workbench/contrib/terminal/browser/xterm/decorationAddon.js:321:57",
      "originalLocation": "src/vs/workbench/contrib/terminal/browser/xterm/decorationAddon.ts:362:55",
      "originalName": "DecorationAddon._createHover"
    }
  ],
  "isLeak": true
}


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants