Async/await Promise dependency graph

src/js/promise.js

Index: src/js/promise.js
diff --git a/src/js/promise.js b/src/js/promise.js
index e020028c244f8f0d45bf429316ae916757012de5..1a626c2c55ff9317b6ff2b25b58b565ffe347888 100644
--- a/src/js/promise.js
+++ b/src/js/promise.js
@@ -12,6 +12,7 @@
 // Imports
 
 var InternalArray = utils.InternalArray;
+var GlobalSet = global.Set;
 var promiseAwaitHandlerSymbol =
     utils.ImportNow("promise_await_handler_symbol");
 var promiseCombinedDeferredSymbol =
@@ -29,12 +30,16 @@ var promiseHandledHintSymbol =
 var promiseRawSymbol = utils.ImportNow("promise_raw_symbol");
 var promiseStateSymbol = utils.ImportNow("promise_state_symbol");
 var promiseResultSymbol = utils.ImportNow("promise_result_symbol");
+var SetAdd;
+var SetHas;
 var SpeciesConstructor;
 var speciesSymbol = utils.ImportNow("species_symbol");
 var toStringTagSymbol = utils.ImportNow("to_string_tag_symbol");
 
 utils.Import(function(from) {
   SpeciesConstructor = from.SpeciesConstructor;
+  SetAdd = from.SetAdd;

[adamk, 2016/09/15 22:39:40]

Nit: sort

[Dan Ehrenberg, 2016/09/17 00:04:31]

Done

+  SetHas = from.SetHas;
 });
 
 // -------------------------------------------------------------------
@@ -236,7 +241,7 @@ function IsPromise(x) {
 }
 
 function PromiseCreate() {
-  return new GlobalPromise(PromiseNopResolver)
+  return new GlobalPromise(PromiseNopResolver);
 }
 
 // ES#sec-promise-resolve-functions
@@ -287,6 +292,11 @@ function ResolvePromise(promise, resolution) {
       var id;
       var name = "PromiseResolveThenableJob";
       var instrumenting = DEBUG_IS_ACTIVE;
+      if (instrumenting && !IS_UNDEFINED(resolution) &&

[adamk, 2016/09/15 22:39:40]

No need to check !IS_UNDEFINED if you're checking IsPromise (which calls
IS_RECEIVER).

[Dan Ehrenberg, 2016/09/17 00:04:31]

Done

+          IsPromise(resolution)) {
+        // Mark the dependency of the new promise on the resolution
+        SET_PRIVATE(resolution, promiseAwaitHandlerSymbol, promise);
+      }
       %EnqueueMicrotask(function() {
         if (instrumenting) {
           %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name });
@@ -310,7 +320,8 @@ function ResolvePromise(promise, resolution) {
       return;
     }
   }
-  FulfillPromise(promise, kFulfilled, resolution, promiseFulfillReactionsSymbol);
+  FulfillPromise(promise, kFulfilled, resolution,
+                 promiseFulfillReactionsSymbol);
 }
 
 // ES#sec-rejectpromise
@@ -536,38 +547,64 @@ function PromiseRace(iterable) {
 
 // Utility for debugger
 
-function PromiseHasUserDefinedRejectHandlerCheck(handler, deferred) {
-  // If this handler was installed by async/await, it does not indicate
-  // that there is a user-defined reject handler.
-  if (GET_PRIVATE(handler, promiseAwaitHandlerSymbol)) return false;
+function PromiseHasUserDefinedRejectHandlerCheck(handler, deferred, visited) {
+  // If this handler was installed by a locally uncaught await, recurse
+  // up to the outer Promise returned by that async function.
+  // In this case, the dependency subsumes any other things attached to the
+  // handler, as the dependency is only present due to async/await and is not
+  // a real catch handler.
+  var outerPromise = GET_PRIVATE(handler, promiseAwaitHandlerSymbol);
+  if (outerPromise) {
+    return PromiseHasUserDefinedRejectHandlerRecursive(outerPromise, visited);
+  }
   if (handler !== PromiseIdRejectHandler) {
     var combinedDeferred = GET_PRIVATE(handler, promiseCombinedDeferredSymbol);
     if (IS_UNDEFINED(combinedDeferred)) return true;
-    if (PromiseHasUserDefinedRejectHandlerRecursive(combinedDeferred.promise)) {
+    if (PromiseHasUserDefinedRejectHandlerRecursive(combinedDeferred.promise,
+                                                    visited)) {
       return true;
     }
-  } else if (PromiseHasUserDefinedRejectHandlerRecursive(deferred.promise)) {
+  } else if (PromiseHasUserDefinedRejectHandlerRecursive(deferred.promise,
+                                                         visited)) {
     return true;
   }
   return false;
 }
 
-function PromiseHasUserDefinedRejectHandlerRecursive(promise) {
+function PromiseHasUserDefinedRejectHandlerRecursive(promise, visited) {
+  // Avoid visiting the Promise multiple times in case there is a cycle
+  // in the dependency graph.
+  if (%_Call(SetHas, visited, promise)) return false;

[adamk, 2016/09/15 22:39:40]

My brain is having trouble groking all the tests; are there tests covering this
cycle-handling?

+  %_Call(SetAdd, visited, promise);
+
   // If this promise was marked as being handled by a catch block
   // in an async function, then it has a user-defined reject handler.
   if (GET_PRIVATE(promise, promiseHandledHintSymbol)) return true;
 
+  // If this Promise is subsumed by another Promise (a Promise resolved
+  // with another Promise, or an intermediate, hidden, throwaway Promise
+  // within async/await), then recurse on the outer Promise.
+  // In this case, the dependency is one possible way that the Promise
+  // could be resolved, so it does not subsume the other following cases.
+  var outerPromise = GET_PRIVATE(promise, promiseAwaitHandlerSymbol);
+  if (outerPromise &&
+      PromiseHasUserDefinedRejectHandlerRecursive(outerPromise, visited)) {
+    return true;
+  }
+
   var queue = GET_PRIVATE(promise, promiseRejectReactionsSymbol);
   var deferreds = GET_PRIVATE(promise, promiseDeferredReactionsSymbol);
 
   if (IS_UNDEFINED(queue)) return false;
 
   if (!IS_ARRAY(queue)) {
-    return PromiseHasUserDefinedRejectHandlerCheck(queue, deferreds);
+    return PromiseHasUserDefinedRejectHandlerCheck(queue, deferreds, visited);
   }
 
   for (var i = 0; i < queue.length; i += 2) {
-    if (PromiseHasUserDefinedRejectHandlerCheck(queue[i], queue[i + 1])) {
+    if (PromiseHasUserDefinedRejectHandlerCheck(queue[i],
+                                                queue[i + 1],
+                                                visited)) {
       return true;
     }
   }
@@ -577,8 +614,11 @@ function PromiseHasUserDefinedRejectHandlerRecursive(promise) {
 // Return whether the promise will be handled by a user-defined reject
 // handler somewhere down the promise chain. For this, we do a depth-first
 // search for a reject handler that's not the default PromiseIdRejectHandler.
+// This function also traverses dependencies of one Promise on another,
+// set up through async/await and Promises resolved with Promises. The graph
+// may contain cycles, so a set of visited Promises is maintained.
 function PromiseHasUserDefinedRejectHandler() {
-  return PromiseHasUserDefinedRejectHandlerRecursive(this);
+  return PromiseHasUserDefinedRejectHandlerRecursive(this, new GlobalSet());
 };