net: use close callback, not process.nextTick
Don't emit the 'close' event with process.nextTick. Closing a handle is an operation that usually *but not always* completes on the next tick of the event loop, hence using process.nextTick is not reliable. Use a proper handle close callback and emit the 'close' event from inside the callback. Update tests that depend on the intricacies of the old model. Fixes #3459.
This commit is contained in:
parent
958ab661cb
commit
fb3ec32b0e
12
lib/net.js
12
lib/net.js
@ -430,18 +430,16 @@ Socket.prototype._destroy = function(exception, cb) {
|
||||
if (this._handle) {
|
||||
if (this !== process.stderr)
|
||||
debug('close handle');
|
||||
this._handle.close();
|
||||
var isException = exception ? true : false;
|
||||
this._handle.close(function() {
|
||||
debug('emit close');
|
||||
self.emit('close', isException);
|
||||
});
|
||||
this._handle.onread = noop;
|
||||
this._handle = null;
|
||||
}
|
||||
|
||||
fireErrorCallbacks();
|
||||
|
||||
process.nextTick(function() {
|
||||
debug('emit close');
|
||||
self.emit('close', exception ? true : false);
|
||||
});
|
||||
|
||||
this.destroyed = true;
|
||||
|
||||
if (this.server) {
|
||||
|
@ -65,7 +65,7 @@ var request1 = http.get(requestOptions, function(response) {
|
||||
// is triggered.
|
||||
request1.socket.destroy();
|
||||
|
||||
process.nextTick(function() {
|
||||
response.once('close', function() {
|
||||
// assert request2 was removed from the queue
|
||||
assert(!agent.requests[key]);
|
||||
console.log("waiting for request2.onSocket's nextTick");
|
||||
|
@ -28,7 +28,11 @@ var sockets = [];
|
||||
|
||||
process.on('exit', function() {
|
||||
assert.equal(server.connections, 0);
|
||||
assert.deepEqual(events, 'client client server'.split(' '));
|
||||
assert.equal(events.length, 3);
|
||||
// Expect to see one server event and two client events. The order of the
|
||||
// events is undefined because they arrive on the same event loop tick.
|
||||
assert.equal(events.join(' ').match(/server/g).length, 1);
|
||||
assert.equal(events.join(' ').match(/client/g).length, 2);
|
||||
});
|
||||
|
||||
var server = net.createServer(function(c) {
|
||||
|
@ -29,12 +29,15 @@ function expect(activeHandles, activeRequests) {
|
||||
assert.equal(process._getActiveRequests().length, activeRequests);
|
||||
}
|
||||
|
||||
var handles = [];
|
||||
|
||||
(function() {
|
||||
expect(0, 0);
|
||||
var server = net.createServer().listen(common.PORT);
|
||||
expect(1, 0);
|
||||
server.close();
|
||||
expect(1, 0); // server handle doesn't shut down until next tick
|
||||
handles.push(server);
|
||||
})();
|
||||
|
||||
(function() {
|
||||
@ -44,18 +47,15 @@ function expect(activeHandles, activeRequests) {
|
||||
expect(2, 1);
|
||||
conn.destroy();
|
||||
expect(2, 1); // client handle doesn't shut down until next tick
|
||||
handles.push(conn);
|
||||
})();
|
||||
|
||||
// Force the nextTicks to be deferred to a later time.
|
||||
process.maxTickDepth = 1;
|
||||
|
||||
process.nextTick(function() {
|
||||
process.nextTick(function() {
|
||||
process.nextTick(function() {
|
||||
process.nextTick(function() {
|
||||
// the handles should be gone but the connect req could still be alive
|
||||
assert.equal(process._getActiveHandles().length, 0);
|
||||
});
|
||||
});
|
||||
(function() {
|
||||
var n = 0;
|
||||
handles.forEach(function(handle) {
|
||||
handle.once('close', onclose);
|
||||
});
|
||||
});
|
||||
function onclose() {
|
||||
if (++n === handles.length) setImmediate(expect, 0, 0);
|
||||
}
|
||||
})();
|
||||
|
Loading…
x
Reference in New Issue
Block a user