summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniele Belardi <dwon.dnl@gmail.com>2020-05-19 08:29:00 +0200
committerMichaƫl Zasso <targos@protonmail.com>2020-12-21 18:24:28 +0100
commit25d7e9038609e19782fd10506932880608d72e8d (patch)
tree2bfb8be7a1451e8d9ee18650fd6cd87ebc2e0f13
parent7efb3111e834407e0a24916c956e2d4c027e6458 (diff)
downloadnode-new-25d7e9038609e19782fd10506932880608d72e8d.tar.gz
http: use `autoDestroy: true` in incoming message
Enable the default `autoDestroy: true` option in IncomingMessage. Refactor `_http_client` and `_http_server` to remove any manual destroying/closing of IncomingMessage. Refactor IncomingMessage `destroy` method to use the standard implementation of the stream module and move the early termination event emitting inside of it. PR-URL: https://github.com/nodejs/node/pull/33035 Refs: https://github.com/nodejs/node/issues/30625 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
-rw-r--r--lib/_http_client.js20
-rw-r--r--lib/_http_incoming.js19
-rw-r--r--lib/_http_server.js14
3 files changed, 13 insertions, 40 deletions
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 6fb5dd65cb..d647325d20 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -430,25 +430,13 @@ function socketCloseListener() {
req.destroyed = true;
if (res) {
// Socket closed before we emitted 'end' below.
- // TOOD(ronag): res.destroy(err)
if (!res.complete) {
- res.aborted = true;
- res.emit('aborted');
- if (res.listenerCount('error') > 0) {
- res.emit('error', connResetException('aborted'));
- }
+ res.destroy(connResetException('aborted'));
}
req._closed = true;
req.emit('close');
if (!res.aborted && res.readable) {
- res.on('end', function() {
- this.destroyed = true;
- this.emit('close');
- });
res.push(null);
- } else {
- res.destroyed = true;
- res.emit('close');
}
} else {
if (!req.socket._hadError) {
@@ -697,7 +685,6 @@ function responseKeepAlive(req) {
req.destroyed = true;
if (req.res) {
- req.res.destroyed = true;
// Detach socket from IncomingMessage to avoid destroying the freed
// socket in IncomingMessage.destroy().
req.res.socket = null;
@@ -752,13 +739,10 @@ function requestOnPrefinish() {
function emitFreeNT(req) {
req._closed = true;
req.emit('close');
- if (req.res) {
- req.res.emit('close');
- }
-
if (req.socket) {
req.socket.emit('free');
}
+
}
function tickOnSocket(req, socket) {
diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js
index 7943c69f54..ace18a83d4 100644
--- a/lib/_http_incoming.js
+++ b/lib/_http_incoming.js
@@ -54,7 +54,7 @@ function IncomingMessage(socket) {
};
}
- Stream.Readable.call(this, { autoDestroy: false, ...streamOptions });
+ Stream.Readable.call(this, streamOptions);
this._readableState.readingMore = true;
@@ -160,19 +160,20 @@ IncomingMessage.prototype._read = function _read(n) {
readStart(this.socket);
};
-
// It's possible that the socket will be destroyed, and removed from
// any messages, before ever calling this. In that case, just skip
// it, since something else is destroying this connection anyway.
-IncomingMessage.prototype.destroy = function destroy(error) {
- // TODO(ronag): Implement in terms of _destroy
- this.destroyed = true;
- if (this.socket)
- this.socket.destroy(error);
- return this;
+IncomingMessage.prototype._destroy = function _destroy(err, cb) {
+ if (!this.readableEnded || !this.complete) {
+ this.aborted = true;
+ this.emit('aborted');
+ }
+ if (this.socket && !this.readableEnded) {
+ this.socket.destroy(err);
+ }
+ this.listenerCount('error') > 0 ? cb(err) : cb();
};
-
IncomingMessage.prototype._addHeaderLines = _addHeaderLines;
function _addHeaderLines(headers, n) {
if (headers && headers.length) {
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 419b08a7a0..8cd10bb3a0 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -575,14 +575,7 @@ function socketOnClose(socket, state) {
function abortIncoming(incoming) {
while (incoming.length) {
const req = incoming.shift();
- // TODO(ronag): req.destroy(err)
- req.aborted = true;
- req.destroyed = true;
- req.emit('aborted');
- if (req.listenerCount('error') > 0) {
- req.emit('error', connResetException('aborted'));
- }
- req.emit('close');
+ req.destroy(connResetException('aborted'));
}
// Abort socket._httpMessage ?
}
@@ -741,14 +734,9 @@ function clearIncoming(req) {
if (parser && parser.incoming === req) {
if (req.readableEnded) {
parser.incoming = null;
- req.destroyed = true;
- req.emit('close');
} else {
req.on('end', clearIncoming);
}
- } else {
- req.destroyed = true;
- req.emit('close');
}
}