Permalink
Browse files

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.
  • Loading branch information...
1 parent 958ab66 commit fb3ec32b0ea5e42f6a9a8f0d5f43779f92cfac4a @bnoordhuis committed Mar 6, 2013
View
@@ -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);
+ }
+})();

0 comments on commit fb3ec32

Please sign in to comment.