From 7cb02f12ee807b950c659d72080c50d4e53bd06c Mon Sep 17 00:00:00 2001 From: Kaidesuyoo Date: Sun, 24 Nov 2024 21:13:24 +0800 Subject: [PATCH 1/4] fix: Prevent incorrect startup of WebWorkerExtensionHost after extension host restart --- .../common/abstractExtensionService.ts | 23 +++++++++++-------- .../common/lazyCreateExtensionHostManager.ts | 3 ++- .../nativeExtensionService.ts | 6 +---- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/vs/workbench/services/extensions/common/abstractExtensionService.ts b/src/vs/workbench/services/extensions/common/abstractExtensionService.ts index 68b71d7ce1bc4..3a4967eb996f3 100644 --- a/src/vs/workbench/services/extensions/common/abstractExtensionService.ts +++ b/src/vs/workbench/services/extensions/common/abstractExtensionService.ts @@ -462,13 +462,7 @@ export abstract class AbstractExtensionService extends Disposable implements IEx try { await this._resolveAndProcessExtensions(lock); // Start extension hosts which are not automatically started - const snapshot = this._registry.getSnapshot(); - for (const extHostManager of this._extensionHostManagers) { - if (extHostManager.startup !== ExtensionHostStartup.EagerAutoStart) { - const extensions = this._runningLocations.filterByExtensionHostManager(snapshot.extensions, extHostManager); - extHostManager.start(snapshot.versionId, snapshot.extensions, extensions.map(extension => extension.identifier)); - } - } + this._startOnDemandExtensionHosts(); } finally { lock.dispose(); } @@ -820,8 +814,8 @@ export abstract class AbstractExtensionService extends Disposable implements IEx protected _doCreateExtensionHostManager(extensionHost: IExtensionHost, initialActivationEvents: string[]): IExtensionHostManager { const internalExtensionService = this._acquireInternalAPI(extensionHost); - if (extensionHost.startup === ExtensionHostStartup.Lazy && initialActivationEvents.length === 0) { - return this._instantiationService.createInstance(LazyCreateExtensionHostManager, extensionHost, internalExtensionService); + if (extensionHost.startup === ExtensionHostStartup.Lazy) { + return this._instantiationService.createInstance(LazyCreateExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService); } return this._instantiationService.createInstance(ExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService); } @@ -920,6 +914,7 @@ export abstract class AbstractExtensionService extends Disposable implements IEx const lock = await this._registry.acquireLock('startExtensionHosts'); try { this._startExtensionHostsIfNecessary(false, Array.from(this._allRequestedActivateEvents.keys())); + this._startOnDemandExtensionHosts(); const localProcessExtensionHosts = this._getExtensionHostManagers(ExtensionHostKind.LocalProcess); await Promise.all(localProcessExtensionHosts.map(extHost => extHost.ready())); @@ -928,6 +923,16 @@ export abstract class AbstractExtensionService extends Disposable implements IEx } } + private _startOnDemandExtensionHosts(): void { + const snapshot = this._registry.getSnapshot(); + for (const extHostManager of this._extensionHostManagers) { + if (extHostManager.startup !== ExtensionHostStartup.EagerAutoStart) { + const extensions = this._runningLocations.filterByExtensionHostManager(snapshot.extensions, extHostManager); + extHostManager.start(snapshot.versionId, snapshot.extensions, extensions.map(extension => extension.identifier)); + } + } + } + //#endregion //#region IExtensionService diff --git a/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts b/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts index 29b7584e662ce..e54c2e9e76729 100644 --- a/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts +++ b/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts @@ -55,6 +55,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten constructor( extensionHost: IExtensionHost, + private readonly _initialActivationEvents: string[], private readonly _internalExtensionService: IInternalExtensionService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private readonly _logService: ILogService @@ -69,7 +70,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten private _createActual(reason: string): ExtensionHostManager { this._logService.info(`Creating lazy extension host (${this.friendyName}). Reason: ${reason}`); - this._actual = this._register(this._instantiationService.createInstance(ExtensionHostManager, this._extensionHost, [], this._internalExtensionService)); + this._actual = this._register(this._instantiationService.createInstance(ExtensionHostManager, this._extensionHost, this._initialActivationEvents, this._internalExtensionService)); this._register(this._actual.onDidChangeResponsiveState((e) => this._onDidChangeResponsiveState.fire(e))); return this._actual; } diff --git a/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts b/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts index 8adc7c19fe7a9..2c4e7b636a4ee 100644 --- a/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts +++ b/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts @@ -555,11 +555,7 @@ class NativeExtensionHostFactory implements IExtensionHostFactory { } case ExtensionHostKind.LocalWebWorker: { if (this._webWorkerExtHostEnablement !== LocalWebWorkerExtHostEnablement.Disabled) { - const startup = ( - isInitialStart - ? (this._webWorkerExtHostEnablement === LocalWebWorkerExtHostEnablement.Lazy ? ExtensionHostStartup.Lazy : ExtensionHostStartup.EagerManualStart) - : ExtensionHostStartup.EagerAutoStart - ); + const startup = this._webWorkerExtHostEnablement === LocalWebWorkerExtHostEnablement.Lazy ? ExtensionHostStartup.Lazy : ExtensionHostStartup.EagerManualStart; return this._instantiationService.createInstance(WebWorkerExtensionHost, runningLocation, startup, this._createWebWorkerExtensionHostDataProvider(runningLocations, runningLocation)); } return null; From 68779a37cf84a40b1ae6cd4983f260fe256cfd82 Mon Sep 17 00:00:00 2001 From: Kaidesuyoo Date: Sun, 24 Nov 2024 21:22:30 +0800 Subject: [PATCH 2/4] fix: resolve incorrect extensionRegistryVersionId during LazyCreateExtensionHostManager startup --- .../browser/webWorkerExtensionHost.ts | 2 +- .../common/lazyCreateExtensionHostManager.ts | 29 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts b/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts index 54fc51bb873cd..05eda46facd8c 100644 --- a/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts +++ b/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts @@ -332,7 +332,7 @@ export class WebWorkerExtensionHost extends Disposable implements IExtensionHost logLevel: this._logService.getLevel(), loggers: [...this._loggerService.getRegisteredLoggers()], logsLocation: this._extensionHostLogsLocation, - autoStart: (this.startup === ExtensionHostStartup.EagerAutoStart), + autoStart: (this.startup === ExtensionHostStartup.EagerAutoStart || this.startup === ExtensionHostStartup.Lazy), remote: { authority: this._environmentService.remoteAuthority, connectionData: null, diff --git a/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts b/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts index e54c2e9e76729..9f7413825838e 100644 --- a/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts +++ b/src/vs/workbench/services/extensions/common/lazyCreateExtensionHostManager.ts @@ -17,7 +17,7 @@ import { IExtensionHostManager } from './extensionHostManagers.js'; import { IExtensionDescriptionDelta } from './extensionHostProtocol.js'; import { IResolveAuthorityResult } from './extensionHostProxy.js'; import { ExtensionRunningLocation } from './extensionRunningLocation.js'; -import { ActivationKind, ExtensionActivationReason, ExtensionHostExtensions, ExtensionHostStartup, IExtensionHost, IInternalExtensionService } from './extensions.js'; +import { ActivationKind, ExtensionActivationReason, ExtensionHostStartup, IExtensionHost, IInternalExtensionService } from './extensions.js'; import { ResponsiveState } from './rpcProtocol.js'; /** @@ -32,7 +32,6 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten private readonly _extensionHost: IExtensionHost; private _startCalled: Barrier; private _actual: ExtensionHostManager | null; - private _lazyStartExtensions: ExtensionHostExtensions | null; public get pid(): number | null { if (this._actual) { @@ -65,7 +64,6 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten this.onDidExit = extensionHost.onExit; this._startCalled = new Barrier(); this._actual = null; - this._lazyStartExtensions = null; } private _createActual(reason: string): ExtensionHostManager { @@ -81,7 +79,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten return this._actual; } const actual = this._createActual(reason); - await actual.start(this._lazyStartExtensions!.versionId, this._lazyStartExtensions!.allExtensions, this._lazyStartExtensions!.myExtensions); + await actual.ready(); return actual; } @@ -91,27 +89,31 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten await this._actual.ready(); } } + public async disconnect(): Promise { await this._actual?.disconnect(); } + public representsRunningLocation(runningLocation: ExtensionRunningLocation): boolean { return this._extensionHost.runningLocation.equals(runningLocation); } + public async deltaExtensions(extensionsDelta: IExtensionDescriptionDelta): Promise { await this._startCalled.wait(); if (this._actual) { return this._actual.deltaExtensions(extensionsDelta); } - this._lazyStartExtensions!.delta(extensionsDelta); if (extensionsDelta.myToAdd.length > 0) { const actual = this._createActual(`contains ${extensionsDelta.myToAdd.length} new extension(s) (installed or enabled): ${extensionsDelta.myToAdd.map(extId => extId.value)}`); - await actual.start(this._lazyStartExtensions!.versionId, this._lazyStartExtensions!.allExtensions, this._lazyStartExtensions!.myExtensions); + await actual.ready(); return; } } + public containsExtension(extensionId: ExtensionIdentifier): boolean { return this._extensionHost.extensions?.containsExtension(extensionId) ?? false; } + public async activate(extension: ExtensionIdentifier, reason: ExtensionActivationReason): Promise { await this._startCalled.wait(); if (this._actual) { @@ -119,6 +121,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten } return false; } + public async activateByEvent(activationEvent: string, activationKind: ActivationKind): Promise { if (activationKind === ActivationKind.Immediate) { // this is an immediate request, so we cannot wait for start to be called @@ -132,6 +135,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten return this._actual.activateByEvent(activationEvent, activationKind); } } + public activationEventIsDone(activationEvent: string): boolean { if (!this._startCalled.isOpen()) { return false; @@ -141,10 +145,12 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten } return true; } + public async getInspectPort(tryEnableInspector: boolean): Promise<{ port: number; host: string } | undefined> { await this._startCalled.wait(); return this._actual?.getInspectPort(tryEnableInspector); } + public async resolveAuthority(remoteAuthority: string, resolveAttempt: number): Promise { await this._startCalled.wait(); if (this._actual) { @@ -159,6 +165,7 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten } }; } + public async getCanonicalURI(remoteAuthority: string, uri: URI): Promise { await this._startCalled.wait(); if (this._actual) { @@ -166,23 +173,25 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten } throw new Error(`Cannot resolve canonical URI`); } + public async start(extensionRegistryVersionId: number, allExtensions: IExtensionDescription[], myExtensions: ExtensionIdentifier[]): Promise { if (myExtensions.length > 0) { - // there are actual extensions, so let's launch the extension host + // there are actual extensions, so let's launch the extension host (auto-start) const actual = this._createActual(`contains ${myExtensions.length} extension(s): ${myExtensions.map(extId => extId.value)}.`); - const result = actual.start(extensionRegistryVersionId, allExtensions, myExtensions); + const result = actual.ready(); this._startCalled.open(); return result; } - // there are no actual extensions running, store extensions in `this._lazyStartExtensions` - this._lazyStartExtensions = new ExtensionHostExtensions(extensionRegistryVersionId, allExtensions, myExtensions); + // there are no actual extensions running this._startCalled.open(); } + public async extensionTestsExecute(): Promise { await this._startCalled.wait(); const actual = await this._getOrCreateActualAndStart(`execute tests.`); return actual.extensionTestsExecute(); } + public async setRemoteEnvironment(env: { [key: string]: string | null }): Promise { await this._startCalled.wait(); if (this._actual) { From c3b219018d48ab6ff271e38e33e639ddf676b799 Mon Sep 17 00:00:00 2001 From: Kaidesuyoo Date: Sun, 24 Nov 2024 21:43:47 +0800 Subject: [PATCH 3/4] fix red tests --- .../services/extensions/test/browser/extensionService.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vs/workbench/services/extensions/test/browser/extensionService.test.ts b/src/vs/workbench/services/extensions/test/browser/extensionService.test.ts index 89b97f4ee7ccf..70d2de0d1a59d 100644 --- a/src/vs/workbench/services/extensions/test/browser/extensionService.test.ts +++ b/src/vs/workbench/services/extensions/test/browser/extensionService.test.ts @@ -202,6 +202,9 @@ suite('ExtensionService', () => { override disconnect() { return Promise.resolve(); } + override start(): Promise { + return Promise.resolve(); + } override dispose(): void { order.push(`dispose ${extensionHostId}`); } From d98a91d24cfbe9c27346272358d07ee2e11e9dec Mon Sep 17 00:00:00 2001 From: Kaidesuyoo Date: Mon, 25 Nov 2024 12:28:38 +0800 Subject: [PATCH 4/4] modify the confusing `ExtensionHostStartup` state --- .../services/extensions/browser/webWorkerExtensionHost.ts | 2 +- .../services/extensions/common/abstractExtensionService.ts | 2 +- src/vs/workbench/services/extensions/common/extensions.ts | 4 ++-- .../extensions/electron-sandbox/nativeExtensionService.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts b/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts index 05eda46facd8c..ec0dd1c739fa9 100644 --- a/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts +++ b/src/vs/workbench/services/extensions/browser/webWorkerExtensionHost.ts @@ -332,7 +332,7 @@ export class WebWorkerExtensionHost extends Disposable implements IExtensionHost logLevel: this._logService.getLevel(), loggers: [...this._loggerService.getRegisteredLoggers()], logsLocation: this._extensionHostLogsLocation, - autoStart: (this.startup === ExtensionHostStartup.EagerAutoStart || this.startup === ExtensionHostStartup.Lazy), + autoStart: (this.startup === ExtensionHostStartup.EagerAutoStart || this.startup === ExtensionHostStartup.LazyAutoStart), remote: { authority: this._environmentService.remoteAuthority, connectionData: null, diff --git a/src/vs/workbench/services/extensions/common/abstractExtensionService.ts b/src/vs/workbench/services/extensions/common/abstractExtensionService.ts index 3a4967eb996f3..b72e98bc25ee0 100644 --- a/src/vs/workbench/services/extensions/common/abstractExtensionService.ts +++ b/src/vs/workbench/services/extensions/common/abstractExtensionService.ts @@ -814,7 +814,7 @@ export abstract class AbstractExtensionService extends Disposable implements IEx protected _doCreateExtensionHostManager(extensionHost: IExtensionHost, initialActivationEvents: string[]): IExtensionHostManager { const internalExtensionService = this._acquireInternalAPI(extensionHost); - if (extensionHost.startup === ExtensionHostStartup.Lazy) { + if (extensionHost.startup === ExtensionHostStartup.LazyAutoStart) { return this._instantiationService.createInstance(LazyCreateExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService); } return this._instantiationService.createInstance(ExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService); diff --git a/src/vs/workbench/services/extensions/common/extensions.ts b/src/vs/workbench/services/extensions/common/extensions.ts index cd50e10acfb7a..1eb681b7b3e17 100644 --- a/src/vs/workbench/services/extensions/common/extensions.ts +++ b/src/vs/workbench/services/extensions/common/extensions.ts @@ -107,9 +107,9 @@ export const enum ExtensionHostStartup { */ EagerManualStart = 2, /** - * The extension host should be launched lazily and only when it has extensions it needs to host. It needs a `$startExtensionHost` call. + * The extension host should be launched lazily and only when it has extensions it needs to host. It doesn't require a `$startExtensionHost` call. */ - Lazy = 3, + LazyAutoStart = 3, } export interface IExtensionHost { diff --git a/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts b/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts index 2c4e7b636a4ee..dcd0e3a00f744 100644 --- a/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts +++ b/src/vs/workbench/services/extensions/electron-sandbox/nativeExtensionService.ts @@ -555,7 +555,7 @@ class NativeExtensionHostFactory implements IExtensionHostFactory { } case ExtensionHostKind.LocalWebWorker: { if (this._webWorkerExtHostEnablement !== LocalWebWorkerExtHostEnablement.Disabled) { - const startup = this._webWorkerExtHostEnablement === LocalWebWorkerExtHostEnablement.Lazy ? ExtensionHostStartup.Lazy : ExtensionHostStartup.EagerManualStart; + const startup = this._webWorkerExtHostEnablement === LocalWebWorkerExtHostEnablement.Lazy ? ExtensionHostStartup.LazyAutoStart : ExtensionHostStartup.EagerManualStart; return this._instantiationService.createInstance(WebWorkerExtensionHost, runningLocation, startup, this._createWebWorkerExtensionHostDataProvider(runningLocations, runningLocation)); } return null;