Skip to content

Commit bb66c3b

Browse files
authored
Make Throttler async (microsoft#185688)
1 parent 4dac9b5 commit bb66c3b

File tree

3 files changed

+51
-16
lines changed

3 files changed

+51
-16
lines changed

src/vs/base/common/async.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,26 +166,36 @@ export interface ITask<T> {
166166
* throttler.queue(deliver);
167167
* }
168168
*/
169-
export class Throttler {
169+
export class Throttler implements IDisposable {
170170

171171
private activePromise: Promise<any> | null;
172172
private queuedPromise: Promise<any> | null;
173173
private queuedPromiseFactory: ITask<Promise<any>> | null;
174174

175+
private isDisposed = false;
176+
175177
constructor() {
176178
this.activePromise = null;
177179
this.queuedPromise = null;
178180
this.queuedPromiseFactory = null;
179181
}
180182

181183
queue<T>(promiseFactory: ITask<Promise<T>>): Promise<T> {
184+
if (this.isDisposed) {
185+
throw new Error('Throttler is disposed');
186+
}
187+
182188
if (this.activePromise) {
183189
this.queuedPromiseFactory = promiseFactory;
184190

185191
if (!this.queuedPromise) {
186192
const onComplete = () => {
187193
this.queuedPromise = null;
188194

195+
if (this.isDisposed) {
196+
return;
197+
}
198+
189199
const result = this.queue(this.queuedPromiseFactory!);
190200
this.queuedPromiseFactory = null;
191201

@@ -214,6 +224,10 @@ export class Throttler {
214224
});
215225
});
216226
}
227+
228+
dispose(): void {
229+
this.isDisposed = true;
230+
}
217231
}
218232

219233
export class Sequencer {

src/vs/base/test/common/async.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,40 @@ suite('Async', () => {
155155

156156
return Promise.all(promises);
157157
});
158+
159+
test('disposal after queueing', async () => {
160+
let factoryCalls = 0;
161+
const factory = async () => {
162+
factoryCalls++;
163+
return async.timeout(0);
164+
};
165+
166+
const throttler = new async.Throttler();
167+
const promises: Promise<any>[] = [];
168+
169+
promises.push(throttler.queue(factory));
170+
promises.push(throttler.queue(factory));
171+
throttler.dispose();
172+
173+
await Promise.all(promises);
174+
assert.strictEqual(factoryCalls, 1);
175+
});
176+
177+
test('disposal before queueing', async () => {
178+
let factoryCalls = 0;
179+
const factory = async () => {
180+
factoryCalls++;
181+
return async.timeout(0);
182+
};
183+
184+
const throttler = new async.Throttler();
185+
const promises: Promise<any>[] = [];
186+
187+
throttler.dispose();
188+
assert.throws(() => promises.push(throttler.queue(factory)));
189+
assert.strictEqual(factoryCalls, 0);
190+
await Promise.all(promises);
191+
});
158192
});
159193

160194
suite('Delayer', function () {

src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/contributedStatusBarItemController.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,8 @@ class CellStatusBarHelper extends Disposable {
7676
private _currentItemLists: INotebookCellStatusBarItemList[] = [];
7777

7878
private _activeToken: CancellationTokenSource | undefined;
79-
private _isDisposed: boolean = false;
8079

81-
private readonly _updateThrottler = new Throttler();
80+
private readonly _updateThrottler = this._register(new Throttler());
8281

8382
constructor(
8483
private readonly _notebookViewModel: INotebookViewModel,
@@ -102,18 +101,7 @@ class CellStatusBarHelper extends Disposable {
102101
private _updateSoon(): void {
103102
// Wait a tick to make sure that the event is fired to the EH before triggering status bar providers
104103
this._register(disposableTimeout(() => {
105-
this._updateThrottler.queue(async () => {
106-
if (this._isDisposed) {
107-
// This order of events can happen
108-
// - Start one update
109-
// - Start a second update, its queued
110-
// - This class is disposed, cancelling the first update
111-
// - The second update runs, and we're disposed. So bail at this point.
112-
return;
113-
}
114-
115-
return this._update();
116-
});
104+
this._updateThrottler.queue(() => this._update());
117105
}, 0));
118106
}
119107

@@ -140,7 +128,6 @@ class CellStatusBarHelper extends Disposable {
140128

141129
override dispose() {
142130
super.dispose();
143-
this._isDisposed = true;
144131
this._activeToken?.dispose(true);
145132

146133
this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items: [] }]);

0 commit comments

Comments
 (0)