Allow microtasks to throw exceptions and handle them gracefully

src/isolate.cc

Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index f7605d8d21b346e85a78c557aca0b74d78cb624c..427f76940caf571c946481c2dfd92f98f1a4586f 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -2281,12 +2281,22 @@ void Isolate::RunMicrotasks() {
     for (int i = 0; i < num_tasks; i++) {
       HandleScope scope(this);
       Handle<JSFunction> microtask(JSFunction::cast(queue->get(i)), this);
-      // TODO(adamk): This should ignore/clear exceptions instead of Checking.
-      Execution::Call(this, microtask, factory()->undefined_value(),
-                      0, NULL).Check();
+      Handle<Object> exception;
+      MaybeHandle<Object> result = Execution::TryCall(
+          microtask, factory()->undefined_value(), 0, NULL, &exception);
+      // If execution is terminating, just bail out.
+      if (result.is_null() &&
+          !exception.is_null() &&
+          *exception == heap()->termination_exception()) {
+        // Clear out any remaining callbacks in the queue.
+        heap()->set_microtask_queue(heap()->empty_fixed_array());
+        set_pending_microtask_count(0);
+        goto done;

[Michael Starzinger, 2014/05/22 11:27:59]

Pretty please with a cherry on top, no gotos ... shouldn't a simple break of the
for-loop actually implicitly kick you out of the while-loop as well?

[Yang, 2014/05/22 11:33:26]

Even nicer could be just a return here. Instead of incrementing/decrementing the
call depth at the start and end, why not use a scope? We even have one, called
SuppressMicrotaskExecutionScope. Might be a misnomer here, but a comment or two
should fix that?

[adamk, 2014/05/22 11:49:25]

Hmm, that seems a bit fragile to me, if someone changes the while loop later we
might get into trouble. But I'm happy to get rid of the goto.

Instead I've switched to using the SuppressMicrotaskExecutionScope already
written as part of the V8 API. It's a little weird to use something from the API
here but I feel weirder writing the same code again just to avoid a few
reinterpret_casts. Let me know what you think.

[danno-g, 2014/05/22 13:35:07]

DBC: Please, no goto. Either the explicit return or the simple break is much
preferable to me over the goto.

On 2014/05/22 11:49:25, adamk wrote:

[adamk, 2014/05/22 13:48:50]

No need to worry, the goto is long gone. I added it before the break was even
possible, as jochen had asked about it on the initial path (and I'd said "hmm, I
could use a goto, but I don't see any in V8...").

Sounds like y'all have an email filter that flags goto :)

On 2014/05/22 13:35:07, danno-g wrote:

+      }
     }
   }
 
+done:
   handle_scope_implementer()->DecrementCallDepth();
 }