diff --git a/extensions/ql-vscode/src/common/vscode/abstract-webview.ts b/extensions/ql-vscode/src/common/vscode/abstract-webview.ts index 715969837eb..99a2dbdc382 100644 --- a/extensions/ql-vscode/src/common/vscode/abstract-webview.ts +++ b/extensions/ql-vscode/src/common/vscode/abstract-webview.ts @@ -9,7 +9,7 @@ import { import { join } from "path"; import { App } from "../app"; -import { DisposableObject, DisposeHandler } from "../disposable-object"; +import { Disposable } from "../disposable-object"; import { tmpDir } from "../../tmp-dir"; import { getHtmlForWebview, WebviewMessage, WebviewKind } from "./webview-html"; @@ -27,16 +27,16 @@ export type WebviewPanelConfig = { export abstract class AbstractWebview< ToMessage extends WebviewMessage, FromMessage extends WebviewMessage, -> extends DisposableObject { +> { protected panel: WebviewPanel | undefined; protected panelLoaded = false; protected panelLoadedCallBacks: Array<() => void> = []; private panelResolves?: Array<(panel: WebviewPanel) => void>; - constructor(protected readonly app: App) { - super(); - } + private disposables: Disposable[] = []; + + constructor(protected readonly app: App) {} public async restoreView(panel: WebviewPanel): Promise { this.panel = panel; @@ -101,6 +101,7 @@ export abstract class AbstractWebview< this.panel = undefined; this.panelLoaded = false; this.onPanelDispose(); + this.disposeAll(); }, null), ); @@ -150,8 +151,27 @@ export abstract class AbstractWebview< return panel.webview.postMessage(msg); } - public dispose(disposeHandler?: DisposeHandler) { + public dispose() { this.panel?.dispose(); - super.dispose(disposeHandler); + this.disposeAll(); + } + + private disposeAll() { + while (this.disposables.length > 0) { + const disposable = this.disposables.pop()!; + disposable.dispose(); + } + } + + /** + * Adds `obj` to a list of objects to dispose when the panel is disposed. Objects added by `push` are + * disposed in reverse order of being added. + * @param obj The object to take ownership of. + */ + protected push(obj: T): T { + if (obj !== undefined) { + this.disposables.push(obj); + } + return obj; } } diff --git a/extensions/ql-vscode/src/local-queries/results-view.ts b/extensions/ql-vscode/src/local-queries/results-view.ts index ff00016f90b..b39b305f260 100644 --- a/extensions/ql-vscode/src/local-queries/results-view.ts +++ b/extensions/ql-vscode/src/local-queries/results-view.ts @@ -75,6 +75,7 @@ import { telemetryListener } from "../common/vscode/telemetry"; import { redactableError } from "../common/errors"; import { ResultsViewCommands } from "../common/commands"; import { App } from "../common/app"; +import { Disposable } from "../common/disposable-object"; /** * results-view.ts @@ -157,6 +158,12 @@ function numInterpretedPages( return Math.ceil(n / pageSize); } +/** + * The results view is used for displaying the results of a local query. It is a singleton; only 1 results view exists + * in the extension. It is created when the extension is activated and disposed of when the extension is deactivated. + * There can be multiple panels linked to this view over the lifetime of the extension, but there is only ever 1 panel + * active at a time. + */ export class ResultsView extends AbstractWebview< IntoResultsViewMsg, FromResultsViewMsg @@ -168,6 +175,9 @@ export class ResultsView extends AbstractWebview< "codeql-query-results", ); + // Event listeners that should be disposed of when the view is disposed. + private disposableEventListeners: Disposable[] = []; + constructor( app: App, private databaseManager: DatabaseManager, @@ -176,14 +186,16 @@ export class ResultsView extends AbstractWebview< private labelProvider: HistoryItemLabelProvider, ) { super(app); - this.push(this._diagnosticCollection); - this.push( + + // We can't use this.push for these two event listeners because they need to be disposed of when the view is + // disposed, not when the panel is disposed. The results view is a singleton, so we shouldn't be calling this.push. + this.disposableEventListeners.push( vscode.window.onDidChangeTextEditorSelection( this.handleSelectionChange.bind(this), ), ); - this.push( + this.disposableEventListeners.push( this.databaseManager.onDidChangeDatabaseItem(({ kind }) => { if (kind === DatabaseEventKind.Remove) { this._diagnosticCollection.clear(); @@ -981,4 +993,12 @@ export class ResultsView extends AbstractWebview< editor.setDecorations(shownLocationLineDecoration, []); } } + + dispose() { + super.dispose(); + + this._diagnosticCollection.dispose(); + this.disposableEventListeners.forEach((d) => d.dispose()); + this.disposableEventListeners = []; + } }