Skip to content

Commit

Permalink
Remove extensions with dependency loops as soon as possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexdima
alexdima committed Feb 8, 2019
1 parent b909133 commit 9b669a0
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 24 deletions.
21 changes: 5 additions & 16 deletions src/vs/workbench/api/node/extHostExtensionActivator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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[] = [];

Expand All @@ -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);
});
}

Expand Down
14 changes: 11 additions & 3 deletions src/vs/workbench/services/extensions/electron-browser/extensionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand Down
117 changes: 112 additions & 5 deletions src/vs/workbench/services/extensions/node/extensionDescriptionRegistry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) : [];
Expand Down

0 comments on commit 9b669a0

Please sign in to comment.