diff --git a/lib/domain.js b/lib/domain.js index b6321a20f80db6..d0f51d9c14296b 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -27,8 +27,13 @@ Object.defineProperty(process, 'domain', { } }); +// It's possible to enter one domain while already inside +// another one. the stack is each entered domain. +const stack = []; +exports._stack = stack; + // let the process know we're using domains -const _domain_flag = process._setupDomainUse(_domain); +const _domain_flag = process._setupDomainUse(_domain, stack); exports.Domain = Domain; @@ -36,10 +41,6 @@ exports.create = exports.createDomain = function() { return new Domain(); }; -// it's possible to enter one domain while already inside -// another one. the stack is each entered domain. -var stack = []; -exports._stack = stack; // the active domain is always the one that we're currently in. exports.active = null; @@ -96,14 +97,20 @@ Domain.prototype._errorHandler = function errorHandler(er) { // that these exceptions are caught, and thus would prevent it from // aborting in these cases. if (stack.length === 1) { - try { - // Set the _emittingTopLevelDomainError so that we know that, even - // if technically the top-level domain is still active, it would - // be ok to abort on an uncaught exception at this point - process._emittingTopLevelDomainError = true; - caught = emitError(); - } finally { - process._emittingTopLevelDomainError = false; + // If there's no error handler, do not emit an 'error' event + // as this would throw an error, make the process exit, and thus + // prevent the process 'uncaughtException' event from being emitted + // if a listener is set. + if (EventEmitter.listenerCount(self, 'error') > 0) { + try { + // Set the _emittingTopLevelDomainError so that we know that, even + // if technically the top-level domain is still active, it would + // be ok to abort on an uncaught exception at this point + process._emittingTopLevelDomainError = true; + caught = emitError(); + } finally { + process._emittingTopLevelDomainError = false; + } } } else { // wrap this in a try/catch so we don't get infinite throwing diff --git a/src/env.h b/src/env.h index 97652146fabf0d..185cc17294ec05 100644 --- a/src/env.h +++ b/src/env.h @@ -236,6 +236,7 @@ namespace node { V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ V(domain_array, v8::Array) \ + V(domains_stack_array, v8::Array) \ V(fs_stats_constructor_function, v8::Function) \ V(generic_internal_field_template, v8::ObjectTemplate) \ V(jsstream_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node.cc b/src/node.cc index 0a2929baffb0b1..cdbcc8b9f471d1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -947,17 +947,53 @@ void* ArrayBufferAllocator::Allocate(size_t size) { return malloc(size); } +static bool DomainHasErrorHandler(const Environment* env, + const Local& domain) { + HandleScope scope(env->isolate()); + + Local domain_event_listeners_v = domain->Get(env->events_string()); + if (!domain_event_listeners_v->IsObject()) + return false; + + Local domain_event_listeners_o = + domain_event_listeners_v.As(); + + Local domain_error_listeners_v = + domain_event_listeners_o->Get(env->error_string()); + + if (domain_error_listeners_v->IsFunction() || + (domain_error_listeners_v->IsArray() && + domain_error_listeners_v.As()->Length() > 0)) + return true; + + return false; +} + +static bool TopDomainHasErrorHandler(const Environment* env) { + HandleScope scope(env->isolate()); -static bool IsDomainActive(const Environment* env) { if (!env->using_domains()) return false; - Local domain_array = env->domain_array().As(); - if (domain_array->Length() == 0) + Local domains_stack_array = env->domains_stack_array().As(); + if (domains_stack_array->Length() == 0) + return false; + + uint32_t domains_stack_length = domains_stack_array->Length(); + if (domains_stack_length == 0) + return false; + + Local top_domain_v = + domains_stack_array->Get(domains_stack_length - 1); + + if (!top_domain_v->IsObject()) return false; - Local domain_v = domain_array->Get(0); - return !domain_v->IsNull(); + Local top_domain = top_domain_v.As(); + if (DomainHasErrorHandler(env, top_domain)) + return true; + + return false; } @@ -971,7 +1007,7 @@ static bool ShouldAbortOnUncaughtException(Isolate* isolate) { bool isEmittingTopLevelDomainError = process_object->Get(emitting_top_level_domain_error_key)->BooleanValue(); - return !IsDomainActive(env) || isEmittingTopLevelDomainError; + return isEmittingTopLevelDomainError || !TopDomainHasErrorHandler(env); } @@ -1000,6 +1036,9 @@ void SetupDomainUse(const FunctionCallbackInfo& args) { CHECK(args[0]->IsArray()); env->set_domain_array(args[0].As()); + CHECK(args[1]->IsArray()); + env->set_domains_stack_array(args[1].As()); + // Do a little housekeeping. env->process_object()->Delete( FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupDomainUse")); diff --git a/test/parallel/test-domain-abort-on-uncaught.js b/test/parallel/test-domain-abort-on-uncaught.js index f4884aa8f33421..3adbb91f93d639 100644 --- a/test/parallel/test-domain-abort-on-uncaught.js +++ b/test/parallel/test-domain-abort-on-uncaught.js @@ -1,96 +1,241 @@ 'use strict'; -// Flags: --abort_on_uncaught_exception - -var common = require('../common'); -var assert = require('assert'); -var domain = require('domain'); - -var tests = [ - nextTick, - timer, - timerPlusNextTick, - netServer, - firstRun, -]; -var errors = 0; +// This test makes sure that when using --abort-on-uncaught-exception and +// when throwing an error from within a domain that has an error handler +// setup, the process _does not_ abort. -process.on('exit', function() { - assert.equal(errors, tests.length); -}); +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); +const child_process = require('child_process'); -tests.forEach(function(test) { test(); }); +var errorHandlerCalled = false; -function nextTick() { - var d = domain.create(); +const tests = [ + function nextTick() { + const d = domain.create(); - d.once('error', function(err) { - errors += 1; - }); - d.run(function() { - process.nextTick(function() { - throw new Error('exceptional!'); + d.once('error', function(err) { + errorHandlerCalled = true; }); - }); -} -function timer() { - var d = domain.create(); + d.run(function() { + process.nextTick(function() { + throw new Error('exceptional!'); + }); + }); + }, - d.on('error', function(err) { - errors += 1; - }); - d.run(function() { - setTimeout(function() { - throw new Error('exceptional!'); - }, 33); - }); -} + function timer() { + const d = domain.create(); -function timerPlusNextTick() { - var d = domain.create(); + d.on('error', function(err) { + errorHandlerCalled = true; + }); - d.on('error', function(err) { - errors += 1; - }); - d.run(function() { - setTimeout(function() { - process.nextTick(function() { + d.run(function() { + setTimeout(function() { throw new Error('exceptional!'); + }, 33); + }); + }, + + function immediate() { + const d = domain.create(); + + d.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + setImmediate(function() { + throw new Error('boom!'); }); - }, 33); - }); -} + }); + }, -function firstRun() { - var d = domain.create(); + function timerPlusNextTick() { + const d = domain.create(); - d.on('error', function(err) { - errors += 1; - }); - d.run(function() { - throw new Error('exceptional!'); - }); -} + d.on('error', function(err) { + errorHandlerCalled = true; + }); -function netServer() { - var net = require('net'); - var d = domain.create(); + d.run(function() { + setTimeout(function() { + process.nextTick(function() { + throw new Error('exceptional!'); + }); + }, 33); + }); + }, - d.on('error', function(err) { - errors += 1; - }); - d.run(function() { - var server = net.createServer(function(conn) { - conn.pipe(conn); - }); - server.listen(common.PORT, '0.0.0.0', function() { - var conn = net.connect(common.PORT, '0.0.0.0'); - conn.once('data', function() { - throw new Error('ok'); + function firstRun() { + const d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + throw new Error('exceptional!'); + }); + }, + + function fsAsync() { + const d = domain.create(); + + d.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }, + + function netServer() { + const net = require('net'); + const d = domain.create(); + + d.on('error', function(err) { + errorHandlerCalled = true; + }); + + d.run(function() { + const server = net.createServer(function(conn) { + conn.pipe(conn); + }); + server.listen(common.PORT, common.localhostIPv4, function() { + const conn = net.connect(common.PORT, common.localhostIPv4); + conn.once('data', function() { + throw new Error('ok'); + }); + conn.end('ok'); + server.close(); + }); + }); + }, + + function firstRunNestedWithErrorHandler() { + const d = domain.create(); + const d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + function timeoutNestedWithErrorHandler() { + const d = domain.create(); + const d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + setTimeout(function() { + console.log('foo'); + throw new Error('boom!'); + }, 33); }); - conn.end('ok'); - server.close(); + }); + }, + + function setImmediateNestedWithErrorHandler() { + const d = domain.create(); + const d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function nextTickNestedWithErrorHandler() { + const d = domain.create(); + const d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function fsAsyncNestedWithErrorHandler() { + const d = domain.create(); + const d2 = domain.create(); + + d2.on('error', function errorHandler() { + errorHandlerCalled = true; + }); + + d.run(function() { + d2.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }); + } +]; + +if (process.argv[2] === 'child') { + const testIndex = +process.argv[3]; + + tests[testIndex](); + + process.on('exit', function onExit() { + assert.equal(errorHandlerCalled, true); + }); +} else { + + tests.forEach(function(test, testIndex) { + var testCmd = ''; + if (process.platform !== 'win32') { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines + testCmd += 'ulimit -c 0 && '; + } + + testCmd += process.argv[0]; + testCmd += ' ' + '--abort-on-uncaught-exception'; + testCmd += ' ' + process.argv[1]; + testCmd += ' ' + 'child'; + testCmd += ' ' + testIndex; + + var child = child_process.exec(testCmd); + + child.on('exit', function onExit(code, signal) { + assert.equal(code, 0, 'Test at index ' + testIndex + + ' should have exited with exit code 0 but instead exited with code ' + + code + ' and signal ' + signal); }); }); } diff --git a/test/parallel/test-domain-no-error-handler-abort-on-uncaught.js b/test/parallel/test-domain-no-error-handler-abort-on-uncaught.js new file mode 100644 index 00000000000000..870afab4ffd27e --- /dev/null +++ b/test/parallel/test-domain-no-error-handler-abort-on-uncaught.js @@ -0,0 +1,189 @@ +'use strict'; + +/* + * This test makes sure that when using --abort-on-uncaught-exception and + * when throwing an error from within a domain that does not have an error + * handler setup, the process aborts. + */ +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); +const child_process = require('child_process'); + +const tests = [ + function() { + const d = domain.create(); + + d.run(function() { + throw new Error('boom!'); + }); + }, + + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + /* + * In this case, only the domain at the top of the stack "d" has an error + * handler, but the domain from which the error is thrown doesn't have an + * error handler. To be consistent with the way domains bubble up errors, + * this test case _should not_ make the process abort. However it was + * determined that making such a change in v4.x might be a breaking change, + * so instead this test case makes sure the process aborts. + */ + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + const d = domain.create(); + + d.run(function() { + setTimeout(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + const d = domain.create(); + + d.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + const d = domain.create(); + + d.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }, + + function() { + const d = domain.create(); + + d.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }, + + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + setTimeout(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + setImmediate(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + process.nextTick(function() { + throw new Error('boom!'); + }); + }); + }); + }, + + function() { + const d = domain.create(); + const d2 = domain.create(); + + d.on('error', function errorHandler() { + }); + + d.run(function() { + d2.run(function() { + var fs = require('fs'); + fs.exists('/non/existing/file', function onExists(exists) { + throw new Error('boom!'); + }); + }); + }); + } +]; + +if (process.argv[2] === 'child') { + const testIndex = +process.argv[3]; + tests[testIndex](); +} else { + + tests.forEach(function(test, testIndex) { + var testCmd = ''; + if (process.platform !== 'win32') { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines + testCmd += 'ulimit -c 0 && '; + } + + testCmd += process.argv[0]; + testCmd += ' ' + '--abort-on-uncaught-exception'; + testCmd += ' ' + process.argv[1]; + testCmd += ' ' + 'child'; + testCmd += ' ' + testIndex; + + var child = child_process.exec(testCmd); + + child.on('exit', function onExit(code, signal) { + assert.ok([132, 133, 134].indexOf(code) !== -1, 'Test at index ' + + testIndex + ' should have aborted but instead exited with code ' + + code + ' and signal ' + signal); + }); + }); +} diff --git a/test/parallel/test-domain-uncaught-exception.js b/test/parallel/test-domain-uncaught-exception.js new file mode 100644 index 00000000000000..f49fd35a5932e2 --- /dev/null +++ b/test/parallel/test-domain-uncaught-exception.js @@ -0,0 +1,205 @@ +'use strict'; + +/* + * The goal of this test is to make sure that errors thrown within domains + * are handled correctly. It checks that the process 'uncaughtException' event + * is emitted when appropriate, and not emitted when it shouldn't. It also + * checks that the proper domain error handlers are called when they should + * be called, and not called when they shouldn't. + */ + +const common = require('../common'); +const assert = require('assert'); +const domain = require('domain'); +const child_process = require('child_process'); + +const uncaughtExceptions = {}; + +const tests = []; + +function test1() { + /* + * Throwing from an async callback from within a domain that doesn't have + * an error handler must result in emitting the process' uncaughtException + * event. + */ + const d = domain.create(); + d.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); +} + +tests.push({ + fn: test1, + expectedMessages: ['uncaughtException'] +}); + +function test2() { + /* + * Throwing from from within a domain that doesn't have an error handler must + * result in emitting the process' uncaughtException event. + */ + const d2 = domain.create(); + d2.run(function() { + throw new Error('boom!'); + }); +} + +tests.push({ + fn: test2, + expectedMessages: ['uncaughtException'] +}); + +function test3() { + /* + * This test creates two nested domains: d3 and d4. d4 doesn't register an + * error handler, but d3 does. The error is handled by the d3 domain and thus + * and 'uncaughtException' event should _not_ be emitted. + */ + const d3 = domain.create(); + const d4 = domain.create(); + + d3.on('error', function onErrorInD3Domain(err) { + process.send('errorHandledByDomain'); + }); + + d3.run(function() { + d4.run(function() { + throw new Error('boom!'); + }); + }); +} + +tests.push({ + fn: test3, + expectedMessages: ['errorHandledByDomain'] +}); + +function test4() { + /* + * This test creates two nested domains: d5 and d6. d6 doesn't register an + * error handler. When the timer's callback is called, because async + * operations like timer callbacks are bound to the domain that was active + * at the time of their creation, and because both d5 and d6 domains have + * exited by the time the timer's callback is called, its callback runs with + * only d6 on the domains stack. Since d6 doesn't register an error handler, + * the process' uncaughtException event should be emitted. + */ + const d5 = domain.create(); + const d6 = domain.create(); + + d5.on('error', function onErrorInD2Domain(err) { + process.send('errorHandledByDomain'); + }); + + d5.run(function() { + d6.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); + }); +} + +tests.push({ + fn: test4, + expectedMessages: ['uncaughtException'] +}); + +function test5() { + /* + * This test creates two nested domains: d7 and d4. d8 _does_ register an + * error handler, so throwing within that domain should not emit an uncaught + * exception. + */ + const d7 = domain.create(); + const d8 = domain.create(); + + d8.on('error', function onErrorInD3Domain(err) { + process.send('errorHandledByDomain'); + }); + + d7.run(function() { + d8.run(function() { + throw new Error('boom!'); + }); + }); +} +tests.push({ + fn: test5, + expectedMessages: ['errorHandledByDomain'] +}); + +function test6() { + /* + * This test creates two nested domains: d9 and d10. d10 _does_ register an + * error handler, so throwing within that domain in an async callback should + * _not_ emit an uncaught exception. + */ + const d9 = domain.create(); + const d10 = domain.create(); + + d10.on('error', function onErrorInD2Domain(err) { + process.send('errorHandledByDomain'); + }); + + d9.run(function() { + d10.run(function() { + setTimeout(function onTimeout() { + throw new Error('boom!'); + }); + }); + }); +} + +tests.push({ + fn: test6, + expectedMessages: ['errorHandledByDomain'] +}); + +if (process.argv[2] === 'child') { + const testIndex = process.argv[3]; + process.on('uncaughtException', function onUncaughtException() { + process.send('uncaughtException'); + }); + + tests[testIndex].fn(); +} else { + // Run each test's function in a child process. Listen on + // messages sent by each child process and compare expected + // messages defined for each ttest from the actual received messages. + tests.forEach(function doTest(test, testIndex) { + const testProcess = child_process.fork(__filename, ['child', testIndex]); + + testProcess.on('message', function onMsg(msg) { + if (test.messagesReceived === undefined) + test.messagesReceived = []; + + test.messagesReceived.push(msg); + }); + + testProcess.on('exit', function onExit() { + // Make sure that all expected messages were sent from the + // child process + test.expectedMessages.forEach(function(expectedMessage) { + if (test.messagesReceived === undefined || + test.messagesReceived.indexOf(expectedMessage) === -1) + assert(false, 'test ' + test.fn.name + + ' should have sent message: ' + expectedMessage + + ' but didn\'t'); + }); + + if (test.messagesReceived) { + test.messagesReceived.forEach(function(receivedMessage) { + if (test.expectedMessages.indexOf(receivedMessage) === -1) { + assert(false, 'test ' + test.fn.name + + ' should not have sent message: ' + receivedMessage + + ' but did'); + } + }); + } + }); + }); +}