Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Commit 9ed5712

Browse files
trevorademhevery
authored andcommitted
Fix ZoneAwarePromise.all to resolve at the correct time (#1150)
For ZoneAwarePromise.all, as [implemented](https://github.com/angular/zone.js/blob/7201d44451f6c67eac2b86eca3cbfafde14035d6/lib/common/promise.ts), the `count` variable serves three purposes: 1. Count the number of values passed-in 2. Specify the index of a resolved value in `resolvedValues` 3. Track when all the promises have been resolved. In the event that `value.then` is immediately called, `count` will be decremented at the incorrect time resulting in a potentially negative value or reaching 0 when not all values have actually been resolved. The fix is to be more meticulous about using the correct indices at the correct time and to not overload the count and number of resolved promises. This updated version is more explicit about the purpose of the `unresolvedCount` and `valueIndex` variables. `unresolvedCount` needs to start at 2 to prevent `resolve` from being called too soon in the event that `value.then` calls the callback immediately. The scoping of the index for use in `resolvedValues` is made constant to guarantee the correct index is used. This buggy behavior specifically manifested as an issue in the Monaco editor but most likely occurs elsewhere as well in cases where promises may immediately call into `.then`. Related issue: microsoft/monaco-editor#790 (comment)
1 parent 7201d44 commit 9ed5712

File tree

1 file changed

+25
-12
lines changed

1 file changed

+25
-12
lines changed

Diff for: lib/common/promise.ts

+25-12
Original file line numberDiff line numberDiff line change
@@ -315,24 +315,37 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
315315
resolve = res;
316316
reject = rej;
317317
});
318-
let count = 0;
318+
319+
// Start at 2 to prevent prematurely resolving if .then is called immediately.
320+
let unresolvedCount = 2;
321+
let valueIndex = 0;
322+
319323
const resolvedValues: any[] = [];
320324
for (let value of values) {
321325
if (!isThenable(value)) {
322326
value = this.resolve(value);
323327
}
324-
value.then(
325-
((index) => (value: any) => {
326-
resolvedValues[index] = value;
327-
count--;
328-
if (!count) {
329-
resolve(resolvedValues);
330-
}
331-
})(count),
332-
reject!);
333-
count++;
328+
329+
const curValueIndex = valueIndex;
330+
value.then((value: any) => {
331+
resolvedValues[curValueIndex] = value;
332+
unresolvedCount--;
333+
if (unresolvedCount === 0) {
334+
resolve!(resolvedValues);
335+
}
336+
}, reject!);
337+
338+
unresolvedCount++;
339+
valueIndex++;
334340
}
335-
if (!count) resolve!(resolvedValues);
341+
342+
// Make the unresolvedCount zero-based again.
343+
unresolvedCount -= 2;
344+
345+
if (unresolvedCount === 0) {
346+
resolve!(resolvedValues);
347+
}
348+
336349
return promise;
337350
}
338351

0 commit comments

Comments
 (0)