Allow microtasks to throw exceptions and handle them gracefully

src/isolate.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdlib.h>
 
 #include "v8.h"
 
 #include "ast.h"
 #include "bootstrapper.h"
@@ skipping 2263 matching lines @@
     HandleScope scope(this);
     int num_tasks = pending_microtask_count();
     Handle<FixedArray> queue(heap()->microtask_queue(), this);
     ASSERT(num_tasks <= queue->length());
     set_pending_microtask_count(0);
     heap()->set_microtask_queue(heap()->empty_fixed_array());
 
     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();
 }
 
 
 } }  // namespace v8::internal