src: fix --abort-on-uncaught-exception

deps/v8/include/v8.h

@@ -5380,6 +5380,19 @@ class V8_EXPORT Isolate {
    */
   static Isolate* GetCurrent();
 
+  /**
+   * Custom callback used by embedders to help V8 determine if it should abort
+   * when it throws and no internal handler is predicted to catch the
+   * exception. If --abort-on-uncaught-exception is used on the command line,
+   * then V8 will abort if either:
+   * - no custom callback is set.
+   * - the custom callback set returns true.
+   * Otherwise, the custom callback will not be called and V8 will not abort.
+   */
+  typedef bool (*AbortOnUncaughtExceptionCallback)(Isolate*);
+  void SetAbortOnUncaughtExceptionCallback(
+      AbortOnUncaughtExceptionCallback callback);
+
   /**
    * Methods below this point require holding a lock (using Locker) in
    * a multi-threaded environment.

deps/v8/src/api.cc

@@ -7176,6 +7176,13 @@ void Isolate::Exit() {
 }
 
 
+void Isolate::SetAbortOnUncaughtExceptionCallback(
+    AbortOnUncaughtExceptionCallback callback) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->SetAbortOnUncaughtExceptionCallback(callback);
+}
+
+
 Isolate::DisallowJavascriptExecutionScope::DisallowJavascriptExecutionScope(
     Isolate* isolate,
     Isolate::DisallowJavascriptExecutionScope::OnFailure on_failure)

deps/v8/src/isolate.cc

@@ -1013,13 +1013,21 @@ Object* Isolate::Throw(Object* exception, MessageLocation* location) {
       Handle<Object> message_obj = CreateMessage(exception_handle, location);
       thread_local_top()->pending_message_obj_ = *message_obj;
 
-      // If the abort-on-uncaught-exception flag is specified, abort on any
-      // exception not caught by JavaScript, even when an external handler is
-      // present.  This flag is intended for use by JavaScript developers, so
-      // print a user-friendly stack trace (not an internal one).
+      // For any exception not caught by JavaScript, even when an external
+      // handler is present:
+      // If the abort-on-uncaught-exception flag is specified, and if the
+      // embedder didn't specify a custom uncaught exception callback,
+      // or if the custom callback determined that V8 should abort, then
+      // abort.
       if (FLAG_abort_on_uncaught_exception &&
-          PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT) {
-        FLAG_abort_on_uncaught_exception = false;  // Prevent endless recursion.
+          PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT &&
+          (!abort_on_uncaught_exception_callback_ ||
+           abort_on_uncaught_exception_callback_(
+               reinterpret_cast<v8::Isolate*>(this)))) {
+        // Prevent endless recursion.
+        FLAG_abort_on_uncaught_exception = false;
+        // This flag is intended for use by JavaScript developers, so
+        // print a user-friendly stack trace (not an internal one).
         PrintF(stderr, "%s\n\nFROM\n",
                MessageHandler::GetLocalizedMessage(this, message_obj).get());
         PrintCurrentStackTrace(stderr);
@@ -1612,6 +1620,12 @@ void Isolate::SetCaptureStackTraceForUncaughtExceptions(
 }
 
 
+void Isolate::SetAbortOnUncaughtExceptionCallback(
+    v8::Isolate::AbortOnUncaughtExceptionCallback callback) {
+  abort_on_uncaught_exception_callback_ = callback;
+}
+
+
 Handle<Context> Isolate::native_context() {
   return handle(context()->native_context());
 }
@@ -1782,7 +1796,8 @@ Isolate::Isolate(bool enable_serializer)
       next_unique_sfi_id_(0),
 #endif
       use_counter_callback_(NULL),
-      basic_block_profiler_(NULL) {
+      basic_block_profiler_(NULL),
+      abort_on_uncaught_exception_callback_(NULL) {
   {
     base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
     CHECK(thread_data_table_);

deps/v8/src/isolate.h

@@ -690,6 +690,9 @@ class Isolate {
       int frame_limit,
       StackTrace::StackTraceOptions options);
 
+  void SetAbortOnUncaughtExceptionCallback(
+      v8::Isolate::AbortOnUncaughtExceptionCallback callback);
+
   enum PrintStackMode { kPrintStackConcise, kPrintStackVerbose };
   void PrintCurrentStackTrace(FILE* out);
   void PrintStack(StringStream* accumulator,
@@ -1363,6 +1366,9 @@ class Isolate {
 
   v8::ArrayBuffer::Allocator* array_buffer_allocator_;
 
+  v8::Isolate::AbortOnUncaughtExceptionCallback
+      abort_on_uncaught_exception_callback_;
+
   friend class ExecutionAccess;
   friend class HandleScopeImplementer;
   friend class OptimizingCompileDispatcher;

deps/v8/test/cctest/test-api.cc

@@ -21880,3 +21880,34 @@ TEST(CompatibleReceiverCheckOnCachedICHandler) {
       "result;\n",
       0);
 }
+
+
+static int nb_uncaught_exception_callback_calls = 0;
+
+
+bool NoAbortOnUncaughtException(v8::Isolate* isolate) {
+  ++nb_uncaught_exception_callback_calls;
+  return false;
+}
+
+
+TEST(AbortOnUncaughtExceptionNoAbort) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Handle<v8::ObjectTemplate> global_template =
+      v8::ObjectTemplate::New(isolate);
+  LocalContext env(NULL, global_template);
+
+  i::FLAG_abort_on_uncaught_exception = true;
+  isolate->SetAbortOnUncaughtExceptionCallback(NoAbortOnUncaughtException);
+
+  CompileRun("function boom() { throw new Error(\"boom\") }");
+
+  v8::Local<v8::Object> global_object = env->Global();
+  v8::Local<v8::Function> foo =
+      v8::Local<v8::Function>::Cast(global_object->Get(v8_str("boom")));
+
+  foo->Call(global_object, 0, NULL);
+
+  CHECK_EQ(1, nb_uncaught_exception_callback_calls);
+}

lib/domain.js

@@ -59,6 +59,20 @@ Domain.prototype._disposed = undefined;
 // Called by process._fatalException in case an error was thrown.
 Domain.prototype._errorHandler = function errorHandler(er) {
   var caught = false;
+  var self = this;
+
+  function emitError() {
+    var handled = self.emit('error', er);
+
+    // Exit all domains on the stack.  Uncaught exceptions end the
+    // current tick and no domains should be left on the stack
+    // between ticks.
+    stack.length = 0;
+    exports.active = process.domain = null;
+
+    return handled;
+  }
+
   // ignore errors on disposed domains.
   //
   // XXX This is a bit stupid.  We should probably get rid of
@@ -71,38 +85,54 @@ Domain.prototype._errorHandler = function errorHandler(er) {
     er.domain = this;
     er.domainThrown = true;
   }
-  // wrap this in a try/catch so we don't get infinite throwing
-  try {
-    // One of three things will happen here.
-    //
-    // 1. There is a handler, caught = true
-    // 2. There is no handler, caught = false
-    // 3. It throws, caught = false
-    //
-    // If caught is false after this, then there's no need to exit()
-    // the domain, because we're going to crash the process anyway.
-    caught = this.emit('error', er);
 
-    // Exit all domains on the stack.  Uncaught exceptions end the
-    // current tick and no domains should be left on the stack
-    // between ticks.
-    stack.length = 0;
-    exports.active = process.domain = null;
-  } catch (er2) {
-    // The domain error handler threw!  oh no!
-    // See if another domain can catch THIS error,
-    // or else crash on the original one.
-    // If the user already exited it, then don't double-exit.
-    if (this === exports.active) {
-      stack.pop();
+  // The top-level domain-handler is handled separately.
+  //
+  // The reason is that if V8 was passed a command line option
+  // asking it to abort on an uncaught exception (currently
+  // "--abort-on-uncaught-exception"), we want an uncaught exception
+  // in the top-level domain error handler to make the
+  // process abort. Using try/catch here would always make V8 think
+  // that these exceptions are caught, and thus would prevent it from
+  // aborting in these cases.
+  if (stack.length === 1) {
+    try {
+      // Set the _emittingTopLevelDomainError so that we know that, even
+      // if technically the top-level domain is still active, it would
+      // be ok to abort on an uncaught exception at this point
+      process._emittingTopLevelDomainError = true;
+      caught = emitError();
+    } finally {
+      process._emittingTopLevelDomainError = false;
     }
-    if (stack.length) {
-      exports.active = process.domain = stack[stack.length - 1];
-      caught = process._fatalException(er2);
-    } else {
-      caught = false;
+  } else {
+    // wrap this in a try/catch so we don't get infinite throwing
+    try {
+      // One of three things will happen here.
+      //
+      // 1. There is a handler, caught = true
+      // 2. There is no handler, caught = false
+      // 3. It throws, caught = false
+      //
+      // If caught is false after this, then there's no need to exit()
+      // the domain, because we're going to crash the process anyway.
+      caught = emitError();
+    } catch (er2) {
+      // The domain error handler threw!  oh no!
+      // See if another domain can catch THIS error,
+      // or else crash on the original one.
+      // If the user already exited it, then don't double-exit.
+      if (this === exports.active) {
+        stack.pop();
+      }
+      if (stack.length) {
+        exports.active = process.domain = stack[stack.length - 1];
+        caught = process._fatalException(er2);
+      } else {
+        caught = false;
+      }
+      return caught;
     }
-    return caught;
   }
   return caught;
 };

src/async-wrap.cc

@@ -168,7 +168,6 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   Local<Object> process = env()->process_object();
   Local<Object> domain;
   bool has_domain = false;
-  bool has_abort_on_uncaught_and_domains = false;
 
   if (env()->using_domains()) {
     Local<Value> domain_v = context->Get(env()->domain_string());
@@ -177,7 +176,6 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
       domain = domain_v.As<Object>();
       if (domain->Get(env()->disposed_string())->IsTrue())
         return Undefined(env()->isolate());
-      has_abort_on_uncaught_and_domains = env()->using_abort_on_uncaught_exc();
     }
   }
 
@@ -201,21 +199,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
     try_catch.SetVerbose(true);
   }
 
-  Local<Value> ret;
-
-  if (has_abort_on_uncaught_and_domains) {
-    Local<Value> fn = process->Get(env()->domain_abort_uncaught_exc_string());
-    if (fn->IsFunction()) {
-      Local<Array> special_context = Array::New(env()->isolate(), 2);
-      special_context->Set(0, context);
-      special_context->Set(1, cb);
-      ret = fn.As<Function>()->Call(special_context, argc, argv);
-    } else {
-      ret = cb->Call(context, argc, argv);
-    }
-  } else {
-    ret = cb->Call(context, argc, argv);
-  }
+  Local<Value> ret = cb->Call(context, argc, argv);
 
   if (try_catch.HasCaught()) {
     return Undefined(env()->isolate());

src/env-inl.h

@@ -208,7 +208,6 @@ inline Environment::Environment(v8::Local<v8::Context> context,
       isolate_data_(IsolateData::GetOrCreate(context->GetIsolate(), loop)),
       timer_base_(uv_now(loop)),
       using_domains_(false),
-      using_abort_on_uncaught_exc_(false),
       using_asyncwrap_(false),
       printed_error_(false),
       trace_sync_io_(false),
@@ -334,14 +333,6 @@ inline uint64_t Environment::timer_base() const {
   return timer_base_;
 }
 
-inline bool Environment::using_abort_on_uncaught_exc() const {
-  return using_abort_on_uncaught_exc_;
-}
-
-inline void Environment::set_using_abort_on_uncaught_exc(bool value) {
-  using_abort_on_uncaught_exc_ = value;
-}
-
 inline bool Environment::using_domains() const {
   return using_domains_;
 }

src/env.h

@@ -69,7 +69,7 @@ namespace node {
   V(dev_string, "dev")                                                        \
   V(disposed_string, "_disposed")                                             \
   V(domain_string, "domain")                                                  \
-  V(domain_abort_uncaught_exc_string, "_makeCallbackAbortOnUncaught")         \
+  V(emitting_top_level_domain_error_string, "_emittingTopLevelDomainError")   \
   V(exchange_string, "exchange")                                              \
   V(idle_string, "idle")                                                      \
   V(irq_string, "irq")                                                        \
@@ -431,9 +431,6 @@ class Environment {
   inline ares_channel* cares_channel_ptr();
   inline ares_task_list* cares_task_list();
 
-  inline bool using_abort_on_uncaught_exc() const;
-  inline void set_using_abort_on_uncaught_exc(bool value);
-
   inline bool using_domains() const;
   inline void set_using_domains(bool value);
 
@@ -538,7 +535,6 @@ class Environment {
   ares_channel cares_channel_;
   ares_task_list cares_task_list_;
   bool using_domains_;
-  bool using_abort_on_uncaught_exc_;
   bool using_asyncwrap_;
   bool printed_error_;
   bool trace_sync_io_;