From ac5293b1734bdfef67cfd514e58e443d2974f79e Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 27 Jul 2026 10:37:35 -0700 Subject: [PATCH] Fix Repository retention leaks in the git and github extensions (#327491) Fix repository retention leaks in git and github extensions ProgressManager subscribed to workspace.onDidChangeConfiguration and repository.onDidChangeOperations in its constructor but discarded both subscriptions, so the global configuration emitter pinned every Repository ever created. GitHubBranchProtectionProviderManager registered a branch protection provider per opened repository into a single DisposableStore that was only disposed when the feature was turned off. There was no onDidCloseRepository handling, and the providers themselves were never disposed. Track them per repository root instead and dispose on close. Also make DisposableStore dispose anything added after it has been disposed, so the async registration in GitHubBranchProtectionProvider's constructor cannot outlive the provider. Refs #327438 (Written by Copilot) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 041724a6-5bc9-4e5f-98b2-a910995f083c --- extensions/git/src/repository.ts | 6 ++- extensions/github/src/branchProtection.ts | 46 ++++++++++++++++++++--- extensions/github/src/util.ts | 9 +++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/extensions/git/src/repository.ts b/extensions/git/src/repository.ts index ec27cbd57fe..062b0067966 100644 --- a/extensions/git/src/repository.ts +++ b/extensions/git/src/repository.ts @@ -361,16 +361,17 @@ class ProgressManager { private enabled = false; private disposable: IDisposable = EmptyDisposable; + private readonly disposables: IDisposable[] = []; constructor(private repository: Repository) { const onDidChange = filterEvent(workspace.onDidChangeConfiguration, e => e.affectsConfiguration('git', Uri.file(this.repository.root))); - onDidChange(_ => this.updateEnablement()); + onDidChange(_ => this.updateEnablement(), null, this.disposables); this.updateEnablement(); this.repository.onDidChangeOperations(() => { // Disable input box when the commit operation is running this.repository.sourceControl.inputBox.enabled = !this.repository.operations.isRunning(OperationKind.Commit); - }); + }, null, this.disposables); } private updateEnablement(): void { @@ -414,6 +415,7 @@ class ProgressManager { dispose(): void { this.disable(); + dispose(this.disposables); } } diff --git a/extensions/github/src/branchProtection.ts b/extensions/github/src/branchProtection.ts index 0c616d33905..c5cf1182bcc 100644 --- a/extensions/github/src/branchProtection.ts +++ b/extensions/github/src/branchProtection.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { EventEmitter, LogOutputChannel, Memento, Uri, workspace } from 'vscode'; +import { Disposable, EventEmitter, LogOutputChannel, Memento, Uri, workspace } from 'vscode'; import { Repository as GitHubRepository, RepositoryRuleset } from '@octokit/graphql-schema'; import { AuthenticationError, OctokitService } from './auth.js'; import type { API, BranchProtection, BranchProtectionProvider, BranchProtectionRule, Repository } from './typings/git.d.ts'; @@ -51,7 +51,12 @@ const REPOSITORY_RULESETS_QUERY = ` export class GitHubBranchProtectionProviderManager { private readonly disposables = new DisposableStore(); - private readonly providerDisposables = new DisposableStore(); + + /** + * Branch protection providers, keyed by repository root. Entries are disposed + * when the repository is closed so that closed repositories are not retained. + */ + private readonly providers = new Map(); private _enabled = false; private set enabled(enabled: boolean) { @@ -61,10 +66,10 @@ export class GitHubBranchProtectionProviderManager { if (enabled) { for (const repository of this.gitAPI.repositories) { - this.providerDisposables.add(this.gitAPI.registerBranchProtectionProvider(repository.rootUri, new GitHubBranchProtectionProvider(repository, this.globalState, this.octokitService, this.logger, this.telemetryReporter))); + this.registerProvider(repository); } } else { - this.providerDisposables.dispose(); + this.disposeProviders(); } this._enabled = enabled; @@ -78,11 +83,14 @@ export class GitHubBranchProtectionProviderManager { private readonly telemetryReporter: TelemetryReporter) { this.disposables.add(this.gitAPI.onDidOpenRepository(repository => { if (this._enabled) { - this.providerDisposables.add(gitAPI.registerBranchProtectionProvider(repository.rootUri, - new GitHubBranchProtectionProvider(repository, this.globalState, this.octokitService, this.logger, this.telemetryReporter))); + this.registerProvider(repository); } })); + this.disposables.add(this.gitAPI.onDidCloseRepository(repository => { + this.disposeProvider(repository.rootUri.toString()); + })); + this.disposables.add(workspace.onDidChangeConfiguration(e => { if (e.affectsConfiguration('github.branchProtection')) { this.updateEnablement(); @@ -92,6 +100,32 @@ export class GitHubBranchProtectionProviderManager { this.updateEnablement(); } + private registerProvider(repository: Repository): void { + const key = repository.rootUri.toString(); + this.disposeProvider(key); + + const provider = new GitHubBranchProtectionProvider(repository, this.globalState, this.octokitService, this.logger, this.telemetryReporter); + const registration = this.gitAPI.registerBranchProtectionProvider(repository.rootUri, provider); + + this.providers.set(key, new Disposable(() => { + registration.dispose(); + provider.dispose(); + })); + } + + private disposeProvider(key: string): void { + this.providers.get(key)?.dispose(); + this.providers.delete(key); + } + + private disposeProviders(): void { + for (const provider of this.providers.values()) { + provider.dispose(); + } + + this.providers.clear(); + } + private updateEnablement(): void { const config = workspace.getConfiguration('github', null); this.enabled = config.get('branchProtection', true) === true; diff --git a/extensions/github/src/util.ts b/extensions/github/src/util.ts index bcdddaed6e5..54627308da7 100644 --- a/extensions/github/src/util.ts +++ b/extensions/github/src/util.ts @@ -9,12 +9,21 @@ import type { Repository } from './typings/git.d.ts'; export class DisposableStore { private disposables = new Set(); + private isDisposed = false; add(disposable: vscode.Disposable): void { + if (this.isDisposed) { + // The store was already disposed, so nothing would ever dispose this. + disposable.dispose(); + return; + } + this.disposables.add(disposable); } dispose(): void { + this.isDisposed = true; + for (const disposable of this.disposables) { disposable.dispose(); }