Skip to content

Commit 68ca2b1

Browse files
author
Julien Gilli
committed
domains: fix no error handler & abort-on-uncaught
Make the process abort if an error is thrown within a domain with no error handler and `--abort-on-uncaught-exception` is used. If the domain within which the error is thrown has no error handler, but a domain further down the domains stack has one, the process will not abort. Fixes #3653
1 parent 412dc11 commit 68ca2b1

File tree

6 files changed

+482
-15
lines changed

6 files changed

+482
-15
lines changed

‎lib/domain.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,20 @@ Object.defineProperty(process, 'domain', {
4545
// between js and c++ w/o much overhead
4646
var _domain_flag = {};
4747

48+
// it's possible to enter one domain while already inside
49+
// another one. the stack is each entered domain.
50+
var stack = [];
51+
exports._stack = stack;
52+
4853
// let the process know we're using domains
49-
process._setupDomainUse(_domain, _domain_flag);
54+
process._setupDomainUse(_domain, _domain_flag, stack);
5055

5156
exports.Domain = Domain;
5257

5358
exports.create = exports.createDomain = function() {
5459
return new Domain();
5560
};
5661

57-
// it's possible to enter one domain while already inside
58-
// another one. the stack is each entered domain.
59-
var stack = [];
60-
exports._stack = stack;
6162
// the active domain is always the one that we're currently in.
6263
exports.active = null;
6364

‎src/env.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ namespace node {
258258
V(buffer_constructor_function, v8::Function) \
259259
V(context, v8::Context) \
260260
V(domain_array, v8::Array) \
261+
V(domains_stack_array, v8::Array) \
261262
V(fs_stats_constructor_function, v8::Function) \
262263
V(gc_info_callback_function, v8::Function) \
263264
V(module_load_list_array, v8::Array) \

‎src/node.cc

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -908,21 +908,57 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
908908
}
909909
#endif
910910

911+
static bool domainHasErrorHandler(const Environment* env,
912+
const Local<Object>& domain) {
913+
HandleScope scope(env->isolate());
911914

912-
static bool IsDomainActive(const Environment* env) {
913-
if (!env->using_domains()) {
915+
Local<Value> domain_event_listeners_v = domain->Get(env->events_string());
916+
if (!domain_event_listeners_v->IsObject())
914917
return false;
915-
}
916918

917-
Local<Array> domain_array = env->domain_array().As<Array>();
918-
uint32_t domains_array_length = domain_array->Length();
919-
if (domains_array_length == 0)
919+
Local<Object> domain_event_listeners_o =
920+
domain_event_listeners_v->ToObject();
921+
922+
if (domain_event_listeners_o->IsNull())
920923
return false;
921924

922-
Local<Value> domain_v = domain_array->Get(0);
923-
return !domain_v->IsNull();
925+
Local<Value> domain_error_listeners_v =
926+
domain_event_listeners_o->Get(env->error_string());
927+
928+
if (domain_error_listeners_v->IsFunction() ||
929+
(domain_error_listeners_v->IsArray() &&
930+
domain_error_listeners_v.As<Array>()->Length() > 0))
931+
return true;
932+
933+
return false;
924934
}
925935

936+
static bool domainsStackHasErrorHandler(const Environment* env) {
937+
HandleScope scope(env->isolate());
938+
939+
if (!env->using_domains())
940+
return false;
941+
942+
Local<Array> domains_stack_array = env->domains_stack_array().As<Array>();
943+
if (domains_stack_array->Length() == 0)
944+
return false;
945+
946+
uint32_t domains_stack_length = domains_stack_array->Length();
947+
for (int i = domains_stack_length - 1; i >= 0; --i) {
948+
Local<Value> domain_v = domains_stack_array->Get(i);
949+
if (domain_v->IsNull())
950+
return false;
951+
952+
Local<Object> domain = domain_v->ToObject();
953+
if (domain->IsNull())
954+
return false;
955+
956+
if (domainHasErrorHandler(env, domain))
957+
return true;
958+
}
959+
960+
return false;
961+
}
926962

927963
bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) {
928964
Environment* env = Environment::GetCurrent(isolate);
@@ -932,7 +968,7 @@ bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) {
932968
bool isEmittingTopLevelDomainError =
933969
process_object->Get(emitting_top_level_domain_error_key)->BooleanValue();
934970

935-
return !IsDomainActive(env) || isEmittingTopLevelDomainError;
971+
return isEmittingTopLevelDomainError || !domainsStackHasErrorHandler(env);
936972
}
937973

938974

@@ -960,8 +996,10 @@ void SetupDomainUse(const FunctionCallbackInfo<Value>& args) {
960996

961997
assert(args[0]->IsArray());
962998
assert(args[1]->IsObject());
999+
assert(args[2]->IsArray());
9631000

9641001
env->set_domain_array(args[0].As<Array>());
1002+
env->set_domains_stack_array(args[2].As<Array>());
9651003

9661004
Local<Object> domain_flag_obj = args[1].As<Object>();
9671005
Environment::DomainFlag* domain_flag = env->domain_flag();

‎test/simple/test-domain-abort-on-uncaught.js

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
'use strict';
2+
3+
/*
4+
* This test makes sure that when using --abort-on-uncaught-exception and
5+
* when throwing an error from within a domain that has an error handler
6+
* setup, the process _does not_ abort.
7+
*/
8+
var common = require('../common');
9+
var assert = require('assert');
10+
var domain = require('domain');
11+
var child_process = require('child_process');
12+
13+
var errorHandlerCalled = false;
14+
15+
var tests = [
16+
function nextTick() {
17+
var d = domain.create();
18+
19+
d.once('error', function(err) {
20+
errorHandlerCalled = true;
21+
});
22+
23+
d.run(function() {
24+
process.nextTick(function() {
25+
throw new Error('exceptional!');
26+
});
27+
});
28+
},
29+
30+
function timer() {
31+
var d = domain.create();
32+
33+
d.on('error', function(err) {
34+
errorHandlerCalled = true;
35+
});
36+
37+
d.run(function() {
38+
setTimeout(function() {
39+
throw new Error('exceptional!');
40+
}, 33);
41+
});
42+
},
43+
44+
function immediate() {
45+
console.log('starting test');
46+
var d = domain.create();
47+
48+
d.on('error', function errorHandler() {
49+
errorHandlerCalled = true;
50+
});
51+
52+
d.run(function() {
53+
setImmediate(function() {
54+
throw new Error('boom!');
55+
});
56+
});
57+
},
58+
59+
function timerPlusNextTick() {
60+
var d = domain.create();
61+
62+
d.on('error', function(err) {
63+
errorHandlerCalled = true;
64+
});
65+
66+
d.run(function() {
67+
setTimeout(function() {
68+
process.nextTick(function() {
69+
throw new Error('exceptional!');
70+
});
71+
}, 33);
72+
});
73+
},
74+
75+
function firstRun() {
76+
var d = domain.create();
77+
78+
d.on('error', function(err) {
79+
errorHandlerCalled = true;
80+
});
81+
82+
d.run(function() {
83+
throw new Error('exceptional!');
84+
});
85+
},
86+
87+
function fsAsync() {
88+
var d = domain.create();
89+
90+
d.on('error', function errorHandler() {
91+
errorHandlerCalled = true;
92+
});
93+
94+
d.run(function() {
95+
var fs = require('fs');
96+
fs.exists('/non/existing/file', function onExists(exists) {
97+
throw new Error('boom!');
98+
});
99+
});
100+
},
101+
102+
function netServer() {
103+
var net = require('net');
104+
var d = domain.create();
105+
106+
d.on('error', function(err) {
107+
errorHandlerCalled = true;
108+
});
109+
110+
d.run(function() {
111+
var server = net.createServer(function(conn) {
112+
conn.pipe(conn);
113+
});
114+
server.listen(common.PORT, '0.0.0.0', function() {
115+
var conn = net.connect(common.PORT, '0.0.0.0');
116+
conn.once('data', function() {
117+
throw new Error('ok');
118+
});
119+
conn.end('ok');
120+
server.close();
121+
});
122+
});
123+
},
124+
125+
function firstRunOnlyTopLevelErrorHandler() {
126+
var d = domain.create();
127+
var d2 = domain.create();
128+
129+
d.on('error', function errorHandler() {
130+
errorHandlerCalled = true;
131+
});
132+
133+
d.run(function() {
134+
d2.run(function() {
135+
throw new Error('boom!');
136+
});
137+
});
138+
},
139+
140+
function firstRunNestedWithErrorHandler() {
141+
var d = domain.create();
142+
var d2 = domain.create();
143+
144+
d2.on('error', function errorHandler() {
145+
errorHandlerCalled = true;
146+
});
147+
148+
d.run(function() {
149+
d2.run(function() {
150+
throw new Error('boom!');
151+
});
152+
});
153+
},
154+
155+
function timeoutNestedWithErrorHandler() {
156+
var d = domain.create();
157+
var d2 = domain.create();
158+
159+
d2.on('error', function errorHandler() {
160+
errorHandlerCalled = true;
161+
});
162+
163+
d.run(function() {
164+
d2.run(function() {
165+
setTimeout(function() {
166+
console.log('foo');
167+
throw new Error('boom!');
168+
}, 33);
169+
});
170+
});
171+
},
172+
173+
function setImmediateNestedWithErrorHandler() {
174+
var d = domain.create();
175+
var d2 = domain.create();
176+
177+
d2.on('error', function errorHandler() {
178+
errorHandlerCalled = true;
179+
});
180+
181+
d.run(function() {
182+
d2.run(function() {
183+
setImmediate(function() {
184+
throw new Error('boom!');
185+
});
186+
});
187+
});
188+
},
189+
190+
function nextTickNestedWithErrorHandler() {
191+
var d = domain.create();
192+
var d2 = domain.create();
193+
194+
d2.on('error', function errorHandler() {
195+
errorHandlerCalled = true;
196+
});
197+
198+
d.run(function() {
199+
d2.run(function() {
200+
process.nextTick(function() {
201+
throw new Error('boom!');
202+
});
203+
});
204+
});
205+
},
206+
207+
function fsAsyncNestedWithErrorHandler() {
208+
var d = domain.create();
209+
var d2 = domain.create();
210+
211+
d2.on('error', function errorHandler() {
212+
errorHandlerCalled = true;
213+
});
214+
215+
d.run(function() {
216+
d2.run(function() {
217+
var fs = require('fs');
218+
fs.exists('/non/existing/file', function onExists(exists) {
219+
throw new Error('boom!');
220+
});
221+
});
222+
});
223+
}
224+
];
225+
226+
if (process.argv[2] === 'child') {
227+
var testIndex = +process.argv[3];
228+
229+
tests[testIndex]();
230+
231+
process.on('exit', function onExit() {
232+
assert.equal(errorHandlerCalled, true);
233+
});
234+
} else {
235+
236+
tests.forEach(function(test, testIndex) {
237+
var testCmd = '';
238+
if (process.platform !== 'win32') {
239+
// Do not create core files, as it can take a lot of disk space on
240+
// continuous testing and developers' machines
241+
testCmd += 'ulimit -c 0 && ';
242+
}
243+
244+
testCmd += process.argv[0];
245+
testCmd += ' ' + '--abort-on-uncaught-exception';
246+
testCmd += ' ' + process.argv[1];
247+
testCmd += ' ' + 'child';
248+
testCmd += ' ' + testIndex;
249+
250+
var child = child_process.exec(testCmd);
251+
252+
child.on('exit', function onExit(code, signal) {
253+
assert.equal(code, 0, 'Test at index ' + testIndex +
254+
' should have exited with exit code 0 but instead exited with code ' +
255+
code + ' and signal ' + signal);
256+
});
257+
258+
});
259+
}

0 commit comments

Comments
 (0)