diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 5d7b3b5ff1..d1175d480b 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -146,9 +146,9 @@ instead. ### DEP0012: Domain.dispose -Type: Runtime +Type: End-of-Life -[`Domain.dispose()`][] is deprecated. Recover from failed I/O actions +[`Domain.dispose()`][] is removed. Recover from failed I/O actions explicitly via error event handlers set on the domain instead. diff --git a/doc/api/domain.md b/doc/api/domain.md index a4a31d4fec..285378c3bd 100644 --- a/doc/api/domain.md +++ b/doc/api/domain.md @@ -217,7 +217,7 @@ added. Implicit binding routes thrown errors and `'error'` events to the Domain's `'error'` event, but does not register the EventEmitter on the -Domain, so [`domain.dispose()`][] will not shut down the EventEmitter. +Domain. Implicit binding only takes care of thrown errors and `'error'` events. ## Explicit Binding @@ -329,15 +329,6 @@ d.on('error', (er) => { }); ``` -### domain.dispose() - -> Stability: 0 - Deprecated. Please recover from failed IO actions -> explicitly via error event handlers set on the domain. - -Once `dispose` has been called, the domain will no longer be used by callbacks -bound into the domain via `run`, `bind`, or `intercept`, and a `'dispose'` event -is emitted. - ### domain.enter() The `enter` method is plumbing used by the `run`, `bind`, and `intercept` @@ -351,9 +342,6 @@ Calling `enter` changes only the active domain, and does not alter the domain itself. `enter` and `exit` can be called an arbitrary number of times on a single domain. -If the domain on which `enter` is called has been disposed, `enter` will return -without setting the domain. - ### domain.exit() The `exit` method exits the current domain, popping it off the domain stack. @@ -369,9 +357,6 @@ Calling `exit` changes only the active domain, and does not alter the domain itself. `enter` and `exit` can be called an arbitrary number of times on a single domain. -If the domain on which `exit` is called has been disposed, `exit` will return -without exiting the domain. - ### domain.intercept(callback) * `callback` {Function} The callback function @@ -500,7 +485,6 @@ rejections. [`EventEmitter`]: events.html#events_class_eventemitter [`domain.add(emitter)`]: #domain_domain_add_emitter [`domain.bind(callback)`]: #domain_domain_bind_callback -[`domain.dispose()`]: #domain_domain_dispose [`domain.exit()`]: #domain_domain_exit [`setInterval()`]: timers.html#timers_setinterval_callback_delay_args [`setTimeout()`]: timers.html#timers_settimeout_callback_delay_args diff --git a/lib/domain.js b/lib/domain.js index 5cef123da8..1006e2a0f5 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -75,21 +75,12 @@ function Domain() { } Domain.prototype.members = undefined; -Domain.prototype._disposed = undefined; // Called by process._fatalException in case an error was thrown. Domain.prototype._errorHandler = function _errorHandler(er) { var caught = false; - // ignore errors on disposed domains. - // - // XXX This is a bit stupid. We should probably get rid of - // domain.dispose() altogether. It's almost always a terrible - // idea. --isaacs - if (this._disposed) - return true; - if (!util.isPrimitive(er)) { er.domain = this; er.domainThrown = true; @@ -160,8 +151,6 @@ Domain.prototype._errorHandler = function _errorHandler(er) { Domain.prototype.enter = function() { - if (this._disposed) return; - // note that this might be a no-op, but we still need // to push it onto the stack so that we can pop it later. exports.active = process.domain = this; @@ -171,10 +160,9 @@ Domain.prototype.enter = function() { Domain.prototype.exit = function() { - // skip disposed domains, as usual, but also don't do anything if this - // domain is not on the stack. + // don't do anything if this domain is not on the stack. var index = stack.lastIndexOf(this); - if (this._disposed || index === -1) return; + if (index === -1) return; // exit all domains until this one. stack.splice(index); @@ -187,8 +175,8 @@ Domain.prototype.exit = function() { // note: this works for timers as well. Domain.prototype.add = function(ee) { - // If the domain is disposed or already added, then nothing left to do. - if (this._disposed || ee.domain === this) + // If the domain is already added, then nothing left to do. + if (ee.domain === this) return; // has a domain already - remove it first. @@ -224,9 +212,6 @@ Domain.prototype.remove = function(ee) { Domain.prototype.run = function(fn) { - if (this._disposed) - return; - var ret; this.enter(); @@ -248,9 +233,6 @@ Domain.prototype.run = function(fn) { function intercepted(_this, self, cb, fnargs) { - if (self._disposed) - return; - if (fnargs[0] && fnargs[0] instanceof Error) { var er = fnargs[0]; util._extend(er, { @@ -291,9 +273,6 @@ Domain.prototype.intercept = function(cb) { function bound(_this, self, cb, fnargs) { - if (self._disposed) - return; - var ret; self.enter(); @@ -318,22 +297,3 @@ Domain.prototype.bind = function(cb) { return runBound; }; - - -Domain.prototype.dispose = util.deprecate(function() { - if (this._disposed) return; - - // if we're the active domain, then get out now. - this.exit(); - - // remove from parent domain, if there is one. - if (this.domain) this.domain.remove(this); - - // kill the references so that they can be properly gc'ed. - this.members.length = 0; - - // mark this domain as 'no longer relevant' - // so that it can't be entered or activated. - this._disposed = true; -}, 'Domain.dispose is deprecated. Recover from failed I/O actions explicitly ' + - 'via error event handlers set on the domain instead.', 'DEP0012'); diff --git a/lib/timers.js b/lib/timers.js index 03c978d076..6651e24a4e 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -250,15 +250,6 @@ function listOnTimeout() { var domain = timer.domain; if (domain) { - - // If the timer callback throws and the - // domain or uncaughtException handler ignore the exception, - // other timers that expire on this tick should still run. - // - // https://github.com/nodejs/node-v0.x-archive/issues/2631 - if (domain._disposed) - continue; - domain.enter(); } diff --git a/src/env.h b/src/env.h index 092e60441c..8de937eb37 100644 --- a/src/env.h +++ b/src/env.h @@ -116,7 +116,6 @@ struct performance_state; V(dest_string, "dest") \ V(destroy_string, "destroy") \ V(detached_string, "detached") \ - V(disposed_string, "_disposed") \ V(dns_a_string, "A") \ V(dns_aaaa_string, "AAAA") \ V(dns_cname_string, "CNAME") \ diff --git a/src/node.cc b/src/node.cc index 213c57a61c..a234569e21 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1159,12 +1159,10 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) { } -bool DomainEnter(Environment* env, Local object) { +void DomainEnter(Environment* env, Local object) { Local domain_v = object->Get(env->domain_string()); if (domain_v->IsObject()) { Local domain = domain_v.As(); - if (domain->Get(env->disposed_string())->IsTrue()) - return true; Local enter_v = domain->Get(env->enter_string()); if (enter_v->IsFunction()) { if (enter_v.As()->Call(domain, 0, nullptr).IsEmpty()) { @@ -1173,16 +1171,13 @@ bool DomainEnter(Environment* env, Local object) { } } } - return false; } -bool DomainExit(Environment* env, v8::Local object) { +void DomainExit(Environment* env, v8::Local object) { Local domain_v = object->Get(env->domain_string()); if (domain_v->IsObject()) { Local domain = domain_v.As(); - if (domain->Get(env->disposed_string())->IsTrue()) - return true; Local exit_v = domain->Get(env->exit_string()); if (exit_v->IsFunction()) { if (exit_v.As()->Call(domain, 0, nullptr).IsEmpty()) { @@ -1191,7 +1186,6 @@ bool DomainExit(Environment* env, v8::Local object) { } } } - return false; } @@ -1398,9 +1392,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env, CHECK_EQ(env->context(), env->isolate()->GetCurrentContext()); if (env->using_domains()) { - failed_ = DomainEnter(env, object_); - if (failed_) - return; + DomainEnter(env, object_); } if (asyncContext.async_id != 0) { @@ -1432,8 +1424,7 @@ void InternalCallbackScope::Close() { } if (env_->using_domains()) { - failed_ = DomainExit(env_, object_); - if (failed_) return; + DomainExit(env_, object_); } if (IsInnerMakeCallback()) { diff --git a/test/parallel/test-domain-abort-when-disposed.js b/test/parallel/test-domain-abort-when-disposed.js deleted file mode 100644 index 3a02b1e94c..0000000000 --- a/test/parallel/test-domain-abort-when-disposed.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const domain = require('domain'); - -// These were picked arbitrarily and are only used to trigger arync_hooks. -const JSStream = process.binding('js_stream').JSStream; -const Socket = require('net').Socket; - -const handle = new JSStream(); -handle.domain = domain.create(); -handle.domain.dispose(); - -handle.close = () => {}; -handle.isAlive = () => { throw new Error(); }; - -const s = new Socket({ handle }); - -// When AsyncWrap::MakeCallback() returned an empty handle the -// MaybeLocal::ToLocalChecked() call caused the process to abort. By returning -// v8::Undefined() it allows the error to propagate to the 'error' event. -s.on('error', common.mustCall((e) => { - assert.strictEqual(e.code, 'EINVAL'); -})); diff --git a/test/parallel/test-domain-exit-dispose-again.js b/test/parallel/test-domain-exit-dispose-again.js deleted file mode 100644 index 31a1ff598a..0000000000 --- a/test/parallel/test-domain-exit-dispose-again.js +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; - -// This test makes sure that when a domain is disposed, timers that are -// attached to that domain are not fired, but timers that are _not_ attached -// to that domain, including those whose callbacks are called from within -// the same invocation of listOnTimeout, _are_ called. - -require('../common'); -const assert = require('assert'); -const domain = require('domain'); -let disposalFailed = false; - -// Repeatedly schedule a timer with a delay different than the timers attached -// to a domain that will eventually be disposed to make sure that they are -// called, regardless of what happens with those timers attached to domains -// that will eventually be disposed. -let a = 0; -log(); -function log() { - console.log(a++, process.domain); - if (a < 10) setTimeout(log, 20); -} - -let secondTimerRan = false; - -// Use the same timeout duration for both "firstTimer" and "secondTimer" -// callbacks so that they are called during the same invocation of the -// underlying native timer's callback (listOnTimeout in lib/timers.js). -const TIMEOUT_DURATION = 50; - -setTimeout(function firstTimer() { - const d = domain.create(); - - d.on('error', function handleError(err) { - // Dispose the domain on purpose, so that we can test that nestedTimer - // is not called since it's associated to this domain and a timer whose - // domain is diposed should not run. - d.dispose(); - console.error(err); - console.error('in domain error handler', - process.domain, process.domain === d); - }); - - d.run(function() { - // Create another nested timer that is by definition associated to the - // domain "d". Because an error is thrown before the timer's callback - // is called, and because the domain's error handler disposes the domain, - // this timer's callback should never run. - setTimeout(function nestedTimer() { - console.error('Nested timer should not run, because it is attached to ' + - 'a domain that should be disposed.'); - disposalFailed = true; - process.exit(1); - }, 1); - - // Make V8 throw an unreferenced error. As a result, the domain's error - // handler is called, which disposes the domain "d" and should prevent the - // nested timer that is attached to it from running. - err3(); // eslint-disable-line no-undef - }); -}, TIMEOUT_DURATION); - -// This timer expires in the same invocation of listOnTimeout than firstTimer, -// but because it's not attached to any domain, it must run regardless of -// domain "d" being disposed. -setTimeout(function secondTimer() { - console.log('In second timer'); - secondTimerRan = true; -}, TIMEOUT_DURATION); - -process.on('exit', function() { - assert.strictEqual(a, 10); - assert.strictEqual(disposalFailed, false); - assert(secondTimerRan); - console.log('ok'); -}); diff --git a/test/parallel/test-domain-exit-dispose.js b/test/parallel/test-domain-exit-dispose.js deleted file mode 100644 index 0bc8adae82..0000000000 --- a/test/parallel/test-domain-exit-dispose.js +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const domain = require('domain'); - -// no matter what happens, we should increment a 10 times. -let a = 0; -log(); -function log() { - console.log(a++, process.domain); - if (a < 10) setTimeout(log, 20); -} - -// in 50ms we'll throw an error. -setTimeout(err, 50); -function err() { - const d = domain.create(); - d.on('error', handle); - d.run(err2); - - function err2() { - // this timeout should never be called, since the domain gets - // disposed when the error happens. - setTimeout(common.mustNotCall(), 1); - - // this function doesn't exist, and throws an error as a result. - err3(); // eslint-disable-line no-undef - } - - function handle(e) { - // this should clean up everything properly. - d.dispose(); - console.error(e); - console.error('in handler', process.domain, process.domain === d); - } -} - -process.on('exit', function() { - assert.strictEqual(a, 10); - console.log('ok'); -}); diff --git a/test/parallel/test-domain-nested-throw.js b/test/parallel/test-domain-nested-throw.js index f7fcbcad64..ec016ada72 100644 --- a/test/parallel/test-domain-nested-throw.js +++ b/test/parallel/test-domain-nested-throw.js @@ -25,31 +25,19 @@ const assert = require('assert'); const domain = require('domain'); -let dispose; -switch (process.argv[2]) { - case 'true': - dispose = true; - break; - case 'false': - dispose = false; - break; - default: - parent(); - return; +if (process.argv[2] !== 'child') { + parent(); + return; } function parent() { const node = process.execPath; const spawn = require('child_process').spawn; const opt = { stdio: 'inherit' }; - let child = spawn(node, [__filename, 'true'], opt); + const child = spawn(node, [__filename, 'child'], opt); child.on('exit', function(c) { assert(!c); - child = spawn(node, [__filename, 'false'], opt); - child.on('exit', function(c) { - assert(!c); - console.log('ok'); - }); + console.log('ok'); }); } @@ -77,7 +65,6 @@ function inner(throw1, throw2) { console.error('got domain 1 twice'); process.exit(1); } - if (dispose) domain1.dispose(); gotDomain1Error = true; throw2(); }); @@ -108,7 +95,7 @@ process.on('exit', function() { assert(gotDomain2Error); assert(threw1); assert(threw2); - console.log('ok', dispose); + console.log('ok'); }); outer(); diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index 7a367920b0..4494e5084d 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -634,7 +634,6 @@ asyncTest( onUnhandledSucceed(done, function(reason, promise) { assert.strictEqual(reason, e); assert.strictEqual(domainReceivedError, domainError); - d.dispose(); }); Promise.reject(e); process.nextTick(function() { diff --git a/test/parallel/test-timers-unref-active-unenrolled-disposed.js b/test/parallel/test-timers-unref-active-unenrolled-disposed.js deleted file mode 100644 index 675da01717..0000000000 --- a/test/parallel/test-timers-unref-active-unenrolled-disposed.js +++ /dev/null @@ -1,46 +0,0 @@ -'use strict'; - -// https://github.com/nodejs/node/pull/2540/files#r38231197 - -const common = require('../common'); -const timers = require('timers'); -const assert = require('assert'); -const domain = require('domain'); - -// Crazy stuff to keep the process open, -// then close it when we are actually done. -const TEST_DURATION = common.platformTimeout(1000); -const keepOpen = setTimeout(function() { - throw new Error('Test timed out. keepOpen was not canceled.'); -}, TEST_DURATION); - -const endTest = makeTimer(2); - -const someTimer = makeTimer(1); -someTimer.domain = domain.create(); -someTimer.domain.dispose(); -someTimer._onTimeout = function() { - throw new Error('someTimer was not supposed to fire!'); -}; - -endTest._onTimeout = common.mustCall(function() { - assert.strictEqual(someTimer._idlePrev, null); - assert.strictEqual(someTimer._idleNext, null); - clearTimeout(keepOpen); -}); - -const cancelsTimer = makeTimer(1); -cancelsTimer._onTimeout = common.mustCall(function() { - someTimer._idleTimeout = 0; -}); - -timers._unrefActive(cancelsTimer); -timers._unrefActive(someTimer); -timers._unrefActive(endTest); - -function makeTimer(msecs) { - const timer = {}; - timers.unenroll(timer); - timers.enroll(timer, msecs); - return timer; -}