Skip to content

Commit

Permalink
domain: remove .dispose()
Browse files Browse the repository at this point in the history
`domain.dispose()` is generally considered an anti-pattern, has been
runtime-deprecated for over 4 years, and is a part of the `domain`
module that can not be emulated by `async_hooks`; so remove it.

Ref: https://nodejs.org/en/docs/guides/domain-postmortem/
Ref: 4a74fc9
  • Loading branch information
addaleax committed Sep 19, 2017
1 parent 1976654 commit 77da410
Show file tree
Hide file tree
Showing 12 changed files with 17 additions and 336 deletions.
4 changes: 2 additions & 2 deletions doc/api/deprecations.md
Expand Up @@ -146,9 +146,9 @@ instead.
<a id="DEP0012"></a>
### 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.

<a id="DEP0013"></a>
Expand Down
18 changes: 1 addition & 17 deletions doc/api/domain.md
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
48 changes: 4 additions & 44 deletions lib/domain.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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.
Expand Down Expand Up @@ -224,9 +212,6 @@ Domain.prototype.remove = function(ee) {


Domain.prototype.run = function(fn) {
if (this._disposed)
return;

var ret;

this.enter();
Expand All @@ -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, {
Expand Down Expand Up @@ -291,9 +273,6 @@ Domain.prototype.intercept = function(cb) {


function bound(_this, self, cb, fnargs) {
if (self._disposed)
return;

var ret;

self.enter();
Expand All @@ -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');
9 changes: 0 additions & 9 deletions lib/timers.js
Expand Up @@ -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();
}

Expand Down
1 change: 0 additions & 1 deletion src/env.h
Expand Up @@ -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") \
Expand Down
17 changes: 4 additions & 13 deletions src/node.cc
Expand Up @@ -1162,12 +1162,10 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) {
}


bool DomainEnter(Environment* env, Local<Object> object) {
void DomainEnter(Environment* env, Local<Object> object) {
Local<Value> domain_v = object->Get(env->domain_string());
if (domain_v->IsObject()) {
Local<Object> domain = domain_v.As<Object>();
if (domain->Get(env->disposed_string())->IsTrue())
return true;
Local<Value> enter_v = domain->Get(env->enter_string());
if (enter_v->IsFunction()) {
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
Expand All @@ -1176,16 +1174,13 @@ bool DomainEnter(Environment* env, Local<Object> object) {
}
}
}
return false;
}


bool DomainExit(Environment* env, v8::Local<v8::Object> object) {
void DomainExit(Environment* env, v8::Local<v8::Object> object) {
Local<Value> domain_v = object->Get(env->domain_string());
if (domain_v->IsObject()) {
Local<Object> domain = domain_v.As<Object>();
if (domain->Get(env->disposed_string())->IsTrue())
return true;
Local<Value> exit_v = domain->Get(env->exit_string());
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
Expand All @@ -1194,7 +1189,6 @@ bool DomainExit(Environment* env, v8::Local<v8::Object> object) {
}
}
}
return false;
}


Expand Down Expand Up @@ -1401,9 +1395,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) {
Expand Down Expand Up @@ -1435,8 +1427,7 @@ void InternalCallbackScope::Close() {
}

if (env_->using_domains()) {
failed_ = DomainExit(env_, object_);
if (failed_) return;
DomainExit(env_, object_);
}

if (IsInnerMakeCallback()) {
Expand Down
25 changes: 0 additions & 25 deletions test/parallel/test-domain-abort-when-disposed.js

This file was deleted.

97 changes: 0 additions & 97 deletions test/parallel/test-domain-exit-dispose-again.js

This file was deleted.

0 comments on commit 77da410

Please sign in to comment.