Skip to content

Commit 828f145

Browse files
trevnorristrevnorris
committed
src: revert domain using AsyncListeners
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.
1 parent 0afdfae commit 828f145

13 files changed

Lines changed: 468 additions & 107 deletions

File tree

‎lib/_http_client.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,12 +457,8 @@ ClientRequest.prototype.onSocket = function(socket) {
457457
var req = this;
458458

459459
process.nextTick(function() {
460-
// If a domain was added to the request, attach it to the socket.
461-
if (req.domain)
462-
socket._handle.addAsyncListener(req.domain._listener);
463460
tickOnSocket(req, socket);
464461
});
465-
466462
};
467463

468464
ClientRequest.prototype._deferToConnect = function(method, arguments_, cb) {

‎lib/domain.js‎

Lines changed: 73 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@ var inherits = util.inherits;
2828
// a few side effects.
2929
EventEmitter.usingDomains = true;
3030

31+
// overwrite process.domain with a getter/setter that will allow for more
32+
// effective optimizations
33+
var _domain = [null];
34+
Object.defineProperty(process, 'domain', {
35+
enumerable: true,
36+
get: function() {
37+
return _domain[0];
38+
},
39+
set: function(arg) {
40+
return _domain[0] = arg;
41+
}
42+
});
43+
44+
// objects with external array data are excellent ways to communicate state
45+
// between js and c++ w/o much overhead
46+
var _domain_flag = {};
47+
48+
// let the process know we're using domains
49+
process._setupDomainUse(_domain, _domain_flag);
50+
3151
exports.Domain = Domain;
3252

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

4464

45-
var listenerObj = {
46-
error: function errorHandler(domain, er) {
47-
var caught = false;
48-
// ignore errors on disposed domains.
49-
//
50-
// XXX This is a bit stupid. We should probably get rid of
51-
// domain.dispose() altogether. It's almost always a terrible
52-
// idea. --isaacs
53-
if (domain._disposed)
54-
return true;
55-
56-
er.domain = domain;
57-
er.domainThrown = true;
58-
// wrap this in a try/catch so we don't get infinite throwing
59-
try {
60-
// One of three things will happen here.
61-
//
62-
// 1. There is a handler, caught = true
63-
// 2. There is no handler, caught = false
64-
// 3. It throws, caught = false
65-
//
66-
// If caught is false after this, then there's no need to exit()
67-
// the domain, because we're going to crash the process anyway.
68-
caught = domain.emit('error', er);
69-
70-
if (stack.length === 0)
71-
process.removeAsyncListener(domain._listener);
72-
73-
// Exit all domains on the stack. Uncaught exceptions end the
74-
// current tick and no domains should be left on the stack
75-
// between ticks.
76-
stack.length = 0;
77-
exports.active = process.domain = null;
78-
} catch (er2) {
79-
// The domain error handler threw! oh no!
80-
// See if another domain can catch THIS error,
81-
// or else crash on the original one.
82-
// If the user already exited it, then don't double-exit.
83-
if (domain === exports.active) {
84-
stack.pop();
85-
}
86-
if (stack.length) {
87-
exports.active = process.domain = stack[stack.length - 1];
88-
caught = process._fatalException(er2);
89-
} else {
90-
caught = false;
91-
}
92-
return caught;
93-
}
94-
return caught;
95-
}
96-
};
97-
98-
9965
inherits(Domain, EventEmitter);
10066

10167
function Domain() {
10268
EventEmitter.call(this);
69+
10370
this.members = [];
104-
this._listener = process.createAsyncListener(listenerObj, this);
10571
}
10672

10773
Domain.prototype.members = undefined;
10874
Domain.prototype._disposed = undefined;
109-
Domain.prototype._listener = undefined;
75+
76+
77+
// Called by process._fatalException in case an error was thrown.
78+
Domain.prototype._errorHandler = function errorHandler(er) {
79+
var caught = false;
80+
// ignore errors on disposed domains.
81+
//
82+
// XXX This is a bit stupid. We should probably get rid of
83+
// domain.dispose() altogether. It's almost always a terrible
84+
// idea. --isaacs
85+
if (this._disposed)
86+
return true;
87+
88+
er.domain = this;
89+
er.domainThrown = true;
90+
// wrap this in a try/catch so we don't get infinite throwing
91+
try {
92+
// One of three things will happen here.
93+
//
94+
// 1. There is a handler, caught = true
95+
// 2. There is no handler, caught = false
96+
// 3. It throws, caught = false
97+
//
98+
// If caught is false after this, then there's no need to exit()
99+
// the domain, because we're going to crash the process anyway.
100+
caught = this.emit('error', er);
101+
102+
// Exit all domains on the stack. Uncaught exceptions end the
103+
// current tick and no domains should be left on the stack
104+
// between ticks.
105+
stack.length = 0;
106+
exports.active = process.domain = null;
107+
} catch (er2) {
108+
// The domain error handler threw! oh no!
109+
// See if another domain can catch THIS error,
110+
// or else crash on the original one.
111+
// If the user already exited it, then don't double-exit.
112+
if (this === exports.active) {
113+
stack.pop();
114+
}
115+
if (stack.length) {
116+
exports.active = process.domain = stack[stack.length - 1];
117+
caught = process._fatalException(er2);
118+
} else {
119+
caught = false;
120+
}
121+
return caught;
122+
}
123+
return caught;
124+
};
110125

111126

112127
Domain.prototype.enter = function() {
@@ -116,22 +131,20 @@ Domain.prototype.enter = function() {
116131
// to push it onto the stack so that we can pop it later.
117132
exports.active = process.domain = this;
118133
stack.push(this);
119-
120-
process.addAsyncListener(this._listener);
134+
_domain_flag[0] = stack.length;
121135
};
122136

123137

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

127-
process.removeAsyncListener(this._listener);
128-
129141
// exit all domains until this one.
130142
var index = stack.lastIndexOf(this);
131143
if (index !== -1)
132144
stack.splice(index + 1);
133145
else
134146
stack.length = 0;
147+
_domain_flag[0] = stack.length;
135148

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

166179
ee.domain = this;
167180
this.members.push(ee);
168-
169-
// Adding the domain._listener to the Wrap associated with the event
170-
// emitter instance will be done automatically either on class
171-
// instantiation or manually, like in cases of net listen().
172-
// The reason it cannot be done here is because in specific cases the
173-
// _handle is not created on EE instantiation, so there's no place to
174-
// add the listener.
175181
};
176182

177183

@@ -180,24 +186,6 @@ Domain.prototype.remove = function(ee) {
180186
var index = this.members.indexOf(ee);
181187
if (index !== -1)
182188
this.members.splice(index, 1);
183-
184-
// First check if the ee is a handle itself.
185-
if (ee.removeAsyncListener)
186-
ee.removeAsyncListener(this._listener);
187-
188-
// Manually remove the asyncListener from the handle, if possible.
189-
if (ee._handle && ee._handle.removeAsyncListener)
190-
ee._handle.removeAsyncListener(this._listener);
191-
192-
// TODO(trevnorris): Are there cases where the handle doesn't live on
193-
// the ee or the _handle.
194-
195-
// TODO(trevnorris): For debugging that we've missed adding AsyncWrap's
196-
// methods to a handle somewhere on the native side.
197-
if (ee._handle && !ee._handle.removeAsyncListener) {
198-
process._rawDebug('Wrap handle is missing AsyncWrap methods');
199-
process.abort();
200-
}
201189
};
202190

203191

‎lib/events.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ EventEmitter.prototype.emit = function(type) {
9191
if (util.isUndefined(handler))
9292
return false;
9393

94+
if (this.domain && this !== process)
95+
this.domain.enter();
96+
9497
if (util.isFunction(handler)) {
9598
switch (arguments.length) {
9699
// fast cases
@@ -123,6 +126,9 @@ EventEmitter.prototype.emit = function(type) {
123126
listeners[i].apply(this, args);
124127
}
125128

129+
if (this.domain && this !== process)
130+
this.domain.exit();
131+
126132
return true;
127133
};
128134

‎lib/net.js‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,20 +1089,9 @@ Server.prototype._listen2 = function(address, port, addressType, backlog, fd) {
10891089
// generate connection key, this should be unique to the connection
10901090
this._connectionKey = addressType + ':' + address + ':' + port;
10911091

1092-
// If a domain is attached to the event emitter then we need to add
1093-
// the listener to the handle.
1094-
if (this.domain) {
1095-
this._handle.addAsyncListener(this.domain._listener);
1096-
process.addAsyncListener(this.domain._listener);
1097-
}
1098-
10991092
process.nextTick(function() {
11001093
self.emit('listening');
11011094
});
1102-
1103-
if (this.domain) {
1104-
process.removeAsyncListener(this.domain._listener);
1105-
}
11061095
};
11071096

11081097

‎lib/timers.js‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,21 @@ function listOnTimeout() {
112112
// other timers that expire on this tick should still run.
113113
//
114114
// https://github.com/joyent/node/issues/2631
115-
if (first.domain && first.domain._disposed)
115+
var domain = first.domain;
116+
if (domain && domain._disposed)
116117
continue;
117118

118119
hasQueue = !!first._asyncQueue;
119120

120121
try {
121122
if (hasQueue)
122123
loadAsyncQueue(first);
124+
if (domain)
125+
domain.enter();
123126
threw = true;
124127
first._onTimeout();
128+
if (domain)
129+
domain.exit();
125130
if (hasQueue)
126131
unloadAsyncQueue(first);
127132
threw = false;
@@ -368,17 +373,20 @@ L.init(immediateQueue);
368373

369374
function processImmediate() {
370375
var queue = immediateQueue;
371-
var hasQueue, immediate;
376+
var domain, hasQueue, immediate;
372377

373378
immediateQueue = {};
374379
L.init(immediateQueue);
375380

376381
while (L.isEmpty(queue) === false) {
377382
immediate = L.shift(queue);
378383
hasQueue = !!immediate._asyncQueue;
384+
domain = immediate.domain;
379385

380386
if (hasQueue)
381387
loadAsyncQueue(immediate);
388+
if (domain)
389+
domain.enter();
382390

383391
var threw = true;
384392
try {
@@ -398,6 +406,8 @@ function processImmediate() {
398406
}
399407
}
400408

409+
if (domain)
410+
domain.exit();
401411
if (hasQueue)
402412
unloadAsyncQueue(immediate);
403413
}
@@ -481,7 +491,7 @@ function unrefTimeout() {
481491

482492
debug('unrefTimer fired');
483493

484-
var diff, first, hasQueue, threw;
494+
var diff, domain, first, hasQueue, threw;
485495
while (first = L.peek(unrefList)) {
486496
diff = now - first._idleStart;
487497
hasQueue = !!first._asyncQueue;
@@ -496,16 +506,21 @@ function unrefTimeout() {
496506

497507
L.remove(first);
498508

509+
domain = first.domain;
510+
499511
if (!first._onTimeout) continue;
500-
if (first.domain && first.domain._disposed) continue;
512+
if (domain && domain._disposed) continue;
501513

502514
try {
503515
if (hasQueue)
504516
loadAsyncQueue(first);
517+
if (domain) domain.enter();
505518
threw = true;
506519
debug('unreftimer firing timeout');
507520
first._onTimeout();
508521
threw = false;
522+
if (domain)
523+
domain.exit();
509524
if (hasQueue)
510525
unloadAsyncQueue(first);
511526
} finally {

0 commit comments

Comments
 (0)