Skip to content

Commit

Permalink
Properly synchronise terminalTaskSystem._activeTasks
Browse files Browse the repository at this point in the history
Fixes microsoft#180578

This fixes the issue by keeping a map of tasks that have been passed
to _executeTask, but which haven't yet been added to _activeTasks.

Code that needs to synchronise with _activeTasks waits on the corresponding
_activatingTasks promise, which will resolve when the task is actually added to
_activeTasks (or when an error causes it to not be started).
  • Loading branch information
markw65 committed Apr 21, 2023
1 parent bede6ba commit 889dce3
Showing 1 changed file with 36 additions and 0 deletions.
36 changes: 36 additions & 0 deletions src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
};

private _activeTasks: IStringDictionary<IActiveTerminalData>;
private _activatingTasks: IStringDictionary<Promise<void>>;
private _instances: IStringDictionary<InstanceManager>;
private _busyTasks: IStringDictionary<Task>;
private _terminals: IStringDictionary<ITerminalData>;
Expand Down Expand Up @@ -256,6 +257,7 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
super();

this._activeTasks = Object.create(null);
this._activatingTasks = Object.create(null);
this._instances = Object.create(null);
this._busyTasks = Object.create(null);
this._terminals = Object.create(null);
Expand Down Expand Up @@ -526,13 +528,43 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
this._showOutput();
}

private async _acquireActivatingTask(mapKey: string) {
while (true) {
const promise = this._activatingTasks[mapKey];
if (!promise) {
break;
}
this._logService.info('Waiting for activatingTask', mapKey);
await promise;
}
this._logService.info('Acquiring activatingTask', mapKey);
// Do not return, or await this promise!
this._activatingTasks[mapKey] = new Promise<void>((resolve) => {
const taskInactiveDisposable = this.onDidStateChange((event) => {
// A Changed event will be sent right after this._activeTasks is updated.
// An End event will be sent, even if the task was never actually activated
// due to an error or misconfiguration.
// In either case, we're done with this promise.

if ((event.kind === TaskEventKind.Changed && this._activeTasks[mapKey]) ||
(event.kind === TaskEventKind.End && event.__task?.getMapKey() === mapKey)) {
this._logService.info('Releasing activatingTask', mapKey);
taskInactiveDisposable.dispose();
resolve();
delete this._activatingTasks[mapKey];
}
});
});
}

private async _executeTask(task: Task, resolver: ITaskResolver, trigger: string, encounteredDependencies: Set<string>, alreadyResolved?: Map<string, string>): Promise<ITaskSummary> {
if (encounteredDependencies.has(task.getCommonTaskId())) {
this._showDependencyCycleMessage(task);
return {};
}

this._showTaskLoadErrors(task);
await this._acquireActivatingTask(task.getMapKey());

alreadyResolved = alreadyResolved ?? new Map<string, string>();
const promises: Promise<ITaskSummary>[] = [];
Expand All @@ -542,6 +574,10 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
if (dependencyTask) {
this._adoptConfigurationForDependencyTask(dependencyTask, task);
const key = dependencyTask.getMapKey();
if (key in this._activatingTasks) {
this._logService.info('Waiting for dependent activatingTask', key);
await this._activatingTasks[key];
}
let promise = this._activeTasks[key] ? this._getDependencyPromise(this._activeTasks[key]) : undefined;
if (!promise) {
this._fireTaskEvent(TaskEvent.create(TaskEventKind.DependsOnStarted, task));
Expand Down

0 comments on commit 889dce3

Please sign in to comment.