Skip to content

fix: memory leak in storageMainService#317841

Open
SimonSiefke wants to merge 3 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-storage-main-service-2
Open

fix: memory leak in storageMainService#317841
SimonSiefke wants to merge 3 commits into
microsoft:mainfrom
SimonSiefke:fix/memory-leak-storage-main-service-2

Conversation

@SimonSiefke
Copy link
Copy Markdown
Contributor

Fixes a memory leak in storageMainService.

Details

In storageMainService, when a window is loaded, the profile storage and workspace storage for that window are intialized:

this._register(this.lifecycleMainService.onWillLoadWindow(e => {

	// Profile Storage: Warmup when related window with profile loads
	if (e.window.profile) {
		this.profileStorage(e.window.profile).init();
	}

	// Workspace Storage: Warmup when related window with workspace loads
	if (e.workspace) {
		this.workspaceStorage(e.workspace).init();
	}
}

But it seems the profile storage and workspace storage aren't being closed their window closes.

Changes

The change ensures when a window is closed, that its profile storage and workspace storage are closed as well:

this._register(this.lifecycleMainService.onWillLoadWindow(e => {

	// Profile Storage: Warmup when related window with profile loads
	if (e.window.profile) {
		this.profileStorage(e.window.profile).init();
	}

	// Workspace Storage: Warmup when related window with workspace loads
	if (e.workspace) {
		this.workspaceStorage(e.workspace).init();
	}


	Event.once(Event.any(e.window.onDidClose, e.window.onDidDestroy))(async () => {
		if (this.shutdownReason) {
			return;
		}
		if (e.window.profile && this.mapProfileToStorage.has(e.window.profile.id)) {
			await this.profileStorage(e.window.profile).close();
		}
		const actualWorkspace = e.workspace ?? e.window.openedWorkspace ?? toWorkspaceIdentifier(e.window.backupPath, e.window.isExtensionDevelopmentHost);
		if (actualWorkspace && this.mapWorkspaceToStorage.has(actualWorkspace.id)) {
			await this.workspaceStorage(actualWorkspace).close();
		}
	});
}));

Also this code

workspaceStorage = this._register(this.createWorkspaceStorage(workspace));
this.mapWorkspaceToStorage.set(workspace.id, workspaceStorage);

is changed to

workspaceStorage = this.createWorkspaceStorage(workspace);
this.mapWorkspaceToStorage.set(workspace.id, workspaceStorage);

because adding the storage disposables via this._register would only remove them when the storageMainService shuts down. instead of when the window closes.

Before

When opening and closing a new window 17 times, the number of storageMainService functions seems to grow each time:
window-open-new 0

After

When opening and closing a new window 17 times, the number of storageMainService function stays constant:

window-open-new

Copilot AI review requested due to automatic review settings May 21, 2026 20:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the Electron main-process storage lifecycle to proactively close profile/workspace storages when a window closes, and adjusts how storages are instantiated and tracked.

Changes:

  • Add window close/destroy hook to close associated profile/workspace storages.
  • Introduce toWorkspaceIdentifier usage to derive a workspace identifier for closing.
  • Stop registering created profile/workspace storage instances with _register().

Comment thread src/vs/platform/storage/electron-main/storageMainService.ts
Comment thread src/vs/platform/storage/electron-main/storageMainService.ts
Comment thread src/vs/platform/storage/electron-main/storageMainService.ts
Comment thread src/vs/platform/storage/electron-main/storageMainService.ts
Comment thread src/vs/platform/storage/electron-main/storageMainService.ts Outdated
SimonSiefke and others added 2 commits May 21, 2026 22:25
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants