Bug 1532265 - Part 2: Handle closed generators in async function resume. r=arai
authorAndré Bargull <andre.bargull@gmail.com>
Wed, 06 Mar 2019 16:24:35 +0000
changeset 520572 413190773025f4c6ea37fd32914ae8c8cc98a1cc
parent 520571 21c52ca0266ebfda36480f1e6acd961f7d43b0dd
child 520573 1f0368105e07f22633e4ea0d0dfe8becccb8bdc0
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1532265
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1532265 - Part 2: Handle closed generators in async function resume. r=arai Debugger or OOM errors can close the generator after JSOP_ASYNCAWAIT enqueued a promise job for AsyncFunctionResume. Change AsyncFunctionResume to handle this case and also try to reject the result promise with the pending OOM error if possible. Differential Revision: https://phabricator.services.mozilla.com/D22301
js/src/jit-test/tests/basic/bug1532265.js
js/src/jit-test/tests/debug/onEnterFrame-async-resumption-08.js
js/src/vm/AsyncFunction.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1532265.js
@@ -0,0 +1,33 @@
+// |jit-test| allow-oom; skip-if: !('oomTest' in this)
+
+ignoreUnhandledRejections();
+
+var v = {}
+async function f() {
+    // Increasing level of stack size during await to make OOM more likely when
+    // saving the stack state.
+              [await v];
+             [[await v]];
+            [[[await v]]];
+           [[[[await v]]]];
+          [[[[[await v]]]]];
+         [[[[[[await v]]]]]];
+        [[[[[[[await v]]]]]]];
+       [[[[[[[[await v]]]]]]]];
+      [[[[[[[[[await v]]]]]]]]];
+     [[[[[[[[[[await v]]]]]]]]]];
+}
+
+oomTest(function() {
+    for (var i = 0; i < 8; ++i) {
+        f();
+    }
+
+    // Drain all jobs, ignoring any OOM errors.
+    while (true) {
+        try {
+            drainJobQueue();
+            break;
+        } catch {}
+    }
+});
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-08.js
@@ -0,0 +1,53 @@
+// Terminate execution from within the onPop handler, the result promise will
+// never be resolved.
+
+load(libdir + "array-compare.js");
+
+let g = newGlobal({newCompartment: true});
+let dbg = new Debugger(g);
+
+let log = [];
+g.log = log;
+
+g.eval(`
+    async function f() {
+        log.push("START");
+        await 0;
+        log.push("MIDDLE");
+        await 0;
+        log.push("END");
+    }
+`);
+
+const expectedTick1 = ["START"];
+const expectedTickN = [
+    "START",
+    "enter: f",
+    "MIDDLE",
+    "pop: f",
+];
+
+Promise.resolve(0)
+.then(() => assertEq(arraysEqual(log, expectedTick1), true))
+.then(() => assertEq(arraysEqual(log, expectedTickN), true))
+.then(() => assertEq(arraysEqual(log, expectedTickN), true))
+.then(() => assertEq(arraysEqual(log, expectedTickN), true))
+.then(() => assertEq(arraysEqual(log, expectedTickN), true))
+.catch(e => {
+    // Quit with non-zero exit code to ensure a test suite error is shown,
+    // even when this function is called within promise handlers which normally
+    // swallow any exceptions.
+    print("Error: " + e + "\nstack:\n" + e.stack);
+    quit(1);
+});
+
+g.f();
+
+dbg.onEnterFrame = frame => {
+    log.push(`enter: ${frame.callee.name}`);
+
+    frame.onPop = () => {
+        log.push(`pop: ${frame.callee.name}`);
+        return null; // Terminate execution.
+    };
+};
--- a/js/src/vm/AsyncFunction.cpp
+++ b/js/src/vm/AsyncFunction.cpp
@@ -67,47 +67,63 @@ enum class ResumeKind { Normal, Throw };
 // Async Functions proposal 2.2 steps 3.f, 3.g.
 // Async Functions proposal 2.2 steps 3.d-e, 3.g.
 // Implemented in js/src/builtin/Promise.cpp
 
 // Async Functions proposal 2.2 steps 3-8, 2.4 steps 2-7, 2.5 steps 2-7.
 static bool AsyncFunctionResume(JSContext* cx,
                                 Handle<AsyncFunctionGeneratorObject*> generator,
                                 ResumeKind kind, HandleValue valueOrReason) {
+  // We're enqueuing the promise job for Await before suspending the execution
+  // of the async function. So when either the debugger or OOM errors terminate
+  // the execution after JSOP_ASYNCAWAIT, but before JSOP_AWAIT, we're in an
+  // inconsistent state, because we don't have a resume index set and therefore
+  // don't know where to resume the async function. Return here in that case.
+  if (generator->isClosed()) {
+    return true;
+  }
+
   Rooted<PromiseObject*> resultPromise(cx, generator->promise());
 
   RootedObject stack(cx);
   Maybe<JS::AutoSetAsyncStackForNewCalls> asyncStack;
   if (JSObject* allocationSite = resultPromise->allocationSite()) {
     // The promise is created within the activation of the async function, so
     // use the parent frame as the starting point for async stacks.
     stack = allocationSite->as<SavedFrame>().getParent();
     if (stack) {
       asyncStack.emplace(
           cx, stack, "async",
           JS::AutoSetAsyncStackForNewCalls::AsyncCallKind::EXPLICIT);
     }
   }
 
-  MOZ_ASSERT(!generator->isClosed(),
-             "closed generator when resuming async function");
   MOZ_ASSERT(generator->isSuspended(),
              "non-suspended generator when resuming async function");
 
   // Execution context switching is handled in generator.
   HandlePropertyName funName = kind == ResumeKind::Normal
                                    ? cx->names().AsyncFunctionNext
                                    : cx->names().AsyncFunctionThrow;
   FixedInvokeArgs<1> args(cx);
   args[0].set(valueOrReason);
   RootedValue generatorOrValue(cx, ObjectValue(*generator));
   if (!CallSelfHostedFunction(cx, funName, generatorOrValue, args,
                               &generatorOrValue)) {
     if (!generator->isClosed()) {
       generator->setClosed();
+
+      // Handle the OOM case mentioned above.
+      if (cx->isExceptionPending()) {
+        RootedValue exn(cx);
+        if (!GetAndClearException(cx, &exn)) {
+          return false;
+        }
+        return AsyncFunctionThrown(cx, resultPromise, exn);
+      }
     }
     return false;
   }
 
   MOZ_ASSERT_IF(generator->isClosed(), generatorOrValue.isObject());
   MOZ_ASSERT_IF(generator->isClosed(),
                 &generatorOrValue.toObject() == resultPromise);
   MOZ_ASSERT_IF(!generator->isClosed(), generator->isAfterAwait());