Add missing disposals

[SvanT]

#155232

I did some V8 heap profiling and found a few more places where
references to closed terminals prevents them from being garbage
collected.

[ghost]

All CLA requirements met.

[SvanT]

@Tyriar I think there is at least one more retainer than these ones which is the action bar button actions (split/kill terminal) inside the terminal tabs list:

One guess is that the registration of the DOM mousedown/tap events doesn't get disposed when the terminal is closed, and more and more event registrations referencing terminals are being done in the action bar without being cleaned up when terminals are closed.

But I get a bit lost in the disposals at this time...

[Tyriar]

Thanks again for this, these PRs are great! We do a big cleanup every now and then when we notice something is off, not sure how we could test to prevent regressions 🤔

I think we could fix this by adding actions here:

vscode/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts

Lines 471 to 480 in 9ba5ee2

	const actions = [
	new Action(TerminalCommandId.SplitInstance, terminalStrings.split.short, ThemeIcon.asClassName(Codicon.splitHorizontal), true, async () => {
	this._runForSelectionOrInstance(instance, async e => {
	this._terminalService.createTerminal({ location: { parentTerminal: e } });
	});
	}),
	new Action(TerminalCommandId.KillInstance, terminalStrings.kill.short, ThemeIcon.asClassName(Codicon.trashcan), true, async () => {
	this._runForSelectionOrInstance(instance, e => this._terminalService.safeDisposeTerminal(e));
	})
	];

To template.elementDisposables?

[Tyriar]

Let me know if that template.elementDisposables idea doesn't work

[SvanT]

@Tyriar Nope, chaning from line 484 to this:

		if (!template.elementDisposables) {
			template.elementDisposables = new DisposableStore();
		}

		for (const action of actions) {
			template.actionBar.push(action, { icon: true, label: false, keybinding: this._keybindingService.lookupKeybinding(action.id)?.getLabel() });
			template.elementDisposables.add(action);
		}

Doesn't seem to change anything, the retainer still looks like the screenshot in my last comment.

[SvanT]

Removing the template.actionBar.push line seems to solve it, but there is pretty much going on in the actionbar push function

[SvanT]

@Tyriar I found it!

Action bars are not disposed when a terminal is closed, they are cached in the list until you open a new terminal, so manually clear it to make it possible to garbage collect the closed terminal directly.

addressed the feedback