From 0df0d9690b9637e23582cd94c860e9d9c306704f Mon Sep 17 00:00:00 2001
From: Trevor Florence <trevorade@users.noreply.github.com>
Date: Tue, 23 Oct 2018 13:12:14 -0600
Subject: [PATCH] Fix ZoneAwarePromise.all to resolve at the correct time

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:
https://github.com/Microsoft/monaco-editor/issues/790#issuecomment-378452532
---
 lib/common/promise.ts | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/lib/common/promise.ts b/lib/common/promise.ts
index ce765a260..aa1f44757 100644
--- a/lib/common/promise.ts
+++ b/lib/common/promise.ts
@@ -315,24 +315,37 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
         resolve = res;
         reject = rej;
       });
-      let count = 0;
+
+      // Start at 2 to prevent prematurely resolving if .then is called immediately.
+      let unresolvedCount = 2;
+      let valueIndex = 0;
+
       const resolvedValues: any[] = [];
       for (let value of values) {
         if (!isThenable(value)) {
           value = this.resolve(value);
         }
-        value.then(
-            ((index) => (value: any) => {
-              resolvedValues[index] = value;
-              count--;
-              if (!count) {
-                resolve(resolvedValues);
-              }
-            })(count),
-            reject!);
-        count++;
+
+        const curValueIndex = valueIndex;
+        value.then((value: any) => {
+          resolvedValues[curValueIndex] = value;
+          unresolvedCount--;
+          if (unresolvedCount === 0) {
+            resolve!(resolvedValues);
+          }
+        }, reject!);
+
+        unresolvedCount++;
+        valueIndex++;
       }
-      if (!count) resolve!(resolvedValues);
+
+      // Make the unresolvedCount zero-based again.
+      unresolvedCount -= 2;
+
+      if (unresolvedCount === 0) {
+        resolve!(resolvedValues);
+      }
+
       return promise;
     }