Skip to content

Commit

Permalink
Fix potential leak of chat editors
Browse files Browse the repository at this point in the history
Fixes microsoft#213209

I believe that chat editors are potentially being leaked when progressive rendering ends due to `contentParts` being set without also being disposed of

Also fixes the wrong selection potentially being used

Fixes microsoft#217835
  • Loading branch information
mjbvz committed Jul 24, 2024
1 parent ea0d703 commit ad0c612
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
7 changes: 5 additions & 2 deletions src/vs/workbench/contrib/chat/browser/chatListRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { ChatFollowups } from 'vs/workbench/contrib/chat/browser/chatFollowups';
import { ChatMarkdownDecorationsRenderer } from 'vs/workbench/contrib/chat/browser/chatMarkdownDecorationsRenderer';
import { ChatMarkdownRenderer } from 'vs/workbench/contrib/chat/browser/chatMarkdownRenderer';
import { ChatEditorOptions } from 'vs/workbench/contrib/chat/browser/chatOptions';
import { ChatCodeBlockContentProvider } from 'vs/workbench/contrib/chat/browser/codeBlockPart';
import { ChatCodeBlockContentProvider, CodeBlockPart } from 'vs/workbench/contrib/chat/browser/codeBlockPart';
import { ChatAgentLocation, IChatAgentMetadata } from 'vs/workbench/contrib/chat/common/chatAgents';
import { CONTEXT_CHAT_RESPONSE_SUPPORT_ISSUE_REPORTING, CONTEXT_REQUEST, CONTEXT_RESPONSE, CONTEXT_RESPONSE_DETECTED_AGENT_COMMAND, CONTEXT_RESPONSE_FILTERED, CONTEXT_RESPONSE_VOTE } from 'vs/workbench/contrib/chat/common/chatContextKeys';
import { IChatRequestVariableEntry, IChatTextEditGroup } from 'vs/workbench/contrib/chat/common/chatModel';
Expand Down Expand Up @@ -162,7 +162,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
return ChatListItemRenderer.ID;
}

editorsInUse() {
editorsInUse(): Iterable<CodeBlockPart> {
return this._editorPool.inUse();
}

Expand Down Expand Up @@ -488,6 +488,9 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
parts.push(newPart);
}
});
if (templateData.renderedParts) {
dispose(templateData.renderedParts);
}
templateData.renderedParts = parts;

if (isRequestVM(element) && element.variables.length) {
Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/contrib/chat/browser/chatWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ export class ChatWidget extends Disposable implements IChatWidget {
inner.setSelection({
startLineNumber: input.options.selection.startLineNumber,
startColumn: input.options.selection.startColumn,
endLineNumber: input.options.selection.startLineNumber ?? input.options.selection.endLineNumber,
endColumn: input.options.selection.startColumn ?? input.options.selection.endColumn
endLineNumber: input.options.selection.endLineNumber ?? input.options.selection.startLineNumber,
endColumn: input.options.selection.endColumn ?? input.options.selection.startColumn
});
}
return inner;
Expand Down Expand Up @@ -413,7 +413,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
`${isResponseVM(element) && element.renderData ? `_${this.visibleChangeCount}` : ''}` +
// Re-render once content references are loaded
(isResponseVM(element) ? `_${element.contentReferences.length}` : '') +
// Rerender request if we got new content references in the response
// Rerender request if we got new content references in the response
// since this may change how we render the corresponding attachments in the request
(isRequestVM(element) && element.contentReferences ? `_${element.contentReferences?.length}` : '');
},
Expand Down

0 comments on commit ad0c612

Please sign in to comment.