fix: memory leak in history service

[SimonSiefke]

Fixes a memory leak in historyService by removing the editor group willMoveEditor event listener when the group is removed.

Before

Extra functions/closures are created by EditorNavigationStack.registerGroupListeners:

After

No more extra functions/closures are created by EditorNavigationStack.registerGroupListeners:

Details

In historyService.ts, a willMoveEditor listener is added to a group. However when the group becomes closed / disposed, the listener doesn't seem to be removed.

private registerGroupListeners(groupId: GroupIdentifier): void {
	if (!this.mapGroupToDisposable.has(groupId)) {
		const group = this.editorGroupService.getGroup(groupId);
		if (group) {
			this.mapGroupToDisposable.set(groupId, group.onWillMoveEditor(e => this.onWillMoveEditor(e)));
		}
	}
}

Additional Details

In the historyService code there is a actually group remove remove function, which should dispose the move event listener. The code looks correct to me and I'm not sure why it seems to be not working.

An ideal solution would probably be to get the existing remove code to work instead of registering another group remove listener. I tried, but couldn't figure out the exact issue why it's not working. Maybe someone can figure it out :)

// historyService.ts
remove(arg1: EditorInput | FileChangesEvent | FileOperationEvent | GroupIdentifier): void {
	/* ... */

	// Clear group listener
	if (typeof arg1 === 'number') {
		this.mapGroupToDisposable.get(arg1)?.dispose();
		this.mapGroupToDisposable.delete(arg1);
	}

	// Event
	this._onDidChange.fire();
}

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@bpasero

Matched files:

• src/vs/workbench/services/history/browser/historyService.ts

[bpasero]

@SimonSiefke thanks, any reason you did not adopt it for the other maps? See my change in b7b3920

[SimonSiefke]

Thanks, yours seems like is a good change to me! I've had something similar in SimonSiefke#44 but didn't want the diff to get too messy. :)