Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Incorrect webWorkerExtensionHost startup process on vscode desktop #234505

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.LazyAutoStart),
remote: {
authority: this._environmentService.remoteAuthority,
connectionData: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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.LazyAutoStart) {
return this._instantiationService.createInstance(LazyCreateExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService);
}
return this._instantiationService.createInstance(ExtensionHostManager, extensionHost, initialActivationEvents, internalExtensionService);
}
Expand Down Expand Up @@ -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()));
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/services/extensions/common/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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) {
Expand All @@ -55,6 +54,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
Expand All @@ -64,12 +64,11 @@ 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 {
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;
}
Expand All @@ -80,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;
}

Expand All @@ -90,34 +89,39 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten
await this._actual.ready();
}
}

public async disconnect(): Promise<void> {
await this._actual?.disconnect();
}

public representsRunningLocation(runningLocation: ExtensionRunningLocation): boolean {
return this._extensionHost.runningLocation.equals(runningLocation);
}

public async deltaExtensions(extensionsDelta: IExtensionDescriptionDelta): Promise<void> {
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<boolean> {
await this._startCalled.wait();
if (this._actual) {
return this._actual.activate(extension, reason);
}
return false;
}

public async activateByEvent(activationEvent: string, activationKind: ActivationKind): Promise<void> {
if (activationKind === ActivationKind.Immediate) {
// this is an immediate request, so we cannot wait for start to be called
Expand All @@ -131,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;
Expand All @@ -140,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<IResolveAuthorityResult> {
await this._startCalled.wait();
if (this._actual) {
Expand All @@ -158,30 +165,33 @@ export class LazyCreateExtensionHostManager extends Disposable implements IExten
}
};
}

public async getCanonicalURI(remoteAuthority: string, uri: URI): Promise<URI | null> {
await this._startCalled.wait();
if (this._actual) {
return this._actual.getCanonicalURI(remoteAuthority, uri);
}
throw new Error(`Cannot resolve canonical URI`);
}

public async start(extensionRegistryVersionId: number, allExtensions: IExtensionDescription[], myExtensions: ExtensionIdentifier[]): Promise<void> {
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<number> {
await this._startCalled.wait();
const actual = await this._getOrCreateActualAndStart(`execute tests.`);
return actual.extensionTestsExecute();
}

public async setRemoteEnvironment(env: { [key: string]: string | null }): Promise<void> {
await this._startCalled.wait();
if (this._actual) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.LazyAutoStart : ExtensionHostStartup.EagerManualStart;
return this._instantiationService.createInstance(WebWorkerExtensionHost, runningLocation, startup, this._createWebWorkerExtensionHostDataProvider(runningLocations, runningLocation));
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ suite('ExtensionService', () => {
override disconnect() {
return Promise.resolve();
}
override start(): Promise<void> {
return Promise.resolve();
}
override dispose(): void {
order.push(`dispose ${extensionHostId}`);
}
Expand Down