Remove extensions with dependency loops as soon as possible

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<void> {
+	private _activateExtensions(extensionDescriptions: IExtensionDescription[], reason: ExtensionActivationReason): Promise<void> {
 		// 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);
 		});
 	}
 

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((<IExtensionDescription[]>[]).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();

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<void>();
 	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<string>();
-		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<string>();
+			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<string, string[]>();
+			private _nodesSet = new Set<string>();
+			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<string>): 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<string, IExtensionDescription>();
+		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<string>();
+		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) : [];