fix: memory leak in linkDetector

src/vs/workbench/contrib/debug/browser/debugANSIHandling.ts

@@ -11,13 +11,13 @@ import { registerThemingParticipant } from '../../../../platform/theme/common/th
 import { IWorkspaceFolder } from '../../../../platform/workspace/common/workspace.js';
 import { PANEL_BACKGROUND, SIDE_BAR_BACKGROUND } from '../../../common/theme.js';
 import { ansiColorIdentifiers } from '../../terminal/common/terminalColorRegistry.js';
-import { ILinkDetector } from './linkDetector.js';
+import { DebugLinkHoverBehaviorTypeData, ILinkDetector } from './linkDetector.js';
 
 /**
  * @param text The content to stylize.
  * @returns An {@link HTMLSpanElement} that contains the potentially stylized text.
  */
-export function handleANSIOutput(text: string, linkDetector: ILinkDetector, workspaceFolder: IWorkspaceFolder | undefined, highlights: IHighlight[] | undefined): HTMLSpanElement {
+export function handleANSIOutput(text: string, linkDetector: ILinkDetector, workspaceFolder: IWorkspaceFolder | undefined, highlights: IHighlight[] | undefined, hoverBehavior: DebugLinkHoverBehaviorTypeData): HTMLSpanElement {
 
 	const root: HTMLSpanElement = document.createElement('span');
 	const textLength: number = text.length;
@@ -63,8 +63,7 @@ export function handleANSIOutput(text: string, linkDetector: ILinkDetector, work
 				unprintedChars += 2 + ansiSequence.length;
 
 				// Flush buffer with previous styles.
-				appendStylizedStringToContainer(root, buffer, styleNames, linkDetector, workspaceFolder, customFgColor, customBgColor, customUnderlineColor, highlights, currentPos - buffer.length - unprintedChars);
-
+				appendStylizedStringToContainer(root, buffer, styleNames, linkDetector, workspaceFolder, customFgColor, customBgColor, customUnderlineColor, highlights, currentPos - buffer.length - unprintedChars, hoverBehavior);
 				buffer = '';
 
 				/*
@@ -109,7 +108,7 @@ export function handleANSIOutput(text: string, linkDetector: ILinkDetector, work
 
 	// Flush remaining text buffer if not empty.
 	if (buffer) {
-		appendStylizedStringToContainer(root, buffer, styleNames, linkDetector, workspaceFolder, customFgColor, customBgColor, customUnderlineColor, highlights, currentPos - buffer.length);
+		appendStylizedStringToContainer(root, buffer, styleNames, linkDetector, workspaceFolder, customFgColor, customBgColor, customUnderlineColor, highlights, currentPos - buffer.length, hoverBehavior);
 	}
 
 	return root;
@@ -401,6 +400,7 @@ export function handleANSIOutput(text: string, linkDetector: ILinkDetector, work
  * @param customUnderlineColor If provided, will apply custom textDecorationColor with inline style.
  * @param highlights The ranges to highlight.
  * @param offset The starting index of the stringContent in the original text.
+ * @param hoverBehavior hover behavior with disposable store for managing event listeners.
  */
 export function appendStylizedStringToContainer(
 	root: HTMLElement,
@@ -413,17 +413,18 @@ export function appendStylizedStringToContainer(
 	customUnderlineColor: RGBA | string | undefined,
 	highlights: IHighlight[] | undefined,
 	offset: number,
+	hoverBehavior: DebugLinkHoverBehaviorTypeData,
 ): void {
 	if (!root || !stringContent) {
 		return;
 	}
 
 	const container = linkDetector.linkify(
 		stringContent,
+		hoverBehavior,
 		true,
 		workspaceFolder,
 		undefined,
-		undefined,
 		highlights?.map(h => ({ start: h.start - offset, end: h.end - offset, extraClasses: h.extraClasses })),
 	);
 

src/vs/workbench/contrib/debug/browser/debugExpressionRenderer.ts

@@ -177,7 +177,7 @@ export class DebugExpressionRenderer {
 
 		const session = options.session ?? ((expressionOrValue instanceof ExpressionContainer) ? expressionOrValue.getSession() : undefined);
 		// Only use hovers for links if thre's not going to be a hover for the value.
-		const hoverBehavior: DebugLinkHoverBehaviorTypeData = options.hover === false ? { type: DebugLinkHoverBehavior.Rich, store } : { type: DebugLinkHoverBehavior.None };
+		const hoverBehavior: DebugLinkHoverBehaviorTypeData = options.hover === false ? { type: DebugLinkHoverBehavior.Rich, store } : { type: DebugLinkHoverBehavior.None, store };
 		dom.clearNode(container);
 		const locationReference = options.locationReference ?? (expressionOrValue instanceof ExpressionContainer && expressionOrValue.valueLocationReference);
 
@@ -187,9 +187,9 @@ export class DebugExpressionRenderer {
 		}
 
 		if (supportsANSI) {
-			container.appendChild(handleANSIOutput(value, linkDetector, session ? session.root : undefined, options.highlights));
+			container.appendChild(handleANSIOutput(value, linkDetector, session ? session.root : undefined, options.highlights, hoverBehavior));
 		} else {
-			container.appendChild(linkDetector.linkify(value, false, session?.root, true, hoverBehavior, options.highlights));
+			container.appendChild(linkDetector.linkify(value, hoverBehavior, false, session?.root, true, options.highlights));
 		}
 
 		if (options.hover !== false) {
@@ -202,7 +202,7 @@ export class DebugExpressionRenderer {
 				if (supportsANSI) {
 					// note: intentionally using `this.linkDetector` so we don't blindly linkify the
 					// entire contents and instead only link file paths that it contains.
-					hoverContentsPre.appendChild(handleANSIOutput(value, this.linkDetector, session ? session.root : undefined, options.highlights));
+					hoverContentsPre.appendChild(handleANSIOutput(value, this.linkDetector, session ? session.root : undefined, options.highlights, hoverBehavior));
 				} else {
 					hoverContentsPre.textContent = value;
 				}

src/vs/workbench/contrib/debug/browser/exceptionWidget.ts

@@ -15,12 +15,13 @@ import { ThemeIcon } from '../../../../base/common/themables.js';
 import { Color } from '../../../../base/common/color.js';
 import { registerColor } from '../../../../platform/theme/common/colorRegistry.js';
 import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
-import { DebugLinkHoverBehavior, LinkDetector } from './linkDetector.js';
+import { DebugLinkHoverBehavior, DebugLinkHoverBehaviorTypeData, LinkDetector } from './linkDetector.js';
 import { EditorOption } from '../../../../editor/common/config/editorOptions.js';
 import { ActionBar } from '../../../../base/browser/ui/actionbar/actionbar.js';
 import { Action } from '../../../../base/common/actions.js';
 import { widgetClose } from '../../../../platform/theme/common/iconRegistry.js';
 import { Range } from '../../../../editor/common/core/range.js';
+
 const $ = dom.$;
 
 // theming
@@ -82,7 +83,7 @@ export class ExceptionWidget extends ZoneWidget {
 		label.textContent = this.exceptionInfo.id ? nls.localize('exceptionThrownWithId', 'Exception has occurred: {0}', this.exceptionInfo.id) : nls.localize('exceptionThrown', 'Exception has occurred.');
 		let ariaLabel = label.textContent;
 
-		const actionBar = new ActionBar(actions);
+		const actionBar = this._disposables.add(new ActionBar(actions));
 		actionBar.push(new Action('editor.closeExceptionWidget', nls.localize('close', "Close"), ThemeIcon.asClassName(widgetClose), true, async () => {
 			const contribution = this.editor.getContribution<IDebugEditorContribution>(EDITOR_CONTRIBUTION_ID);
 			contribution?.closeExceptionWidget();
@@ -100,7 +101,11 @@ export class ExceptionWidget extends ZoneWidget {
 		if (this.exceptionInfo.details && this.exceptionInfo.details.stackTrace) {
 			const stackTrace = $('.stack-trace');
 			const linkDetector = this.instantiationService.createInstance(LinkDetector);
-			const linkedStackTrace = linkDetector.linkify(this.exceptionInfo.details.stackTrace, true, this.debugSession ? this.debugSession.root : undefined, undefined, { type: DebugLinkHoverBehavior.Rich, store: this._disposables });
+			const hoverBehaviour: DebugLinkHoverBehaviorTypeData = {
+				store: this._disposables,
+				type: DebugLinkHoverBehavior.Rich,
+			};
+			const linkedStackTrace = linkDetector.linkify(this.exceptionInfo.details.stackTrace, hoverBehaviour, true, this.debugSession ? this.debugSession.root : undefined, undefined);
 			stackTrace.appendChild(linkedStackTrace);
 			dom.append(container, stackTrace);
 			ariaLabel += ', ' + this.exceptionInfo.details.stackTrace;

src/vs/workbench/contrib/debug/browser/linkDetector.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { getWindow, isHTMLElement, reset } from '../../../../base/browser/dom.js';
+import { addDisposableListener, getWindow, isHTMLElement, reset } from '../../../../base/browser/dom.js';
 import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js';
 import { getDefaultHoverDelegate } from '../../../../base/browser/ui/hover/hoverDelegateFactory.js';
 import { KeyCode } from '../../../../base/common/keyCodes.js';
@@ -61,12 +61,16 @@ export const enum DebugLinkHoverBehavior {
 }
 
 /** Store implies HoverBehavior=rich */
-export type DebugLinkHoverBehaviorTypeData = { type: DebugLinkHoverBehavior.None | DebugLinkHoverBehavior.Basic }
+export type DebugLinkHoverBehaviorTypeData =
+	| { type: DebugLinkHoverBehavior.None; store: DisposableStore }
+	| { type: DebugLinkHoverBehavior.Basic; store: DisposableStore }
 	| { type: DebugLinkHoverBehavior.Rich; store: DisposableStore };
 
+
+
 export interface ILinkDetector {
-	linkify(text: string, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, hoverBehavior?: DebugLinkHoverBehaviorTypeData, highlights?: IHighlight[]): HTMLElement;
-	linkifyLocation(text: string, locationReference: number, session: IDebugSession, hoverBehavior?: DebugLinkHoverBehaviorTypeData): HTMLElement;
+	linkify(text: string, hoverBehavior: DebugLinkHoverBehaviorTypeData, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, highlights?: IHighlight[]): HTMLElement;
+	linkifyLocation(text: string, locationReference: number, session: IDebugSession, hoverBehavior: DebugLinkHoverBehaviorTypeData): HTMLElement;
 }
 
 export class LinkDetector implements ILinkDetector {
@@ -89,14 +93,13 @@ export class LinkDetector implements ILinkDetector {
 	 * 'onclick' event is attached to all anchored links that opens them in the editor.
 	 * When splitLines is true, each line of the text, even if it contains no links, is wrapped in a <span>
 	 * and added as a child of the returned <span>.
-	 * If a `hoverBehavior` is passed, hovers may be added using the workbench hover service.
-	 * This should be preferred for new code where hovers are desirable.
+	 * The `hoverBehavior` is required and manages the lifecycle of event listeners.
 	 */
-	linkify(text: string, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, hoverBehavior?: DebugLinkHoverBehaviorTypeData, highlights?: IHighlight[]): HTMLElement {
-		return this._linkify(text, splitLines, workspaceFolder, includeFulltext, hoverBehavior, highlights);
+	linkify(text: string, hoverBehavior: DebugLinkHoverBehaviorTypeData, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, highlights?: IHighlight[]): HTMLElement {
+		return this._linkify(text, hoverBehavior, splitLines, workspaceFolder, includeFulltext, highlights);
 	}
 
-	private _linkify(text: string, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, hoverBehavior?: DebugLinkHoverBehaviorTypeData, highlights?: IHighlight[], defaultRef?: { locationReference: number; session: IDebugSession }): HTMLElement {
+	private _linkify(text: string, hoverBehavior: DebugLinkHoverBehaviorTypeData, splitLines?: boolean, workspaceFolder?: IWorkspaceFolder, includeFulltext?: boolean, highlights?: IHighlight[], defaultRef?: { locationReference: number; session: IDebugSession }): HTMLElement {
 		if (splitLines) {
 			const lines = text.split('\n');
 			for (let i = 0; i < lines.length - 1; i++) {
@@ -106,7 +109,7 @@ export class LinkDetector implements ILinkDetector {
 				// Remove the last element ('') that split added.
 				lines.pop();
 			}
-			const elements = lines.map(line => this._linkify(line, false, workspaceFolder, includeFulltext, hoverBehavior, highlights, defaultRef));
+			const elements = lines.map(line => this._linkify(line, hoverBehavior, false, workspaceFolder, includeFulltext, highlights, defaultRef));
 			if (elements.length === 1) {
 				// Do not wrap single line with extra span.
 				return elements[0];
@@ -193,7 +196,7 @@ export class LinkDetector implements ILinkDetector {
 	/**
 	 * Linkifies a location reference.
 	 */
-	linkifyLocation(text: string, locationReference: number, session: IDebugSession, hoverBehavior?: DebugLinkHoverBehaviorTypeData) {
+	linkifyLocation(text: string, locationReference: number, session: IDebugSession, hoverBehavior: DebugLinkHoverBehaviorTypeData) {
 		const link = this.createLink(text);
 		this.decorateLink(link, undefined, text, hoverBehavior, async (preserveFocus: boolean) => {
 			const location = await session.resolveLocationReference(locationReference);
@@ -214,13 +217,13 @@ export class LinkDetector implements ILinkDetector {
 	 */
 	makeReferencedLinkDetector(locationReference: number, session: IDebugSession): ILinkDetector {
 		return {
-			linkify: (text, splitLines, workspaceFolder, includeFulltext, hoverBehavior, highlights) =>
-				this._linkify(text, splitLines, workspaceFolder, includeFulltext, hoverBehavior, highlights, { locationReference, session }),
+			linkify: (text, hoverBehavior, splitLines, workspaceFolder, includeFulltext, highlights) =>
+				this._linkify(text, hoverBehavior, splitLines, workspaceFolder, includeFulltext, highlights, { locationReference, session }),
 			linkifyLocation: this.linkifyLocation.bind(this),
 		};
 	}
 
-	private createWebLink(fulltext: string | undefined, url: string, hoverBehavior?: DebugLinkHoverBehaviorTypeData): Node {
+	private createWebLink(fulltext: string | undefined, url: string, hoverBehavior: DebugLinkHoverBehaviorTypeData): Node {
 		const link = this.createLink(url);
 
 		let uri = URI.parse(url);
@@ -264,7 +267,7 @@ export class LinkDetector implements ILinkDetector {
 		return link;
 	}
 
-	private createPathLink(fulltext: string | undefined, text: string, path: string, lineNumber: number, columnNumber: number, workspaceFolder: IWorkspaceFolder | undefined, hoverBehavior?: DebugLinkHoverBehaviorTypeData): Node {
+	private createPathLink(fulltext: string | undefined, text: string, path: string, lineNumber: number, columnNumber: number, workspaceFolder: IWorkspaceFolder | undefined, hoverBehavior: DebugLinkHoverBehaviorTypeData): Node {
 		if (path[0] === '/' && path[1] === '/') {
 			// Most likely a url part which did not match, for example ftp://path.
 			return document.createTextNode(text);
@@ -312,22 +315,31 @@ export class LinkDetector implements ILinkDetector {
 		return link;
 	}
 
-	private decorateLink(link: HTMLElement, uri: URI | undefined, fulltext: string | undefined, hoverBehavior: DebugLinkHoverBehaviorTypeData | undefined, onClick: (preserveFocus: boolean) => void) {
+	private decorateLink(link: HTMLElement, uri: URI | undefined, fulltext: string | undefined, hoverBehavior: DebugLinkHoverBehaviorTypeData, onClick: (preserveFocus: boolean) => void) {
+		if (hoverBehavior.store.isDisposed) {
+			return;
+		}
 		link.classList.add('link');
 		const followLink = uri && this.tunnelService.canTunnel(uri) ? localize('followForwardedLink', "follow link using forwarded port") : localize('followLink', "follow link");
 		const title = link.ariaLabel = fulltext
 			? (platform.isMacintosh ? localize('fileLinkWithPathMac', "Cmd + click to {0}\n{1}", followLink, fulltext) : localize('fileLinkWithPath', "Ctrl + click to {0}\n{1}", followLink, fulltext))
 			: (platform.isMacintosh ? localize('fileLinkMac', "Cmd + click to {0}", followLink) : localize('fileLink', "Ctrl + click to {0}", followLink));
 
-		if (hoverBehavior?.type === DebugLinkHoverBehavior.Rich) {
+		if (hoverBehavior.type === DebugLinkHoverBehavior.Rich) {
 			hoverBehavior.store.add(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), link, title));
-		} else if (hoverBehavior?.type !== DebugLinkHoverBehavior.None) {
+		} else if (hoverBehavior.type !== DebugLinkHoverBehavior.None) {
 			link.title = title;
 		}
 
-		link.onmousemove = (event) => { link.classList.toggle('pointer', platform.isMacintosh ? event.metaKey : event.ctrlKey); };
-		link.onmouseleave = () => link.classList.remove('pointer');
-		link.onclick = (event) => {
+		hoverBehavior.store.add(addDisposableListener(link, 'mousemove', (event: MouseEvent) => {
+			link.classList.toggle('pointer', platform.isMacintosh ? event.metaKey : event.ctrlKey);
+		}));
+
+		hoverBehavior.store.add(addDisposableListener(link, 'mouseleave', () => {
+			link.classList.remove('pointer');
+		}));
+
+		hoverBehavior.store.add(addDisposableListener(link, 'click', (event: MouseEvent) => {
 			const selection = getWindow(link).getSelection();
 			if (!selection || selection.type === 'Range') {
 				return; // do not navigate when user is selecting
@@ -339,15 +351,16 @@ export class LinkDetector implements ILinkDetector {
 			event.preventDefault();
 			event.stopImmediatePropagation();
 			onClick(false);
-		};
-		link.onkeydown = e => {
+		}));
+
+		hoverBehavior.store.add(addDisposableListener(link, 'keydown', (e: KeyboardEvent) => {
 			const event = new StandardKeyboardEvent(e);
 			if (event.keyCode === KeyCode.Enter || event.keyCode === KeyCode.Space) {
 				event.preventDefault();
 				event.stopPropagation();
 				onClick(event.keyCode === KeyCode.Space);
 			}
-		};
+		}));
 	}
 
 	private detectLinks(text: string): LinkPart[] {