Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 546e833

Browse filesBrowse files
whitlockjcjasnell
whitlockjc
authored and
jasnell
committed
src: fix --abort-on-uncaught-exception
Revert 0af4c9e, parts of 921f2de and port nodejs/node-v0.x-archive#25835 from v0.12 to master so that node aborts at the right time when an error is thrown and --abort-on-uncaught-exception is used. Fixes #3035. PR: #3036 PR-URL: #3036 Reviewed-By: Ben Noordhuis <ben@strongloop.com>
1 parent 8b50e95 commit 546e833
Copy full SHA for 546e833

File tree

Expand file treeCollapse file tree

8 files changed

+363
-78
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+363
-78
lines changed
Open diff view settings
Collapse file

‎lib/domain.js‎

Copy file name to clipboardExpand all lines: lib/domain.js
+59-29Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ Domain.prototype._disposed = undefined;
5959
// Called by process._fatalException in case an error was thrown.
6060
Domain.prototype._errorHandler = function errorHandler(er) {
6161
var caught = false;
62+
var self = this;
63+
64+
function emitError() {
65+
var handled = self.emit('error', er);
66+
67+
// Exit all domains on the stack. Uncaught exceptions end the
68+
// current tick and no domains should be left on the stack
69+
// between ticks.
70+
stack.length = 0;
71+
exports.active = process.domain = null;
72+
73+
return handled;
74+
}
75+
6276
// ignore errors on disposed domains.
6377
//
6478
// XXX This is a bit stupid. We should probably get rid of
@@ -71,38 +85,54 @@ Domain.prototype._errorHandler = function errorHandler(er) {
7185
er.domain = this;
7286
er.domainThrown = true;
7387
}
74-
// wrap this in a try/catch so we don't get infinite throwing
75-
try {
76-
// One of three things will happen here.
77-
//
78-
// 1. There is a handler, caught = true
79-
// 2. There is no handler, caught = false
80-
// 3. It throws, caught = false
81-
//
82-
// If caught is false after this, then there's no need to exit()
83-
// the domain, because we're going to crash the process anyway.
84-
caught = this.emit('error', er);
8588

86-
// Exit all domains on the stack. Uncaught exceptions end the
87-
// current tick and no domains should be left on the stack
88-
// between ticks.
89-
stack.length = 0;
90-
exports.active = process.domain = null;
91-
} catch (er2) {
92-
// The domain error handler threw! oh no!
93-
// See if another domain can catch THIS error,
94-
// or else crash on the original one.
95-
// If the user already exited it, then don't double-exit.
96-
if (this === exports.active) {
97-
stack.pop();
89+
// The top-level domain-handler is handled separately.
90+
//
91+
// The reason is that if V8 was passed a command line option
92+
// asking it to abort on an uncaught exception (currently
93+
// "--abort-on-uncaught-exception"), we want an uncaught exception
94+
// in the top-level domain error handler to make the
95+
// process abort. Using try/catch here would always make V8 think
96+
// that these exceptions are caught, and thus would prevent it from
97+
// aborting in these cases.
98+
if (stack.length === 1) {
99+
try {
100+
// Set the _emittingTopLevelDomainError so that we know that, even
101+
// if technically the top-level domain is still active, it would
102+
// be ok to abort on an uncaught exception at this point
103+
process._emittingTopLevelDomainError = true;
104+
caught = emitError();
105+
} finally {
106+
process._emittingTopLevelDomainError = false;
98107
}
99-
if (stack.length) {
100-
exports.active = process.domain = stack[stack.length - 1];
101-
caught = process._fatalException(er2);
102-
} else {
103-
caught = false;
108+
} else {
109+
// wrap this in a try/catch so we don't get infinite throwing
110+
try {
111+
// One of three things will happen here.
112+
//
113+
// 1. There is a handler, caught = true
114+
// 2. There is no handler, caught = false
115+
// 3. It throws, caught = false
116+
//
117+
// If caught is false after this, then there's no need to exit()
118+
// the domain, because we're going to crash the process anyway.
119+
caught = emitError();
120+
} catch (er2) {
121+
// The domain error handler threw! oh no!
122+
// See if another domain can catch THIS error,
123+
// or else crash on the original one.
124+
// If the user already exited it, then don't double-exit.
125+
if (this === exports.active) {
126+
stack.pop();
127+
}
128+
if (stack.length) {
129+
exports.active = process.domain = stack[stack.length - 1];
130+
caught = process._fatalException(er2);
131+
} else {
132+
caught = false;
133+
}
134+
return caught;
104135
}
105-
return caught;
106136
}
107137
return caught;
108138
};
Collapse file

‎src/async-wrap.cc‎

Copy file name to clipboardExpand all lines: src/async-wrap.cc
+1-17Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
168168
Local<Object> process = env()->process_object();
169169
Local<Object> domain;
170170
bool has_domain = false;
171-
bool has_abort_on_uncaught_and_domains = false;
172171

173172
if (env()->using_domains()) {
174173
Local<Value> domain_v = context->Get(env()->domain_string());
@@ -177,7 +176,6 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
177176
domain = domain_v.As<Object>();
178177
if (domain->Get(env()->disposed_string())->IsTrue())
179178
return Undefined(env()->isolate());
180-
has_abort_on_uncaught_and_domains = env()->using_abort_on_uncaught_exc();
181179
}
182180
}
183181

@@ -201,21 +199,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
201199
try_catch.SetVerbose(true);
202200
}
203201

204-
Local<Value> ret;
205-
206-
if (has_abort_on_uncaught_and_domains) {
207-
Local<Value> fn = process->Get(env()->domain_abort_uncaught_exc_string());
208-
if (fn->IsFunction()) {
209-
Local<Array> special_context = Array::New(env()->isolate(), 2);
210-
special_context->Set(0, context);
211-
special_context->Set(1, cb);
212-
ret = fn.As<Function>()->Call(special_context, argc, argv);
213-
} else {
214-
ret = cb->Call(context, argc, argv);
215-
}
216-
} else {
217-
ret = cb->Call(context, argc, argv);
218-
}
202+
Local<Value> ret = cb->Call(context, argc, argv);
219203

220204
if (try_catch.HasCaught()) {
221205
return Undefined(env()->isolate());
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
-9Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@ inline Environment::Environment(v8::Local<v8::Context> context,
208208
isolate_data_(IsolateData::GetOrCreate(context->GetIsolate(), loop)),
209209
timer_base_(uv_now(loop)),
210210
using_domains_(false),
211-
using_abort_on_uncaught_exc_(false),
212211
using_asyncwrap_(false),
213212
printed_error_(false),
214213
trace_sync_io_(false),
@@ -341,14 +340,6 @@ inline uint64_t Environment::timer_base() const {
341340
return timer_base_;
342341
}
343342

344-
inline bool Environment::using_abort_on_uncaught_exc() const {
345-
return using_abort_on_uncaught_exc_;
346-
}
347-
348-
inline void Environment::set_using_abort_on_uncaught_exc(bool value) {
349-
using_abort_on_uncaught_exc_ = value;
350-
}
351-
352343
inline bool Environment::using_domains() const {
353344
return using_domains_;
354345
}
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+1-5Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ namespace node {
6969
V(dev_string, "dev") \
7070
V(disposed_string, "_disposed") \
7171
V(domain_string, "domain") \
72-
V(domain_abort_uncaught_exc_string, "_makeCallbackAbortOnUncaught") \
72+
V(emitting_top_level_domain_error_string, "_emittingTopLevelDomainError") \
7373
V(exchange_string, "exchange") \
7474
V(idle_string, "idle") \
7575
V(irq_string, "irq") \
@@ -432,9 +432,6 @@ class Environment {
432432
inline ares_channel* cares_channel_ptr();
433433
inline ares_task_list* cares_task_list();
434434

435-
inline bool using_abort_on_uncaught_exc() const;
436-
inline void set_using_abort_on_uncaught_exc(bool value);
437-
438435
inline bool using_domains() const;
439436
inline void set_using_domains(bool value);
440437

@@ -540,7 +537,6 @@ class Environment {
540537
ares_channel cares_channel_;
541538
ares_task_list cares_task_list_;
542539
bool using_domains_;
543-
bool using_abort_on_uncaught_exc_;
544540
bool using_asyncwrap_;
545541
bool printed_error_;
546542
bool trace_sync_io_;
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+32-11Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ static bool force_repl = false;
124124
static bool syntax_check_only = false;
125125
static bool trace_deprecation = false;
126126
static bool throw_deprecation = false;
127-
static bool abort_on_uncaught_exception = false;
128127
static bool trace_sync_io = false;
129128
static bool track_heap_objects = false;
130129
static const char* eval_string = nullptr;
@@ -907,6 +906,33 @@ void* ArrayBufferAllocator::Allocate(size_t size) {
907906
}
908907

909908

909+
static bool IsDomainActive(const Environment* env) {
910+
if (!env->using_domains())
911+
return false;
912+
913+
Local<Array> domain_array = env->domain_array().As<Array>();
914+
if (domain_array->Length() == 0)
915+
return false;
916+
917+
Local<Value> domain_v = domain_array->Get(0);
918+
return !domain_v->IsNull();
919+
}
920+
921+
922+
static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
923+
HandleScope scope(isolate);
924+
925+
Environment* env = Environment::GetCurrent(isolate);
926+
Local<Object> process_object = env->process_object();
927+
Local<String> emitting_top_level_domain_error_key =
928+
env->emitting_top_level_domain_error_string();
929+
bool isEmittingTopLevelDomainError =
930+
process_object->Get(emitting_top_level_domain_error_key)->BooleanValue();
931+
932+
return !IsDomainActive(env) || isEmittingTopLevelDomainError;
933+
}
934+
935+
910936
void SetupDomainUse(const FunctionCallbackInfo<Value>& args) {
911937
Environment* env = Environment::GetCurrent(args);
912938

@@ -2197,11 +2223,7 @@ void FatalException(Isolate* isolate,
21972223

21982224
if (false == caught->BooleanValue()) {
21992225
ReportException(env, error, message);
2200-
if (abort_on_uncaught_exception) {
2201-
ABORT();
2202-
} else {
2203-
exit(1);
2204-
}
2226+
exit(1);
22052227
}
22062228
}
22072229

@@ -3229,9 +3251,6 @@ static void ParseArgs(int* argc,
32293251
track_heap_objects = true;
32303252
} else if (strcmp(arg, "--throw-deprecation") == 0) {
32313253
throw_deprecation = true;
3232-
} else if (strcmp(arg, "--abort-on-uncaught-exception") == 0 ||
3233-
strcmp(arg, "--abort_on_uncaught_exception") == 0) {
3234-
abort_on_uncaught_exception = true;
32353254
} else if (strcmp(arg, "--v8-options") == 0) {
32363255
new_v8_argv[new_v8_argc] = "--help";
32373256
new_v8_argc += 1;
@@ -3935,8 +3954,10 @@ static void StartNodeInstance(void* arg) {
39353954
Environment* env = CreateEnvironment(isolate, context, instance_data);
39363955
array_buffer_allocator->set_env(env);
39373956
Context::Scope context_scope(context);
3938-
if (instance_data->is_main())
3939-
env->set_using_abort_on_uncaught_exc(abort_on_uncaught_exception);
3957+
3958+
node_isolate->SetAbortOnUncaughtExceptionCallback(
3959+
ShouldAbortOnUncaughtException);
3960+
39403961
// Start debug agent when argv has --debug
39413962
if (instance_data->use_debug_agent())
39423963
StartDebug(env, debug_wait_connect);
Collapse file

‎src/node.js‎

Copy file name to clipboardExpand all lines: src/node.js
-7Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,6 @@
210210
};
211211

212212
startup.processFatal = function() {
213-
process._makeCallbackAbortOnUncaught = function() {
214-
try {
215-
return this[1].apply(this[0], arguments);
216-
} catch (err) {
217-
process._fatalException(err);
218-
}
219-
};
220213

221214
process._fatalException = function(er) {
222215
var caught;
Collapse file

‎test/parallel/test-domain-top-level-error-handler-throw.js‎

Copy file name to clipboard
+50Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
3+
/*
4+
* The goal of this test is to make sure that when a top-level error
5+
* handler throws an error following the handling of a previous error,
6+
* the process reports the error message from the error thrown in the
7+
* top-level error handler, not the one from the previous error.
8+
*/
9+
10+
const common = require('../common');
11+
12+
const domainErrHandlerExMessage = 'exception from domain error handler';
13+
const internalExMessage = 'You should NOT see me';
14+
15+
if (process.argv[2] === 'child') {
16+
var domain = require('domain');
17+
var d = domain.create();
18+
19+
d.on('error', function() {
20+
throw new Error(domainErrHandlerExMessage);
21+
});
22+
23+
d.run(function doStuff() {
24+
process.nextTick(function() {
25+
throw new Error(internalExMessage);
26+
});
27+
});
28+
} else {
29+
var fork = require('child_process').fork;
30+
var assert = require('assert');
31+
32+
var child = fork(process.argv[1], ['child'], {silent:true});
33+
var stderrOutput = '';
34+
if (child) {
35+
child.stderr.on('data', function onStderrData(data) {
36+
stderrOutput += data.toString();
37+
});
38+
39+
child.on('exit', function onChildExited(exitCode, signal) {
40+
assert(stderrOutput.indexOf(domainErrHandlerExMessage) !== -1);
41+
assert(stderrOutput.indexOf(internalExMessage) === -1);
42+
43+
var expectedExitCode = 7;
44+
var expectedSignal = null;
45+
46+
assert.equal(exitCode, expectedExitCode);
47+
assert.equal(signal, expectedSignal);
48+
});
49+
}
50+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.