Skip to content

Commit

Permalink
net: add support for finished after .destroy()
Browse files Browse the repository at this point in the history
Calling `finished(socket, cb)` would previously not
invoked the callback if the socket was already detroyed.

PR-URL: nodejs#36635
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ronag authored and jasnell committed Jan 4, 2021
1 parent 64a4001 commit 2da3611
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 7 deletions.
8 changes: 2 additions & 6 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,8 @@ IncomingMessage.prototype._destroy = function _destroy(err, cb) {
this.emit('aborted');
}

// If aborted and the underlying socket is not already destroyed,
// destroy it.
// We have to check if the socket is already destroyed because finished
// does not call the callback when this methdod is invoked from `_http_client`
// in `test/parallel/test-http-client-spurious-aborted.js`
if (this.socket && !this.socket.destroyed && this.aborted) {
// If aborted destroy socket.
if (this.socket && this.aborted) {
this.socket.destroy(err);
const cleanup = finished(this.socket, (e) => {
cleanup();
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/streams/end-of-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ const {
} = require('internal/errors').codes;
const { once } = require('internal/util');

function isSocket(stream) {
return (
typeof stream.connect === 'function' &&
typeof stream.setNoDelay === 'function' &&
typeof stream.setKeepAlive === 'function'
);
}

function isRequest(stream) {
return stream.setHeader && typeof stream.abort === 'function';
}
Expand Down Expand Up @@ -75,7 +83,7 @@ function eos(stream, options, callback) {
let willEmitClose = (
state &&
state.autoDestroy &&
state.emitClose &&
(state.emitClose || isSocket(stream)) &&
state.closed === false &&
isReadable(stream) === readable &&
isWritable(stream) === writable
Expand Down
12 changes: 12 additions & 0 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,12 @@ Socket.prototype._destroy = function(exception, cb) {

this._handle.close(() => {
debug('emit close');
if (this._writableState) {
this._writableState.closed = true;
}
if (this._readableState) {
this._readableState.closed = true;
}
this.emit('close', isException);
});
this._handle.onread = noop;
Expand Down Expand Up @@ -1651,6 +1657,12 @@ Server.prototype._emitCloseIfDrained = function() {

function emitCloseNT(self) {
debug('SERVER: emit close');
if (self._writableState) {
self._writableState.closed = true;
}
if (self._readableState) {
self._readableState.closed = true;
}
self.emit('close');
}

Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-net-finished.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';
const common = require('../common');
const net = require('net');
const { finished } = require('stream');

const server = net.createServer(function(con) {
con.on('close', common.mustCall());
});

server.listen(0, common.mustCall(function() {
const con = net.connect({
port: this.address().port
})
.on('connect', () => {
finished(con, common.mustCall());
con.destroy();
finished(con, common.mustCall());
con.on('close', common.mustCall(() => {
finished(con, common.mustCall(() => {
server.close();
}));
}));
})
.end();
}));

0 comments on commit 2da3611

Please sign in to comment.