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

If run with --abort-on-uncaught-exception, V8 will abort the process
whenever it does not see a JS-installed CatchClause in the stack. C++
TryCatch clauses are ignored. Domains work by setting a FatalException
handler which is ignored when running in abort mode.

This patch modifies MakeCallback to call its target function through a
JS function that installs a CatchClause and manually calls _fatalException
on error, if the process is both using domains and is in abort mode.

Semver: patch
PR-URL: #922
Fixes: #836
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

src/async-wrap.cc

@@ -7,6 +7,7 @@
 
 #include "v8.h"
 
+using v8::Array;
 using v8::Context;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -81,6 +82,7 @@ Handle<Value> AsyncWrap::MakeCallback(const Handle<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());
@@ -89,6 +91,7 @@ Handle<Value> AsyncWrap::MakeCallback(const Handle<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();
     }
   }
 
@@ -112,7 +115,21 @@ Handle<Value> AsyncWrap::MakeCallback(const Handle<Function> cb,
     try_catch.SetVerbose(true);
   }
 
-  Local<Value> ret = cb->Call(context, argc, argv);
+  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);
+  }
 
   if (try_catch.HasCaught()) {
     return Undefined(env()->isolate());

src/env-inl.h

@@ -165,6 +165,7 @@ inline Environment::Environment(v8::Local<v8::Context> context,
       isolate_data_(IsolateData::GetOrCreate(context->GetIsolate(), loop)),
       using_smalloc_alloc_cb_(false),
       using_domains_(false),
+      using_abort_on_uncaught_exc_(false),
       using_asyncwrap_(false),
       printed_error_(false),
       debugger_agent_(this),
@@ -283,6 +284,14 @@ inline void Environment::set_using_smalloc_alloc_cb(bool value) {
   using_smalloc_alloc_cb_ = value;
 }
 
+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

@@ -67,6 +67,7 @@ namespace node {
   V(dev_string, "dev")                                                        \
   V(disposed_string, "_disposed")                                             \
   V(domain_string, "domain")                                                  \
+  V(domain_abort_uncaught_exc_string, "_makeCallbackAbortOnUncaught")         \
   V(exchange_string, "exchange")                                              \
   V(idle_string, "idle")                                                      \
   V(irq_string, "irq")                                                        \
@@ -402,6 +403,9 @@ class Environment {
   inline bool using_smalloc_alloc_cb() const;
   inline void set_using_smalloc_alloc_cb(bool value);
 
+  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);
 
@@ -496,6 +500,7 @@ class Environment {
   ares_task_list cares_task_list_;
   bool using_smalloc_alloc_cb_;
   bool using_domains_;
+  bool using_abort_on_uncaught_exc_;
   bool using_asyncwrap_;
   bool printed_error_;
   debugger::Agent debugger_agent_;

src/node.cc

@@ -112,6 +112,7 @@ static bool print_eval = false;
 static bool force_repl = false;
 static bool trace_deprecation = false;
 static bool throw_deprecation = false;
+static bool abort_on_uncaught_exception = false;
 static const char* eval_string = nullptr;
 static bool use_debug_agent = false;
 static bool debug_wait_connect = false;
@@ -3109,6 +3110,9 @@ static void ParseArgs(int* argc,
       trace_deprecation = true;
     } else if (strcmp(arg, "--throw-deprecation") == 0) {
       throw_deprecation = true;
+    } else if (strcmp(arg, "--abort-on-uncaught-exception") == 0 ||
+               strcmp(arg, "--abort_on_uncaught_exception") == 0) {
+      abort_on_uncaught_exception = true;
     } else if (strcmp(arg, "--v8-options") == 0) {
       new_v8_argv[new_v8_argc] = "--help";
       new_v8_argc += 1;
@@ -3789,7 +3793,7 @@ int Start(int argc, char** argv) {
         exec_argc,
         exec_argv);
     Context::Scope context_scope(context);
-
+    env->set_using_abort_on_uncaught_exc(abort_on_uncaught_exception);
     // Start debug agent when argv has --debug
     if (use_debug_agent)
       StartDebug(env, debug_wait_connect);

src/node.js

@@ -209,6 +209,14 @@
   };
 
   startup.processFatal = function() {
+    process._makeCallbackAbortOnUncaught = function() {
+      try {
+        return this[1].apply(this[0], arguments);
+      } catch (err) {
+        process._fatalException(err);
+      }
+    };
+
     process._fatalException = function(er) {
       var caught;
 

test/parallel/test-domain-abort-on-uncaught.js

@@ -0,0 +1,109 @@
+var common = require('../common');
+var assert = require('assert');
+var spawn = require('child_process').spawn;
+
+var tests = [
+  nextTick,
+  timer,
+  timerPlusNextTick,
+  firstRun,
+  netServer
+]
+
+tests.forEach(function(test) {
+  console.log(test.name);
+  var child = spawn(process.execPath, [
+    '--abort-on-uncaught-exception',
+    '-e',
+    '(' + test + ')()',
+    common.PORT
+  ]);
+  child.stderr.pipe(process.stderr);
+  child.stdout.pipe(process.stdout);
+  child.on('exit', function(code) {
+    assert.strictEqual(code, 0);
+  });
+});
+
+function nextTick() {
+  var domain = require('domain');
+  var d = domain.create();
+
+  d.on('error', function(err) {
+    console.log('ok');
+    process.exit(0);
+  });
+  d.run(function() {
+    process.nextTick(function() {
+      throw new Error('exceptional!');
+    });
+  });
+}
+
+function timer() {
+  var domain = require('domain');
+  var d = domain.create();
+
+  d.on('error', function(err) {
+    console.log('ok');
+    process.exit(0);
+  });
+  d.run(function() {
+    setTimeout(function() {
+      throw new Error('exceptional!');
+    }, 33);
+  });
+}
+
+function timerPlusNextTick() {
+  var domain = require('domain');
+  var d = domain.create();
+
+  d.on('error', function(err) {
+    console.log('ok');
+    process.exit(0);
+  });
+  d.run(function() {
+    setTimeout(function() {
+      process.nextTick(function() {
+        throw new Error('exceptional!');
+      });
+    }, 33);
+  });
+}
+
+function firstRun() {
+  var domain = require('domain');
+  var d = domain.create();
+
+  d.on('error', function(err) {
+    console.log('ok');
+    process.exit(0);
+  });
+  d.run(function() {
+    throw new Error('exceptional!');
+  });
+}
+
+function netServer() {
+  var domain = require('domain');
+  var net = require('net');
+  var d = domain.create();
+
+  d.on('error', function(err) {
+    console.log('ok');
+    process.exit(0);
+  });
+  d.run(function() {
+    var server = net.createServer(function(conn) {
+      conn.pipe(conn);
+    });
+    server.listen(Number(process.argv[1]), '0.0.0.0', function() {
+      var conn = net.connect(Number(process.argv[1]), '0.0.0.0')
+      conn.once('data', function() {
+        throw new Error('ok');
+      })
+      conn.end('ok');
+    });
+  });
+}