Skip to content

Commit 55e83cb

Browse files
dnlupnodejs-github-bot
authored andcommitted
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: #33035 Refs: #30625 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent eb14b10 commit 55e83cb

File tree

3 files changed

+13
-40
lines changed

3 files changed

+13
-40
lines changed

lib/_http_client.js

+2-18
Original file line numberDiff line numberDiff line change
@@ -430,25 +430,13 @@ function socketCloseListener() {
430430
req.destroyed = true;
431431
if (res) {
432432
// Socket closed before we emitted 'end' below.
433-
// TOOD(ronag): res.destroy(err)
434433
if (!res.complete) {
435-
res.aborted = true;
436-
res.emit('aborted');
437-
if (res.listenerCount('error') > 0) {
438-
res.emit('error', connResetException('aborted'));
439-
}
434+
res.destroy(connResetException('aborted'));
440435
}
441436
req._closed = true;
442437
req.emit('close');
443438
if (!res.aborted && res.readable) {
444-
res.on('end', function() {
445-
this.destroyed = true;
446-
this.emit('close');
447-
});
448439
res.push(null);
449-
} else {
450-
res.destroyed = true;
451-
res.emit('close');
452440
}
453441
} else {
454442
if (!req.socket._hadError) {
@@ -697,7 +685,6 @@ function responseKeepAlive(req) {
697685

698686
req.destroyed = true;
699687
if (req.res) {
700-
req.res.destroyed = true;
701688
// Detach socket from IncomingMessage to avoid destroying the freed
702689
// socket in IncomingMessage.destroy().
703690
req.res.socket = null;
@@ -752,13 +739,10 @@ function requestOnPrefinish() {
752739
function emitFreeNT(req) {
753740
req._closed = true;
754741
req.emit('close');
755-
if (req.res) {
756-
req.res.emit('close');
757-
}
758-
759742
if (req.socket) {
760743
req.socket.emit('free');
761744
}
745+
762746
}
763747

764748
function tickOnSocket(req, socket) {

lib/_http_incoming.js

+10-9
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function IncomingMessage(socket) {
5454
};
5555
}
5656

57-
Stream.Readable.call(this, { autoDestroy: false, ...streamOptions });
57+
Stream.Readable.call(this, streamOptions);
5858

5959
this._readableState.readingMore = true;
6060

@@ -160,19 +160,20 @@ IncomingMessage.prototype._read = function _read(n) {
160160
readStart(this.socket);
161161
};
162162

163-
164163
// It's possible that the socket will be destroyed, and removed from
165164
// any messages, before ever calling this. In that case, just skip
166165
// it, since something else is destroying this connection anyway.
167-
IncomingMessage.prototype.destroy = function destroy(error) {
168-
// TODO(ronag): Implement in terms of _destroy
169-
this.destroyed = true;
170-
if (this.socket)
171-
this.socket.destroy(error);
172-
return this;
166+
IncomingMessage.prototype._destroy = function _destroy(err, cb) {
167+
if (!this.readableEnded || !this.complete) {
168+
this.aborted = true;
169+
this.emit('aborted');
170+
}
171+
if (this.socket && !this.readableEnded) {
172+
this.socket.destroy(err);
173+
}
174+
this.listenerCount('error') > 0 ? cb(err) : cb();
173175
};
174176

175-
176177
IncomingMessage.prototype._addHeaderLines = _addHeaderLines;
177178
function _addHeaderLines(headers, n) {
178179
if (headers && headers.length) {

lib/_http_server.js

+1-13
Original file line numberDiff line numberDiff line change
@@ -575,14 +575,7 @@ function socketOnClose(socket, state) {
575575
function abortIncoming(incoming) {
576576
while (incoming.length) {
577577
const req = incoming.shift();
578-
// TODO(ronag): req.destroy(err)
579-
req.aborted = true;
580-
req.destroyed = true;
581-
req.emit('aborted');
582-
if (req.listenerCount('error') > 0) {
583-
req.emit('error', connResetException('aborted'));
584-
}
585-
req.emit('close');
578+
req.destroy(connResetException('aborted'));
586579
}
587580
// Abort socket._httpMessage ?
588581
}
@@ -741,14 +734,9 @@ function clearIncoming(req) {
741734
if (parser && parser.incoming === req) {
742735
if (req.readableEnded) {
743736
parser.incoming = null;
744-
req.destroyed = true;
745-
req.emit('close');
746737
} else {
747738
req.on('end', clearIncoming);
748739
}
749-
} else {
750-
req.destroyed = true;
751-
req.emit('close');
752740
}
753741
}
754742

0 commit comments

Comments
 (0)