Permalink
Browse files

net: emit 'close' after 'end'

Currently the writable side of the socket is closed as soon as `UV_EOF`
is read regardless of the state of the socket. This allows the handle
to be closed before `'end'` is emitted and thus `'close'` can be
emitted before `'end'` if the socket is paused.

This commit prevents the handle from being closed until `'end'` is
emitted ensuring the correct order of events.

PR-URL: #19241
Fixes: #19166
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information...
lpinca committed Mar 8, 2018
1 parent 7455346 commit 9b7a6914a7f0bd754e78b42b48c75851cfd6b3c4
@@ -373,8 +373,6 @@ function afterShutdown(status, handle) {
if (self._readableState.ended) {
debug('readableState ended, destroying');
self.destroy();
} else {
self.once('_socketEnd', self.destroy);
}
}
@@ -530,6 +528,11 @@ Socket.prototype.end = function(data, encoding, callback) {
// Called when the 'end' event is emitted.
function onReadableStreamEnd() {
if (!this.allowHalfOpen) {
this.write = writeAfterFIN;
if (this.writable)
this.end();
}
maybeDestroy(this);
}
@@ -649,16 +652,6 @@ function onread(nread, buffer) {
// `end` -> `close`
self.push(null);
self.read(0);
if (!self.allowHalfOpen) {
self.write = writeAfterFIN;
self.destroySoon();
}
// internal end event so that we know that the actual socket
// is no longer readable, and we can start the shutdown
// procedure. No need to wait for all the data to be consumed.
self.emit('_socketEnd');
}
@@ -130,6 +130,7 @@ if (process.argv[2] === 'child') {
console.error('[m] CLIENT: close event');
disconnected += 1;
});
client.resume();
}
});
@@ -66,4 +66,5 @@ server.listen(0, () => {
// either way if it is, but we don't want to die if it is.
client.on('error', () => {});
client.on('close', common.mustCall(() => server.close()));
client.resume();
});
@@ -79,5 +79,9 @@ server.listen(0, function() {
console.log('close2');
server.close();
});
client2.resume();
});
client1.resume();
});
@@ -32,6 +32,7 @@ const server = net.createServer((socket) => {
assert.strictEqual(socket.bytesRead, prev);
assert.strictEqual(big.length, prev);
}));
socket.end();
});
socket.end();
});
@@ -31,23 +31,29 @@ const CLIENT_VARIANTS = 12;
});
// CLIENT_VARIANTS depends on the following code
net.connect(serverPath, getConnectCb());
net.connect(serverPath, getConnectCb()).resume();
net.connect(serverPath)
.on('connect', getConnectCb());
net.createConnection(serverPath, getConnectCb());
.on('connect', getConnectCb())
.resume();
net.createConnection(serverPath, getConnectCb()).resume();
net.createConnection(serverPath)
.on('connect', getConnectCb());
new net.Socket().connect(serverPath, getConnectCb());
.on('connect', getConnectCb())
.resume();
new net.Socket().connect(serverPath, getConnectCb()).resume();
new net.Socket().connect(serverPath)
.on('connect', getConnectCb());
net.connect({ path: serverPath }, getConnectCb());
.on('connect', getConnectCb())
.resume();
net.connect({ path: serverPath }, getConnectCb()).resume();
net.connect({ path: serverPath })
.on('connect', getConnectCb());
net.createConnection({ path: serverPath }, getConnectCb());
.on('connect', getConnectCb())
.resume();
net.createConnection({ path: serverPath }, getConnectCb()).resume();
net.createConnection({ path: serverPath })
.on('connect', getConnectCb());
new net.Socket().connect({ path: serverPath }, getConnectCb());
.on('connect', getConnectCb())
.resume();
new net.Socket().connect({ path: serverPath }, getConnectCb()).resume();
new net.Socket().connect({ path: serverPath })
.on('connect', getConnectCb());
.on('connect', getConnectCb())
.resume();
}));
}
@@ -102,27 +102,33 @@ const net = require('net');
function doConnect(args, getCb) {
return [
function createConnectionWithCb() {
return net.createConnection.apply(net, args.concat(getCb()));
return net.createConnection.apply(net, args.concat(getCb()))
.resume();
},
function createConnectionWithoutCb() {
return net.createConnection.apply(net, args)
.on('connect', getCb());
.on('connect', getCb())
.resume();
},
function connectWithCb() {
return net.connect.apply(net, args.concat(getCb()));
return net.connect.apply(net, args.concat(getCb()))
.resume();
},
function connectWithoutCb() {
return net.connect.apply(net, args)
.on('connect', getCb());
.on('connect', getCb())
.resume();
},
function socketConnectWithCb() {
const socket = new net.Socket();
return socket.connect.apply(socket, args.concat(getCb()));
return socket.connect.apply(socket, args.concat(getCb()))
.resume();
},
function socketConnectWithoutCb() {
const socket = new net.Socket();
return socket.connect.apply(socket, args)
.on('connect', getCb());
.on('connect', getCb())
.resume();
}
];
}
@@ -37,6 +37,8 @@ if (process.argv[2] === 'child') {
assert.strictEqual(server.connections, null);
server.close();
}));
connect.resume();
}));
});
@@ -0,0 +1,31 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const net = require('net');
const server = net.createServer();
server.on('connection', (socket) => {
let endEmitted = false;
socket.once('readable', () => {
setTimeout(() => {
socket.read();
}, common.platformTimeout(100));
});
socket.on('end', () => {
endEmitted = true;
});
socket.on('close', () => {
assert(endEmitted);
server.close();
});
socket.end('foo');
});
server.listen(common.mustCall(() => {
const socket = net.createConnection(server.address().port, () => {
socket.end('foo');
});
}));
@@ -73,5 +73,9 @@ server.listen(0, function() {
console.log('close2');
server.close();
});
client2.resume();
});
client1.resume();
});
@@ -42,6 +42,7 @@ const writes = [
let receivedWrites = 0;
const server = tls.createServer(options, function(c) {
c.resume();
writes.forEach(function(str) {
c.write(str);
});
@@ -26,6 +26,7 @@ const server = tls.createServer(options, function(s) {
const client = tls.connect(opts, function() {
putImmediate(client);
});
client.resume();
});
function putImmediate(client) {

0 comments on commit 9b7a691

Please sign in to comment.