Skip to content

fix: dispose createModelReference ref on stale element in disassemblyView#305995

Open
Copilot wants to merge 2 commits intomainfrom
copilot/fix-model-reference-leak
Open

fix: dispose createModelReference ref on stale element in disassemblyView#305995
Copilot wants to merge 2 commits intomainfrom
copilot/fix-model-reference-leak

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 28, 2026

In renderElementInner, the model reference acquired via createModelReference was leaked when the async race guard detected a stale element — the ref was never disposed before the early return.

Changes

  • disassemblyView.ts: Call ref.dispose() before returning when templateData.currentElement.element !== element, preventing the model reference leak on the stale-element code path.
// Before
const ref = await this.textModelService.createModelReference(sourceURI);
if (templateData.currentElement.element !== element) {
    return; // ← ref leaked
}

// After
const ref = await this.textModelService.createModelReference(sourceURI);
if (templateData.currentElement.element !== element) {
    ref.dispose(); // dispose before bailing out
    return;
}

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix model reference leak in disassemblyView.ts fix: dispose createModelReference ref on stale element in disassemblyView Mar 28, 2026
Copilot AI requested a review from connor4312 March 28, 2026 18:02
@connor4312 connor4312 marked this pull request as ready for review March 29, 2026 03:40
Copilot AI review requested due to automatic review settings March 29, 2026 03:40
@connor4312 connor4312 enabled auto-merge (squash) March 29, 2026 03:40
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

Fixes a resource leak in the Debug Disassembly View’s instruction renderer by ensuring ITextModelService.createModelReference() results are disposed when an async render detects the element has become stale.

Changes:

  • Dispose the acquired model reference before early-returning on the stale-element race guard in InstructionRenderer.renderElementInner.

@@ -862,7 +862,8 @@ class InstructionRenderer extends Disposable implements ITableRenderer<IDisassem
const sourceSB = new StringBuilder(10000);
const ref = await this.textModelService.createModelReference(sourceURI);
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The new stale-element guard disposes the ref when the template is reused, but the same async race can still leak when the row/template gets disposed (e.g. list/view disposed) while awaiting createModelReference: currentElement.element remains element, so the guard won’t trigger and the ref can be pushed after disposeElement has already run. Consider clearing templateData.currentElement.element in disposeElement (or tracking a render/dispose token) so the post-await check reliably detects disposed templates and disposes the ref before returning/continuing.

Suggested change
const ref = await this.textModelService.createModelReference(sourceURI);
const ref = await this.textModelService.createModelReference(sourceURI);
// If the template has been disposed while awaiting the model, its DOM node will be detached.
// In that case, dispose the ref and stop to avoid leaking the model or touching stale DOM.
if (!templateData.sourcecode.isConnected) {
ref.dispose();
return;
}

Copilot uses AI. Check for mistakes.
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.

createModelReference leak in disassemblyView.ts — ref not disposed on stale element

4 participants