Skip to content

Commit 5969fdd

Browse files
authored
commands: fix for startup command in vscode.dev/edu route (microsoft#250680)
* commands: fix for startup command in vscode.dev/edu route Closes microsoft/vscode-dev#1290 properly. I didn't realize that, if given a promise, `Promise.resolve` just returns that same promise, so my attempt to hide the `cancel` method didn't work. * update comment
1 parent 89fbfc2 commit 5969fdd

File tree

2 files changed

+25
-8
lines changed

2 files changed

+25
-8
lines changed

src/vs/base/common/async.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ export function raceCancellationError<T>(promise: Promise<T>, token: Cancellatio
116116
});
117117
}
118118

119+
/**
120+
* Wraps a cancellable promise such that it is no cancellable. Can be used to
121+
* avoid issues with shared promises that would normally be returned as
122+
* cancellable to consumers.
123+
*/
124+
export function notCancellablePromise<T>(promise: CancelablePromise<T>): Promise<T> {
125+
return new Promise<T>((resolve, reject) => {
126+
promise.then(resolve, reject);
127+
});
128+
}
129+
119130
/**
120131
* Returns as soon as one of the promises resolves or rejects and cancels remaining promises
121132
*/

src/vs/workbench/services/commands/common/commandService.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { raceCancellablePromises, timeout } from '../../../../base/common/async.js';
6+
import { CancelablePromise, notCancellablePromise, raceCancellablePromises, timeout } from '../../../../base/common/async.js';
77
import { Emitter, Event } from '../../../../base/common/event.js';
88
import { Disposable } from '../../../../base/common/lifecycle.js';
99
import { CommandsRegistry, ICommandEvent, ICommandService } from '../../../../platform/commands/common/commands.js';
@@ -17,7 +17,7 @@ export class CommandService extends Disposable implements ICommandService {
1717
declare readonly _serviceBrand: undefined;
1818

1919
private _extensionHostIsReady: boolean = false;
20-
private _starActivation: Promise<void> | null;
20+
private _starActivation: CancelablePromise<void> | null;
2121

2222
private readonly _onWillExecuteCommand: Emitter<ICommandEvent> = this._register(new Emitter<ICommandEvent>());
2323
public readonly onWillExecuteCommand: Event<ICommandEvent> = this._onWillExecuteCommand.event;
@@ -37,15 +37,16 @@ export class CommandService extends Disposable implements ICommandService {
3737

3838
private _activateStar(): Promise<void> {
3939
if (!this._starActivation) {
40-
// wait for * activation, limited to at most 30s. This is wrapped with
41-
// Promise.resolve() so it doesn't get cancelled early because it is
42-
// shared between consumers.
43-
this._starActivation = Promise.resolve(raceCancellablePromises([
40+
// wait for * activation, limited to at most 30s.
41+
this._starActivation = raceCancellablePromises([
4442
this._extensionService.activateByEvent(`*`),
4543
timeout(30000)
46-
]));
44+
]);
4745
}
48-
return this._starActivation;
46+
47+
// This is wrapped with notCancellablePromise so it doesn't get cancelled
48+
// early because it is shared between consumers.
49+
return notCancellablePromise(this._starActivation);
4950
}
5051

5152
async executeCommand<T>(id: string, ...args: any[]): Promise<T> {
@@ -102,6 +103,11 @@ export class CommandService extends Disposable implements ICommandService {
102103
return Promise.reject(err);
103104
}
104105
}
106+
107+
public override dispose(): void {
108+
super.dispose();
109+
this._starActivation?.cancel();
110+
}
105111
}
106112

107113
registerSingleton(ICommandService, CommandService, InstantiationType.Delayed);

0 commit comments

Comments
 (0)