Skip to content

Conversation

@SimonSiefke
Copy link
Contributor

Fixes a memory leak in the chat session tracker.

Overview


Untitled

The chat session tracker registers a listener to a group each time a group is added. But when a group is removed, the listener doesn't seem to be disposed.

// Listen for new groups
this._register(this.editorGroupsService.onDidAddGroup(group => {
  this.registerGroupListeners(group);
}));

Change

The change is to also listen for group removal events and dispose the group change listener for the group when the group is removed:

// Listen to all editor groups
this.editorGroupsService.groups.forEach(group => {
	this.groupDisposables.set(group.id, this.registerGroupListeners(group));
});

// Listen for new groups
this._register(this.editorGroupsService.onDidAddGroup(group => {
	this.groupDisposables.set(group.id, this.registerGroupListeners(group));
}));

// listen for groups to be removed
this.editorGroupsService.onDidRemoveGroup(group => {
	this.groupDisposables.deleteAndDispose(group.id); // remove listener
});

Results after change

After the change there seem to be no more chat session tracker leaks.

editor split-down

alexr00
alexr00 previously approved these changes Oct 15, 2025
@vs-code-engineering vs-code-engineering bot added this to the October 2025 milestone Oct 15, 2025
Tyriar
Tyriar previously approved these changes Oct 15, 2025
osortega
osortega previously approved these changes Oct 15, 2025
@SimonSiefke SimonSiefke dismissed stale reviews from osortega, Tyriar, and alexr00 via 15518f5 October 16, 2025 06:36
@alexr00 alexr00 requested a review from osortega October 16, 2025 16:12
Tyriar
Tyriar previously approved these changes Oct 16, 2025
alexr00
alexr00 previously approved these changes Oct 16, 2025
@bpasero
Copy link
Member

bpasero commented Oct 25, 2025

Merge conflict.

alexr00
alexr00 previously approved these changes Oct 27, 2025
@vs-code-engineering
Copy link

vs-code-engineering bot commented Nov 1, 2025

📬 CODENOTIFY

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

@bpasero

Matched files:

  • src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionTracker.ts

@alexr00 alexr00 enabled auto-merge (squash) November 3, 2025 09:24
@alexr00 alexr00 merged commit 402271f into microsoft:main Nov 6, 2025
17 checks passed
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