From 9b669a0c71544049728ac7bf3aaf19d667eb0916 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 28 Jan 2019 17:07:41 +0100 Subject: [PATCH] Remove extensions with dependency loops as soon as possible --- .../api/node/extHostExtensionActivator.ts | 21 +--- .../electron-browser/extensionService.ts | 14 ++- .../node/extensionDescriptionRegistry.ts | 117 +++++++++++++++++- 3 files changed, 128 insertions(+), 24 deletions(-) diff --git a/src/vs/workbench/api/node/extHostExtensionActivator.ts b/src/vs/workbench/api/node/extHostExtensionActivator.ts index a21a307e7b9..102fd5bedce 100644 --- a/src/vs/workbench/api/node/extHostExtensionActivator.ts +++ b/src/vs/workbench/api/node/extHostExtensionActivator.ts @@ -231,7 +231,7 @@ export class ExtensionsActivator { return NO_OP_VOID_PROMISE; } let activateExtensions = this._registry.getExtensionDescriptionsForActivationEvent(activationEvent); - return this._activateExtensions(activateExtensions, reason, 0).then(() => { + return this._activateExtensions(activateExtensions, reason).then(() => { this._alreadyActivatedEvents[activationEvent] = true; }); } @@ -242,7 +242,7 @@ export class ExtensionsActivator { throw new Error('Extension `' + extensionId + '` is not known'); } - return this._activateExtensions([desc], reason, 0); + return this._activateExtensions([desc], reason); } /** @@ -295,7 +295,7 @@ export class ExtensionsActivator { } } - private _activateExtensions(extensionDescriptions: IExtensionDescription[], reason: ExtensionActivationReason, recursionLevel: number): Promise { + private _activateExtensions(extensionDescriptions: IExtensionDescription[], reason: ExtensionActivationReason): Promise { // console.log(recursionLevel, '_activateExtensions: ', extensionDescriptions.map(p => p.id)); if (extensionDescriptions.length === 0) { return Promise.resolve(undefined); @@ -306,17 +306,6 @@ export class ExtensionsActivator { return Promise.resolve(undefined); } - if (recursionLevel > 10) { - // More than 10 dependencies deep => most likely a dependency loop - for (let i = 0, len = extensionDescriptions.length; i < len; i++) { - // Error condition 3: dependency loop - this._host.showMessage(Severity.Error, nls.localize('failedDep2', "Extension '{0}' failed to activate. Reason: more than 10 levels of dependencies (most likely a dependency loop).", extensionDescriptions[i].identifier.value)); - const error = new Error('More than 10 levels of dependencies (most likely a dependency loop)'); - this._activatedExtensions.set(ExtensionIdentifier.toKey(extensionDescriptions[i].identifier), new FailedExtension(error)); - } - return Promise.resolve(undefined); - } - let greenMap: { [id: string]: IExtensionDescription; } = Object.create(null), red: IExtensionDescription[] = []; @@ -342,8 +331,8 @@ export class ExtensionsActivator { return Promise.all(green.map((p) => this._activateExtension(p, reason))).then(_ => undefined); } - return this._activateExtensions(green, reason, recursionLevel + 1).then(_ => { - return this._activateExtensions(red, reason, recursionLevel + 1); + return this._activateExtensions(green, reason).then(_ => { + return this._activateExtensions(red, reason); }); } diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts index 801b69f39a2..306aa82dc67 100644 --- a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts +++ b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts @@ -227,7 +227,11 @@ export class ExtensionService extends Disposable implements IExtensionService { } // Update the local registry - this._registry.deltaExtensions(toAdd, toRemove.map(e => e.identifier)); + const result = this._registry.deltaExtensions(toAdd, toRemove.map(e => e.identifier)); + toRemove = toRemove.concat(result.removedDueToLooping); + if (result.removedDueToLooping.length > 0) { + this._logOrShowMessage(Severity.Error, nls.localize('looping', "The following extensions contain dependency loops and have been disabled: {0}", result.removedDueToLooping.map(e => `'${e.identifier.value}'`).join(', '))); + } // Update extension points this._rehandleExtensionPoints(([]).concat(toAdd).concat(toRemove)); @@ -625,12 +629,16 @@ export class ExtensionService extends Disposable implements IExtensionService { const enabledExtensions = await this._getRuntimeExtensions(extensions); this._handleExtensionPoints(enabledExtensions); - extensionHost.start(enabledExtensions.map(extension => extension.identifier)); + extensionHost.start(enabledExtensions.map(extension => extension.identifier).filter(id => this._registry.containsExtension(id))); this._releaseBarrier(); } private _handleExtensionPoints(allExtensions: IExtensionDescription[]): void { - this._registry = new ExtensionDescriptionRegistry(allExtensions); + this._registry = new ExtensionDescriptionRegistry([]); + const result = this._registry.deltaExtensions(allExtensions, []); + if (result.removedDueToLooping.length > 0) { + this._logOrShowMessage(Severity.Error, nls.localize('looping', "The following extensions contain dependency loops and have been disabled: {0}", result.removedDueToLooping.map(e => `'${e.identifier.value}'`).join(', '))); + } let availableExtensions = this._registry.getAllExtensionDescriptions(); let extensionPoints = ExtensionsRegistry.getExtensionPoints(); diff --git a/src/vs/workbench/services/extensions/node/extensionDescriptionRegistry.ts b/src/vs/workbench/services/extensions/node/extensionDescriptionRegistry.ts index 3eddf8c90ed..c4182d11adf 100644 --- a/src/vs/workbench/services/extensions/node/extensionDescriptionRegistry.ts +++ b/src/vs/workbench/services/extensions/node/extensionDescriptionRegistry.ts @@ -7,6 +7,12 @@ import { IExtensionDescription } from 'vs/workbench/services/extensions/common/e import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { Emitter } from 'vs/base/common/event'; +export class DeltaExtensionsResult { + constructor( + public readonly removedDueToLooping: IExtensionDescription[] + ) { } +} + export class ExtensionDescriptionRegistry { private readonly _onDidChange = new Emitter(); public readonly onDidChange = this._onDidChange.event; @@ -60,19 +66,120 @@ export class ExtensionDescriptionRegistry { this._onDidChange.fire(undefined); } - public deltaExtensions(toAdd: IExtensionDescription[], toRemove: ExtensionIdentifier[]) { - this._extensionDescriptions = this._extensionDescriptions.concat(toAdd); - const toRemoveSet = new Set(); - toRemove.forEach(extensionId => toRemoveSet.add(ExtensionIdentifier.toKey(extensionId))); - this._extensionDescriptions = this._extensionDescriptions.filter(extension => !toRemoveSet.has(ExtensionIdentifier.toKey(extension.identifier))); + public deltaExtensions(toAdd: IExtensionDescription[], toRemove: ExtensionIdentifier[]): DeltaExtensionsResult { + if (toAdd.length > 0) { + this._extensionDescriptions = this._extensionDescriptions.concat(toAdd); + } + + // Immediately remove looping extensions! + const looping = ExtensionDescriptionRegistry._findLoopingExtensions(this._extensionDescriptions); + toRemove = toRemove.concat(looping.map(ext => ext.identifier)); + + if (toRemove.length > 0) { + const toRemoveSet = new Set(); + toRemove.forEach(extensionId => toRemoveSet.add(ExtensionIdentifier.toKey(extensionId))); + this._extensionDescriptions = this._extensionDescriptions.filter(extension => !toRemoveSet.has(ExtensionIdentifier.toKey(extension.identifier))); + } + this._initialize(); this._onDidChange.fire(undefined); + return new DeltaExtensionsResult(looping); + } + + private static _findLoopingExtensions(extensionDescriptions: IExtensionDescription[]): IExtensionDescription[] { + const G = new class { + + private _arcs = new Map(); + private _nodesSet = new Set(); + private _nodesArr: string[] = []; + + addNode(id: string): void { + if (!this._nodesSet.has(id)) { + this._nodesSet.add(id); + this._nodesArr.push(id); + } + } + + addArc(from: string, to: string): void { + this.addNode(from); + this.addNode(to); + if (this._arcs.has(from)) { + this._arcs.get(from).push(to); + } else { + this._arcs.set(from, [to]); + } + } + + getArcs(id: string): string[] { + if (this._arcs.has(id)) { + return this._arcs.get(id); + } + return []; + } + + hasOnlyGoodArcs(id: string, good: Set): boolean { + const dependencies = G.getArcs(id); + for (let i = 0; i < dependencies.length; i++) { + if (!good.has(dependencies[i])) { + return false; + } + } + return true; + } + + getNodes(): string[] { + return this._nodesArr; + } + }; + + let descs = new Map(); + for (let extensionDescription of extensionDescriptions) { + const extensionId = ExtensionIdentifier.toKey(extensionDescription.identifier); + descs.set(extensionId, extensionDescription); + if (extensionDescription.extensionDependencies) { + for (let _depId of extensionDescription.extensionDependencies) { + const depId = ExtensionIdentifier.toKey(_depId); + G.addArc(extensionId, depId); + } + } + } + + // initialize with all extensions with no dependencies. + let good = new Set(); + G.getNodes().filter(id => G.getArcs(id).length === 0).forEach(id => good.add(id)); + + // all other extensions will be processed below. + let nodes = G.getNodes().filter(id => !good.has(id)); + + let madeProgress: boolean; + do { + madeProgress = false; + + // find one extension which has only good deps + for (let i = 0; i < nodes.length; i++) { + const id = nodes[i]; + + if (G.hasOnlyGoodArcs(id, good)) { + nodes.splice(i, 1); + i--; + good.add(id); + madeProgress = true; + } + } + } while (madeProgress); + + // The remaining nodes are bad and have loops + return nodes.map(id => descs.get(id)); } public containsActivationEvent(activationEvent: string): boolean { return this._activationMap.has(activationEvent); } + public containsExtension(extensionId: ExtensionIdentifier): boolean { + return this._extensionsMap.has(ExtensionIdentifier.toKey(extensionId)); + } + public getExtensionDescriptionsForActivationEvent(activationEvent: string): IExtensionDescription[] { const extensions = this._activationMap.get(activationEvent); return extensions ? extensions.slice(0) : []; -- GitLab