Skip to content

Commit

Permalink
src: revert domain using AsyncListeners
Browse files Browse the repository at this point in the history 
This is a slightly modified revert of bc39bdd.

Getting domains to use AsyncListeners became too much of a challenge
with many edge cases. While this is still a goal, it will have to be
deferred for now until more test coverage can be provided.
  • Loading branch information
@trevnorris
trevnorris committed Jan 9, 2014
1 parent 0afdfae commit 828f145
Show file tree
Hide file tree
Showing 13 changed files with 468 additions and 107 deletions.
4 changes: 0 additions & 4 deletions lib/_http_client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,12 +457,8 @@ ClientRequest.prototype.onSocket = function(socket) {
var req = this;

process.nextTick(function() {
// If a domain was added to the request, attach it to the socket.
if (req.domain)
socket._handle.addAsyncListener(req.domain._listener);
tickOnSocket(req, socket);
});

};

ClientRequest.prototype._deferToConnect = function(method, arguments_, cb) {
Expand Down
158 changes: 73 additions & 85 deletions lib/domain.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@ var inherits = util.inherits;
// a few side effects.
EventEmitter.usingDomains = true;

// overwrite process.domain with a getter/setter that will allow for more
// effective optimizations
var _domain = [null];
Object.defineProperty(process, 'domain', {
enumerable: true,
get: function() {
return _domain[0];
},
set: function(arg) {
return _domain[0] = arg;
}
});

// objects with external array data are excellent ways to communicate state
// between js and c++ w/o much overhead
var _domain_flag = {};

// let the process know we're using domains
process._setupDomainUse(_domain, _domain_flag);

exports.Domain = Domain;

exports.create = exports.createDomain = function() {
Expand All @@ -42,71 +62,66 @@ exports._stack = stack;
exports.active = null;


var listenerObj = {
error: function errorHandler(domain, 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 (domain._disposed)
return true;

er.domain = domain;
er.domainThrown = true;
// wrap this in a try/catch so we don't get infinite throwing
try {
// One of three things will happen here.
//
// 1. There is a handler, caught = true
// 2. There is no handler, caught = false
// 3. It throws, caught = false
//
// If caught is false after this, then there's no need to exit()
// the domain, because we're going to crash the process anyway.
caught = domain.emit('error', er);

if (stack.length === 0)
process.removeAsyncListener(domain._listener);

// Exit all domains on the stack. Uncaught exceptions end the
// current tick and no domains should be left on the stack
// between ticks.
stack.length = 0;
exports.active = process.domain = null;
} catch (er2) {
// The domain error handler threw! oh no!
// See if another domain can catch THIS error,
// or else crash on the original one.
// If the user already exited it, then don't double-exit.
if (domain === exports.active) {
stack.pop();
}
if (stack.length) {
exports.active = process.domain = stack[stack.length - 1];
caught = process._fatalException(er2);
} else {
caught = false;
}
return caught;
}
return caught;
}
};


inherits(Domain, EventEmitter);

function Domain() {
EventEmitter.call(this);

this.members = [];
this._listener = process.createAsyncListener(listenerObj, this);
}

Domain.prototype.members = undefined;
Domain.prototype._disposed = undefined;
Domain.prototype._listener = 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;

er.domain = this;
er.domainThrown = true;
// wrap this in a try/catch so we don't get infinite throwing
try {
// One of three things will happen here.
//
// 1. There is a handler, caught = true
// 2. There is no handler, caught = false
// 3. It throws, caught = false
//
// If caught is false after this, then there's no need to exit()
// the domain, because we're going to crash the process anyway.
caught = this.emit('error', er);

// Exit all domains on the stack. Uncaught exceptions end the
// current tick and no domains should be left on the stack
// between ticks.
stack.length = 0;
exports.active = process.domain = null;
} catch (er2) {
// The domain error handler threw! oh no!
// See if another domain can catch THIS error,
// or else crash on the original one.
// If the user already exited it, then don't double-exit.
if (this === exports.active) {
stack.pop();
}
if (stack.length) {
exports.active = process.domain = stack[stack.length - 1];
caught = process._fatalException(er2);
} else {
caught = false;
}
return caught;
}
return caught;
};


Domain.prototype.enter = function() {
Expand All @@ -116,22 +131,20 @@ Domain.prototype.enter = function() {
// to push it onto the stack so that we can pop it later.
exports.active = process.domain = this;
stack.push(this);

process.addAsyncListener(this._listener);
_domain_flag[0] = stack.length;
};


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

process.removeAsyncListener(this._listener);

// exit all domains until this one.
var index = stack.lastIndexOf(this);
if (index !== -1)
stack.splice(index + 1);
else
stack.length = 0;
_domain_flag[0] = stack.length;

exports.active = stack[stack.length - 1];
process.domain = exports.active;
Expand Down Expand Up @@ -165,13 +178,6 @@ Domain.prototype.add = function(ee) {

ee.domain = this;
this.members.push(ee);

// Adding the domain._listener to the Wrap associated with the event
// emitter instance will be done automatically either on class
// instantiation or manually, like in cases of net listen().
// The reason it cannot be done here is because in specific cases the
// _handle is not created on EE instantiation, so there's no place to
// add the listener.
};


Expand All @@ -180,24 +186,6 @@ Domain.prototype.remove = function(ee) {
var index = this.members.indexOf(ee);
if (index !== -1)
this.members.splice(index, 1);

// First check if the ee is a handle itself.
if (ee.removeAsyncListener)
ee.removeAsyncListener(this._listener);

// Manually remove the asyncListener from the handle, if possible.
if (ee._handle && ee._handle.removeAsyncListener)
ee._handle.removeAsyncListener(this._listener);

// TODO(trevnorris): Are there cases where the handle doesn't live on
// the ee or the _handle.

// TODO(trevnorris): For debugging that we've missed adding AsyncWrap's
// methods to a handle somewhere on the native side.
if (ee._handle && !ee._handle.removeAsyncListener) {
process._rawDebug('Wrap handle is missing AsyncWrap methods');
process.abort();
}
};


Expand Down
6 changes: 6 additions & 0 deletions lib/events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ EventEmitter.prototype.emit = function(type) {
if (util.isUndefined(handler))
return false;

if (this.domain && this !== process)
this.domain.enter();

if (util.isFunction(handler)) {
switch (arguments.length) {
// fast cases
Expand Down Expand Up @@ -123,6 +126,9 @@ EventEmitter.prototype.emit = function(type) {
listeners[i].apply(this, args);
}

if (this.domain && this !== process)
this.domain.exit();

return true;
};

Expand Down
11 changes: 0 additions & 11 deletions lib/net.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1089,20 +1089,9 @@ Server.prototype._listen2 = function(address, port, addressType, backlog, fd) {
// generate connection key, this should be unique to the connection
this._connectionKey = addressType + ':' + address + ':' + port;

// If a domain is attached to the event emitter then we need to add
// the listener to the handle.
if (this.domain) {
this._handle.addAsyncListener(this.domain._listener);
process.addAsyncListener(this.domain._listener);
}

process.nextTick(function() {
self.emit('listening');
});

if (this.domain) {
process.removeAsyncListener(this.domain._listener);
}
};


Expand Down
23 changes: 19 additions & 4 deletions lib/timers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,21 @@ function listOnTimeout() {
// other timers that expire on this tick should still run.
//
// https://github.com/joyent/node/issues/2631
if (first.domain && first.domain._disposed)
var domain = first.domain;
if (domain && domain._disposed)
continue;

hasQueue = !!first._asyncQueue;

try {
if (hasQueue)
loadAsyncQueue(first);
if (domain)
domain.enter();
threw = true;
first._onTimeout();
if (domain)
domain.exit();
if (hasQueue)
unloadAsyncQueue(first);
threw = false;
Expand Down Expand Up @@ -368,17 +373,20 @@ L.init(immediateQueue);

function processImmediate() {
var queue = immediateQueue;
var hasQueue, immediate;
var domain, hasQueue, immediate;

immediateQueue = {};
L.init(immediateQueue);

while (L.isEmpty(queue) === false) {
immediate = L.shift(queue);
hasQueue = !!immediate._asyncQueue;
domain = immediate.domain;

if (hasQueue)
loadAsyncQueue(immediate);
if (domain)
domain.enter();

var threw = true;
try {
Expand All @@ -398,6 +406,8 @@ function processImmediate() {
}
}

if (domain)
domain.exit();
if (hasQueue)
unloadAsyncQueue(immediate);
}
Expand Down Expand Up @@ -481,7 +491,7 @@ function unrefTimeout() {

debug('unrefTimer fired');

var diff, first, hasQueue, threw;
var diff, domain, first, hasQueue, threw;
while (first = L.peek(unrefList)) {
diff = now - first._idleStart;
hasQueue = !!first._asyncQueue;
Expand All @@ -496,16 +506,21 @@ function unrefTimeout() {

L.remove(first);

domain = first.domain;

if (!first._onTimeout) continue;
if (first.domain && first.domain._disposed) continue;
if (domain && domain._disposed) continue;

try {
if (hasQueue)
loadAsyncQueue(first);
if (domain) domain.enter();
threw = true;
debug('unreftimer firing timeout');
first._onTimeout();
threw = false;
if (domain)
domain.exit();
if (hasQueue)
unloadAsyncQueue(first);
} finally {
Expand Down
Loading

0 comments on commit 828f145

Please sign in to comment.