From bc199fbbe97ca321e552846e950e76e9e3c898a2 Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 4 Dec 2025 21:20:41 +0100 Subject: [PATCH] completions: keep instantiated ghost text provider in a service such that it can be shared between joint and normal providers (#2405) because ghost text provider leaks and creating two of them results in a leak --- ...ilotInlineCompletionItemProviderService.ts | 28 +++++++++++++ .../completionsCoreContribution.ts | 32 +++++---------- ...ilotInlineCompletionItemProviderService.ts | 40 +++++++++++++++++++ .../extension/vscode-node/services.ts | 3 ++ .../jointInlineCompletionProvider.ts | 26 ++++-------- .../extension/test/vscode-node/services.ts | 2 + 6 files changed, 91 insertions(+), 40 deletions(-) create mode 100644 extensions/copilot/src/extension/completions/common/copilotInlineCompletionItemProviderService.ts create mode 100644 extensions/copilot/src/extension/completions/vscode-node/copilotInlineCompletionItemProviderService.ts diff --git a/extensions/copilot/src/extension/completions/common/copilotInlineCompletionItemProviderService.ts b/extensions/copilot/src/extension/completions/common/copilotInlineCompletionItemProviderService.ts new file mode 100644 index 00000000000..ddd7032f791 --- /dev/null +++ b/extensions/copilot/src/extension/completions/common/copilotInlineCompletionItemProviderService.ts @@ -0,0 +1,28 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import type { InlineCompletionItemProvider } from 'vscode'; +import { createServiceIdentifier } from '../../../util/common/services'; +import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; + +export interface ICopilotInlineCompletionItemProviderService { + readonly _serviceBrand: undefined; + + getOrCreateInstantiationService(): IInstantiationService; + getOrCreateProvider(): InlineCompletionItemProvider; +} + +export const ICopilotInlineCompletionItemProviderService = createServiceIdentifier('ICopilotInlineCompletionItemProviderService'); + +export class NullCopilotInlineCompletionItemProviderService implements ICopilotInlineCompletionItemProviderService { + readonly _serviceBrand: undefined; + + getOrCreateInstantiationService(): IInstantiationService { + throw new Error('Not implemented'); + } + getOrCreateProvider(): InlineCompletionItemProvider { + throw new Error('Not implemented'); + } +} diff --git a/extensions/copilot/src/extension/completions/vscode-node/completionsCoreContribution.ts b/extensions/copilot/src/extension/completions/vscode-node/completionsCoreContribution.ts index c2d2d09503d..a082e490245 100644 --- a/extensions/copilot/src/extension/completions/vscode-node/completionsCoreContribution.ts +++ b/extensions/copilot/src/extension/completions/vscode-node/completionsCoreContribution.ts @@ -7,23 +7,18 @@ import { commands, languages } from 'vscode'; import { IAuthenticationService } from '../../../platform/authentication/common/authentication'; import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService'; import { IExperimentationService } from '../../../platform/telemetry/common/nullExperimentationService'; -import { Disposable, DisposableStore } from '../../../util/vs/base/common/lifecycle'; +import { Disposable } from '../../../util/vs/base/common/lifecycle'; import { autorun, observableFromEvent } from '../../../util/vs/base/common/observableInternal'; -import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; -import { createContext, registerUnificationCommands, setup } from '../../completions-core/vscode-node/completionsServiceBridges'; -import { CopilotInlineCompletionItemProvider } from '../../completions-core/vscode-node/extension/src/inlineCompletion'; +import { registerUnificationCommands } from '../../completions-core/vscode-node/completionsServiceBridges'; +import { ICopilotInlineCompletionItemProviderService } from '../common/copilotInlineCompletionItemProviderService'; import { unificationStateObservable } from './completionsUnificationContribution'; export class CompletionsCoreContribution extends Disposable { - private _provider: CopilotInlineCompletionItemProvider | undefined; - private readonly _copilotToken = observableFromEvent(this, this.authenticationService.onDidAuthenticationChange, () => this.authenticationService.copilotToken); - private _completionsInstantiationService: IInstantiationService | undefined; - constructor( - @IInstantiationService private readonly _instantiationService: IInstantiationService, + @ICopilotInlineCompletionItemProviderService _copilotInlineCompletionItemProviderService: ICopilotInlineCompletionItemProviderService, @IConfigurationService configurationService: IConfigurationService, @IExperimentationService experimentationService: IExperimentationService, @IAuthenticationService private readonly authenticationService: IAuthenticationService @@ -37,8 +32,9 @@ export class CompletionsCoreContribution extends Disposable { const configEnabled = configurationService.getExperimentBasedConfigObservable(ConfigKey.TeamInternal.InlineEditsEnableGhCompletionsProvider, experimentationService).read(reader); const extensionUnification = unificationStateValue?.extensionUnification ?? false; + let hasInstantiatedProvider = false; if (unificationStateValue?.codeUnification || extensionUnification || configEnabled || this._copilotToken.read(reader)?.isNoAuthUser) { - const provider = this._getOrCreateProvider(); + const provider = _copilotInlineCompletionItemProviderService.getOrCreateProvider(); reader.store.add( languages.registerInlineCompletionItemProvider( { pattern: '**' }, @@ -50,12 +46,14 @@ export class CompletionsCoreContribution extends Disposable { } ) ); + hasInstantiatedProvider = true; } void commands.executeCommand('setContext', 'github.copilot.extensionUnification.activated', extensionUnification); - if (extensionUnification && this._completionsInstantiationService) { - reader.store.add(this._completionsInstantiationService.invokeFunction(registerUnificationCommands)); + if (extensionUnification && hasInstantiatedProvider) { + const completionsInstaService = _copilotInlineCompletionItemProviderService.getOrCreateInstantiationService(); + reader.store.add(completionsInstaService.invokeFunction(registerUnificationCommands)); } })); @@ -64,14 +62,4 @@ export class CompletionsCoreContribution extends Disposable { void commands.executeCommand('setContext', 'github.copilot.activated', token !== undefined); })); } - - private _getOrCreateProvider() { - if (!this._provider) { - const disposables = this._register(new DisposableStore()); - this._completionsInstantiationService = this._instantiationService.invokeFunction(createContext, disposables); - this._completionsInstantiationService.invokeFunction(setup, disposables); - this._provider = disposables.add(this._completionsInstantiationService.createInstance(CopilotInlineCompletionItemProvider)); - } - return this._provider; - } } diff --git a/extensions/copilot/src/extension/completions/vscode-node/copilotInlineCompletionItemProviderService.ts b/extensions/copilot/src/extension/completions/vscode-node/copilotInlineCompletionItemProviderService.ts new file mode 100644 index 00000000000..7c41ed8ddfb --- /dev/null +++ b/extensions/copilot/src/extension/completions/vscode-node/copilotInlineCompletionItemProviderService.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { InlineCompletionItemProvider } from 'vscode'; +import { Disposable } from '../../../util/vs/base/common/lifecycle'; +import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; +import { createContext, setup } from '../../completions-core/vscode-node/completionsServiceBridges'; +import { CopilotInlineCompletionItemProvider } from '../../completions-core/vscode-node/extension/src/inlineCompletion'; +import { ICopilotInlineCompletionItemProviderService } from '../common/copilotInlineCompletionItemProviderService'; + +export class CopilotInlineCompletionItemProviderService extends Disposable implements ICopilotInlineCompletionItemProviderService { + readonly _serviceBrand: undefined; + + private _provider: InlineCompletionItemProvider | undefined; + private _completionsInstantiationService: IInstantiationService | undefined; + + constructor( + @IInstantiationService private readonly _instantiationService: IInstantiationService, + ) { + super(); + } + + getOrCreateInstantiationService(): IInstantiationService { + if (!this._completionsInstantiationService) { + this._completionsInstantiationService = this._instantiationService.invokeFunction(createContext, this._store); + } + return this._completionsInstantiationService; + } + + getOrCreateProvider(): InlineCompletionItemProvider { + if (!this._provider) { + this._completionsInstantiationService = this.getOrCreateInstantiationService(); + this._completionsInstantiationService.invokeFunction(setup, this._store); + this._provider = this._register(this._completionsInstantiationService.createInstance(CopilotInlineCompletionItemProvider)); + } + return this._provider; + } +} diff --git a/extensions/copilot/src/extension/extension/vscode-node/services.ts b/extensions/copilot/src/extension/extension/vscode-node/services.ts index 69ffbd35080..e3764173263 100644 --- a/extensions/copilot/src/extension/extension/vscode-node/services.ts +++ b/extensions/copilot/src/extension/extension/vscode-node/services.ts @@ -111,6 +111,8 @@ import { LanguageContextServiceImpl } from '../../typescriptContext/vscode-node/ import { IWorkspaceListenerService } from '../../workspaceRecorder/common/workspaceListenerService'; import { WorkspacListenerService } from '../../workspaceRecorder/vscode-node/workspaceListenerService'; import { registerServices as registerCommonServices } from '../vscode/services'; +import { ICopilotInlineCompletionItemProviderService } from '../../completions/common/copilotInlineCompletionItemProviderService'; +import { CopilotInlineCompletionItemProviderService } from '../../completions/vscode-node/copilotInlineCompletionItemProviderService'; // ########################################################################################### // ### ### @@ -210,6 +212,7 @@ export function registerServices(builder: IInstantiationServiceBuilder, extensio builder.define(IRerankerService, new SyncDescriptor(RerankerService)); builder.define(IProxyModelsService, new SyncDescriptor(ProxyModelsService)); builder.define(IInlineEditsModelService, new SyncDescriptor(InlineEditsModelService)); + builder.define(ICopilotInlineCompletionItemProviderService, new SyncDescriptor(CopilotInlineCompletionItemProviderService)); } function setupMSFTExperimentationService(builder: IInstantiationServiceBuilder, extensionContext: ExtensionContext) { diff --git a/extensions/copilot/src/extension/inlineEdits/vscode-node/jointInlineCompletionProvider.ts b/extensions/copilot/src/extension/inlineEdits/vscode-node/jointInlineCompletionProvider.ts index f174c6279a7..825402a2169 100644 --- a/extensions/copilot/src/extension/inlineEdits/vscode-node/jointInlineCompletionProvider.ts +++ b/extensions/copilot/src/extension/inlineEdits/vscode-node/jointInlineCompletionProvider.ts @@ -22,7 +22,7 @@ import { coalesce } from '../../../util/vs/base/common/arrays'; import { assertNever, softAssert } from '../../../util/vs/base/common/assert'; import { raceCancellation, raceTimeout } from '../../../util/vs/base/common/async'; import { CancellationToken, CancellationTokenSource } from '../../../util/vs/base/common/cancellation'; -import { Disposable, DisposableStore } from '../../../util/vs/base/common/lifecycle'; +import { Disposable } from '../../../util/vs/base/common/lifecycle'; import { autorun, derived, derivedDisposable, observableFromEvent } from '../../../util/vs/base/common/observable'; import { StopWatch } from '../../../util/vs/base/common/stopwatch'; import { URI } from '../../../util/vs/base/common/uri'; @@ -31,8 +31,9 @@ import { Range } from '../../../util/vs/editor/common/core/range'; import { StringText } from '../../../util/vs/editor/common/core/text/abstractText'; import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; import { IExtensionContribution } from '../../common/contributions'; -import { createContext, registerUnificationCommands, setup } from '../../completions-core/vscode-node/completionsServiceBridges'; +import { registerUnificationCommands } from '../../completions-core/vscode-node/completionsServiceBridges'; import { CopilotInlineCompletionItemProvider } from '../../completions-core/vscode-node/extension/src/inlineCompletion'; +import { ICopilotInlineCompletionItemProviderService } from '../../completions/common/copilotInlineCompletionItemProviderService'; import { CompletionsCoreContribution } from '../../completions/vscode-node/completionsCoreContribution'; import { unificationStateObservable } from '../../completions/vscode-node/completionsUnificationContribution'; import { TelemetrySender } from '../node/nextEditProviderTelemetry'; @@ -82,6 +83,7 @@ export class JointCompletionsProviderContribution extends Disposable implements constructor( @IVSCodeExtensionContext private readonly _vscodeExtensionContext: IVSCodeExtensionContext, @IInstantiationService private readonly _instantiationService: IInstantiationService, + @ICopilotInlineCompletionItemProviderService private readonly _copilotInlineCompletionItemProviderService: ICopilotInlineCompletionItemProviderService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IExperimentationService private readonly _expService: IExperimentationService, @IAuthenticationService private readonly _authenticationService: IAuthenticationService, @@ -178,13 +180,14 @@ export class JointCompletionsProviderContribution extends Disposable implements // @ulugbekna: note that we don't want it if modelUnification is on const modelUnification = unificationStateValue?.modelUnification ?? false; if (!modelUnification || unificationStateValue?.codeUnification || extensionUnification || configEnabled || this._copilotToken.read(reader)?.isNoAuthUser) { - completionsProvider = this._getOrCreateProvider(); + completionsProvider = this._copilotInlineCompletionItemProviderService.getOrCreateProvider() as CopilotInlineCompletionItemProvider; } void vscode.commands.executeCommand('setContext', 'github.copilot.extensionUnification.activated', extensionUnification); - if (extensionUnification && this._completionsInstantiationService) { - reader.store.add(this._completionsInstantiationService.invokeFunction(registerUnificationCommands)); + if (extensionUnification && completionsProvider) { + const completionsInstaService = this._copilotInlineCompletionItemProviderService.getOrCreateInstantiationService(); + reader.store.add(completionsInstaService.invokeFunction(registerUnificationCommands)); } } @@ -210,19 +213,6 @@ export class JointCompletionsProviderContribution extends Disposable implements })); })); } - - private _provider: CopilotInlineCompletionItemProvider | undefined; - private _completionsInstantiationService: IInstantiationService | undefined; - private _getOrCreateProvider() { - if (!this._provider) { - const disposables = this._register(new DisposableStore()); - this._completionsInstantiationService = this._instantiationService.invokeFunction(createContext, disposables); - this._completionsInstantiationService.invokeFunction(setup, disposables); - this._provider = disposables.add(this._completionsInstantiationService.createInstance(CopilotInlineCompletionItemProvider)); - } - return this._provider; - } - } type SingularCompletionItem = diff --git a/extensions/copilot/src/extension/test/vscode-node/services.ts b/extensions/copilot/src/extension/test/vscode-node/services.ts index 30646fc1163..7f5362e655f 100644 --- a/extensions/copilot/src/extension/test/vscode-node/services.ts +++ b/extensions/copilot/src/extension/test/vscode-node/services.ts @@ -89,6 +89,7 @@ import { ExtensionTextDocumentManager } from '../../../platform/workspace/vscode import { GithubAvailableEmbeddingTypesService, IGithubAvailableEmbeddingTypesService } from '../../../platform/workspaceChunkSearch/common/githubAvailableEmbeddingTypes'; import { SyncDescriptor } from '../../../util/vs/platform/instantiation/common/descriptors'; import { CommandServiceImpl, ICommandService } from '../../commands/node/commandService'; +import { ICopilotInlineCompletionItemProviderService, NullCopilotInlineCompletionItemProviderService } from '../../completions/common/copilotInlineCompletionItemProviderService'; import { IPromptWorkspaceLabels, PromptWorkspaceLabels } from '../../context/node/resolvers/promptWorkspaceLabels'; import { IUserFeedbackService, UserFeedbackService } from '../../conversation/vscode-node/userActions'; import { ConversationStore, IConversationStore } from '../../conversationStore/node/conversationStore'; @@ -192,6 +193,7 @@ export function createExtensionTestingServices(): TestingServiceCollection { testingServiceCollection.define(IGithubAvailableEmbeddingTypesService, new SyncDescriptor(GithubAvailableEmbeddingTypesService)); testingServiceCollection.define(IProxyModelsService, new SyncDescriptor(NullProxyModelsService)); testingServiceCollection.define(IInlineEditsModelService, new SyncDescriptor(InlineEditsModelService)); + testingServiceCollection.define(ICopilotInlineCompletionItemProviderService, new SyncDescriptor(NullCopilotInlineCompletionItemProviderService)); return testingServiceCollection; }