Skip to content

comments: fix memory leak when recycling tree items in comment panel#304666

Merged
alexr00 merged 5 commits intomicrosoft:mainfrom
xingsy97:wt/comments-dispose-leak
Mar 30, 2026
Merged

comments: fix memory leak when recycling tree items in comment panel#304666
alexr00 merged 5 commits intomicrosoft:mainfrom
xingsy97:wt/comments-dispose-leak

Conversation

@xingsy97
Copy link
Copy Markdown
Member

Problem

CommentNodeRenderer.renderElement() pushes new disposables (DisposableStore, rendered markdown element, hover) into templateData.disposables on every call. When the tree recycles a template for a different element, these disposables accumulate because there is no disposeElement method to clean them up between reuses. Only disposeTemplate (called on template destruction) disposes them.

Fix

Add disposeElement() that disposes and clears the per-element disposables array when a template is recycled, matching the IListRenderer lifecycle contract.

Copilot AI review requested due to automatic review settings March 25, 2026 06:54
@vs-code-engineering vs-code-engineering bot added this to the 1.114.0 milestone Mar 25, 2026
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

This PR addresses disposable leaks in the Comments view tree by adding disposeElement() to CommentNodeRenderer, aiming to clean up per-element resources when a tree template is recycled.

Changes:

  • Implement disposeElement() in CommentNodeRenderer to dispose and clear template-associated disposables between element reuses.

Comment thread src/vs/workbench/contrib/comments/browser/commentsTreeViewer.ts Outdated
@xingsy97 xingsy97 changed the title comments: fix disposable accumulation on tree template reuse comments: fix memory leak when recycling tree items in comment panel Mar 25, 2026
@xingsy97 xingsy97 marked this pull request as ready for review March 25, 2026 07:19
@alexr00 alexr00 removed this from the 1.114.0 milestone Mar 30, 2026
@alexr00 alexr00 enabled auto-merge (squash) March 30, 2026 14:43
@alexr00 alexr00 added this to the 1.115.0 milestone Mar 30, 2026
@alexr00 alexr00 merged commit d919f29 into microsoft:main Mar 30, 2026
18 checks passed
@xingsy97 xingsy97 deleted the wt/comments-dispose-leak branch March 31, 2026 06:19
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.

4 participants