Skip to content

Commit

Permalink
src: fix domains + --abort-on-uncaught-exception
Browse files Browse the repository at this point in the history
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: nodejs#922
Fixes: nodejs#836
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
chrisdickinson committed Feb 25, 2015
1 parent 2ca22aa commit 0af4c9e
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 2 deletions.
19 changes: 18 additions & 1 deletion src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "v8.h"

using v8::Array;
using v8::Context;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -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());
Expand All @@ -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();
}
}

Expand All @@ -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());
Expand Down
9 changes: 9 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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_;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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_;
Expand Down
6 changes: 5 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
109 changes: 109 additions & 0 deletions test/parallel/test-domain-abort-on-uncaught.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
}

0 comments on commit 0af4c9e

Please sign in to comment.