Skip to content

Commit

Permalink
net: require two events to destroy a socket
Browse files Browse the repository at this point in the history
Closing a TCP connection requires 2 FIN/ACK
exchanges, so we should wait for both events
before destroying a socket

Fixes: nodejs#36180
  • Loading branch information
mmomtchev committed Nov 20, 2020
1 parent cbfa2d1 commit dd2e3ed
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
14 changes: 12 additions & 2 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -631,10 +631,20 @@ Socket.prototype.destroySoon = function() {
if (this.writable)
this.end();

if (this.writableFinished)
if (this.writableFinished && this.readableEnded)
this.destroy();
else
this.once('finish', this.destroy);
this.once('finish', () => {
// If we destroy the socket before we have received an incoming EOF
// the remote might still be sending data that will trigger a RST
// from this host once it is received
// Closing a TCP socket requires two FIN/ACK exchanges, one in every
// direction - this means two events
if (this.readableEnded)
this.destroy();
else
this.once('end', this.destroy);
});
};


Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-should-keep-alive.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const countdown = new Countdown(SHOULD_KEEP_ALIVE.length, () => server.close());
const getCountdownIndex = () => SERVER_RESPONSES.length - countdown.remaining;

const server = net.createServer(function(socket) {
socket.write(SERVER_RESPONSES[getCountdownIndex()]);
socket.end(SERVER_RESPONSES[getCountdownIndex()]);
}).listen(0, function() {
function makeRequest() {
const req = http.get({ port: server.address().port }, function(res) {
Expand Down

0 comments on commit dd2e3ed

Please sign in to comment.