From 7ccdd2cbf45a87dea76f3f3e3ac83e37630a9ddf Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 14:54:25 -0800 Subject: [PATCH 1/7] 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. Fixes: https://github.com/iojs/io.js/issues/836 --- src/async-wrap.cc | 15 ++++- src/env-inl.h | 9 +++ src/env.h | 5 ++ src/node.cc | 6 +- src/node.js | 8 +++ test/parallel/test-abort-on-uncaught.js | 84 +++++++++++++++++++++++++ 6 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-abort-on-uncaught.js diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 7887caf4f5b027..e2e68c1eb0aaf5 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -81,6 +81,7 @@ Handle AsyncWrap::MakeCallback(const Handle cb, Local process = env()->process_object(); Local domain; bool has_domain = false; + bool has_abort_on_uncaught_and_domains = false; if (env()->using_domains()) { Local domain_v = context->Get(env()->domain_string()); @@ -89,6 +90,7 @@ Handle AsyncWrap::MakeCallback(const Handle cb, domain = domain_v.As(); 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 +114,18 @@ Handle AsyncWrap::MakeCallback(const Handle cb, try_catch.SetVerbose(true); } - Local ret = cb->Call(context, argc, argv); + Local ret; + + if (has_abort_on_uncaught_and_domains) { + Local fn = context->Get(env()->domain_abort_uncaught_exc_string()); + if (fn->IsFunction()) { + ret = fn.As()->Call(cb, argc, argv); + } else { + ret = cb->Call(context, argc, argv); + } + } else { + ret = cb->Call(context, argc, argv); + } if (try_catch.HasCaught()) { return Undefined(env()->isolate()); diff --git a/src/env-inl.h b/src/env-inl.h index abe2a5a5260dd7..d3a723a0c246e5 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -165,6 +165,7 @@ inline Environment::Environment(v8::Local 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_; } diff --git a/src/env.h b/src/env.h index ccacbb09f52c2f..9d1290e81e31c9 100644 --- a/src/env.h +++ b/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") \ @@ -392,6 +393,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); @@ -486,6 +490,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_; diff --git a/src/node.cc b/src/node.cc index e060304bffc25c..41b828a48d3d9e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -110,6 +110,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; @@ -3053,6 +3054,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; @@ -3733,7 +3737,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); diff --git a/src/node.js b/src/node.js index f0bee17ef26758..eeedc61a09de04 100644 --- a/src/node.js +++ b/src/node.js @@ -208,6 +208,14 @@ }; startup.processFatal = function() { + process._makeCallbackAbortOnUncaught = function() { + try { + return this.apply(null, arguments); + } catch (err) { + process._fatalException(er); + } + }; + process._fatalException = function(er) { var caught; diff --git a/test/parallel/test-abort-on-uncaught.js b/test/parallel/test-abort-on-uncaught.js new file mode 100644 index 00000000000000..a28f4f683f01e0 --- /dev/null +++ b/test/parallel/test-abort-on-uncaught.js @@ -0,0 +1,84 @@ +var common = require('../common'); +var assert = require('assert'); +var spawn = require('child_process').spawn; + +var tests = [ + nextTick, + timer, + timerPlusNextTick, + firstRun +] + +tests.forEach(function(test) { + console.log(test.name); + var child = spawn(process.execPath, [ + '--abort-on-uncaught-exception', + '-e', + '(' + test+ ')()' + ]); + 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!'); + }); +} From 10e8bc61cfd1a7629cedc3a60adce93202b3fe01 Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 15:42:16 -0800 Subject: [PATCH 2/7] add netServer test --- test/parallel/test-abort-on-uncaught.js | 29 +++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-abort-on-uncaught.js b/test/parallel/test-abort-on-uncaught.js index a28f4f683f01e0..52864544aec483 100644 --- a/test/parallel/test-abort-on-uncaught.js +++ b/test/parallel/test-abort-on-uncaught.js @@ -6,7 +6,8 @@ var tests = [ nextTick, timer, timerPlusNextTick, - firstRun + firstRun, + netServer ] tests.forEach(function(test) { @@ -14,7 +15,8 @@ tests.forEach(function(test) { var child = spawn(process.execPath, [ '--abort-on-uncaught-exception', '-e', - '(' + test+ ')()' + '(' + test+ ')()', + common.PORT ]); child.stderr.pipe(process.stderr); child.stdout.pipe(process.stdout); @@ -82,3 +84,26 @@ function firstRun() { 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'); + }); + }); +} From b67ed52d212d6bae2301dcb5e112b38141e9973f Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 15:42:54 -0800 Subject: [PATCH 3/7] rename test --- ...test-abort-on-uncaught.js => test-domain-abort-on-uncaught.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/parallel/{test-abort-on-uncaught.js => test-domain-abort-on-uncaught.js} (100%) diff --git a/test/parallel/test-abort-on-uncaught.js b/test/parallel/test-domain-abort-on-uncaught.js similarity index 100% rename from test/parallel/test-abort-on-uncaught.js rename to test/parallel/test-domain-abort-on-uncaught.js From 43bed4d69dfbd37ba50dcd95a102643aa720eafd Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 15:44:44 -0800 Subject: [PATCH 4/7] preserve original context --- src/async-wrap.cc | 6 +++++- src/node.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index e2e68c1eb0aaf5..36414018938013 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -7,6 +7,7 @@ #include "v8.h" +using v8::Array; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; @@ -119,7 +120,10 @@ Handle AsyncWrap::MakeCallback(const Handle cb, if (has_abort_on_uncaught_and_domains) { Local fn = context->Get(env()->domain_abort_uncaught_exc_string()); if (fn->IsFunction()) { - ret = fn.As()->Call(cb, argc, argv); + Local specialContext; + specialContext->Set(0, context); + specialContext->Set(1, cb); + ret = fn.As()->Call(specialContext, argc, argv); } else { ret = cb->Call(context, argc, argv); } diff --git a/src/node.js b/src/node.js index eeedc61a09de04..de3191606887db 100644 --- a/src/node.js +++ b/src/node.js @@ -210,7 +210,7 @@ startup.processFatal = function() { process._makeCallbackAbortOnUncaught = function() { try { - return this.apply(null, arguments); + return this[1].apply(this[0], arguments); } catch (err) { process._fatalException(er); } From 6c55bae3a9bbf7ecad3ff2dd652c3cdb16fda58f Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 15:48:40 -0800 Subject: [PATCH 5/7] fix typo --- src/node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.js b/src/node.js index de3191606887db..e3111d9ee120fb 100644 --- a/src/node.js +++ b/src/node.js @@ -212,7 +212,7 @@ try { return this[1].apply(this[0], arguments); } catch (err) { - process._fatalException(er); + process._fatalException(err); } }; From ecc033a96a0f21e6f98a974451b8cf7cf1ea76d4 Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Sun, 22 Feb 2015 16:28:58 -0800 Subject: [PATCH 6/7] actually execute the code! --- src/async-wrap.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 36414018938013..31a476dd3d013a 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -118,12 +118,12 @@ Handle AsyncWrap::MakeCallback(const Handle cb, Local ret; if (has_abort_on_uncaught_and_domains) { - Local fn = context->Get(env()->domain_abort_uncaught_exc_string()); + Local fn = process->Get(env()->domain_abort_uncaught_exc_string()); if (fn->IsFunction()) { - Local specialContext; + Local specialContext = Array::New(env()->isolate(), 2); specialContext->Set(0, context); specialContext->Set(1, cb); - ret = fn.As()->Call(specialContext, argc, argv); + ret = fn.As()->Call(specialContext.As(), argc, argv); } else { ret = cb->Call(context, argc, argv); } From 2e1332b77ca64fa0c66d31125aca6187c6e791f2 Mon Sep 17 00:00:00 2001 From: Chris Dickinson Date: Tue, 24 Feb 2015 13:22:40 -0800 Subject: [PATCH 7/7] fix per @bnoordhuis review --- src/async-wrap.cc | 8 ++++---- test/parallel/test-domain-abort-on-uncaught.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 31a476dd3d013a..5710c43146060b 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -120,10 +120,10 @@ Handle AsyncWrap::MakeCallback(const Handle cb, if (has_abort_on_uncaught_and_domains) { Local fn = process->Get(env()->domain_abort_uncaught_exc_string()); if (fn->IsFunction()) { - Local specialContext = Array::New(env()->isolate(), 2); - specialContext->Set(0, context); - specialContext->Set(1, cb); - ret = fn.As()->Call(specialContext.As(), argc, argv); + Local special_context = Array::New(env()->isolate(), 2); + special_context->Set(0, context); + special_context->Set(1, cb); + ret = fn.As()->Call(special_context, argc, argv); } else { ret = cb->Call(context, argc, argv); } diff --git a/test/parallel/test-domain-abort-on-uncaught.js b/test/parallel/test-domain-abort-on-uncaught.js index 52864544aec483..9a4bd132a33391 100644 --- a/test/parallel/test-domain-abort-on-uncaught.js +++ b/test/parallel/test-domain-abort-on-uncaught.js @@ -15,7 +15,7 @@ tests.forEach(function(test) { var child = spawn(process.execPath, [ '--abort-on-uncaught-exception', '-e', - '(' + test+ ')()', + '(' + test + ')()', common.PORT ]); child.stderr.pipe(process.stderr);