Commit 25d7e90
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 <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>1 parent 7efb311 commit 25d7e90
3 files changed
Lines changed: 13 additions & 40 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -430,25 +430,13 @@ function socketCloseListener() { | |||
| 430 | 430 | req.destroyed = true; | |
| 431 | 431 | if (res) { | |
| 432 | 432 | // Socket closed before we emitted 'end' below. | |
| 433 | - // TOOD(ronag): res.destroy(err) | ||
| 434 | 433 | 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')); | ||
| 440 | 435 | } | |
| 441 | 436 | req._closed = true; | |
| 442 | 437 | req.emit('close'); | |
| 443 | 438 | if (!res.aborted && res.readable) { | |
| 444 | - res.on('end', function() { | ||
| 445 | - this.destroyed = true; | ||
| 446 | - this.emit('close'); | ||
| 447 | - }); | ||
| 448 | 439 | res.push(null); | |
| 449 | - } else { | ||
| 450 | - res.destroyed = true; | ||
| 451 | - res.emit('close'); | ||
| 452 | 440 | } | |
| 453 | 441 | } else { | |
| 454 | 442 | if (!req.socket._hadError) { | |
@@ -697,7 +685,6 @@ function responseKeepAlive(req) { | |||
| 697 | 685 | ||
| 698 | 686 | req.destroyed = true; | |
| 699 | 687 | if (req.res) { | |
| 700 | - req.res.destroyed = true; | ||
| 701 | 688 | // Detach socket from IncomingMessage to avoid destroying the freed | |
| 702 | 689 | // socket in IncomingMessage.destroy(). | |
| 703 | 690 | req.res.socket = null; | |
@@ -752,13 +739,10 @@ function requestOnPrefinish() { | |||
| 752 | 739 | function emitFreeNT(req) { | |
| 753 | 740 | req._closed = true; | |
| 754 | 741 | req.emit('close'); | |
| 755 | - if (req.res) { | ||
| 756 | - req.res.emit('close'); | ||
| 757 | - } | ||
| 758 | - | ||
| 759 | 742 | if (req.socket) { | |
| 760 | 743 | req.socket.emit('free'); | |
| 761 | 744 | } | |
| 745 | + | ||
| 762 | 746 | } | |
| 763 | 747 | ||
| 764 | 748 | function tickOnSocket(req, socket) { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -54,7 +54,7 @@ function IncomingMessage(socket) { | |||
| 54 | 54 | }; | |
| 55 | 55 | } | |
| 56 | 56 | ||
| 57 | - Stream.Readable.call(this, { autoDestroy: false, ...streamOptions }); | ||
| 57 | + Stream.Readable.call(this, streamOptions); | ||
| 58 | 58 | ||
| 59 | 59 | this._readableState.readingMore = true; | |
| 60 | 60 | ||
@@ -160,19 +160,20 @@ IncomingMessage.prototype._read = function _read(n) { | |||
| 160 | 160 | readStart(this.socket); | |
| 161 | 161 | }; | |
| 162 | 162 | ||
| 163 | - | ||
| 164 | 163 | // It's possible that the socket will be destroyed, and removed from | |
| 165 | 164 | // any messages, before ever calling this. In that case, just skip | |
| 166 | 165 | // 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(); | ||
| 173 | 175 | }; | |
| 174 | 176 | ||
| 175 | - | ||
| 176 | 177 | IncomingMessage.prototype._addHeaderLines = _addHeaderLines; | |
| 177 | 178 | function _addHeaderLines(headers, n) { | |
| 178 | 179 | if (headers && headers.length) { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -575,14 +575,7 @@ function socketOnClose(socket, state) { | |||
| 575 | 575 | function abortIncoming(incoming) { | |
| 576 | 576 | while (incoming.length) { | |
| 577 | 577 | 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')); | ||
| 586 | 579 | } | |
| 587 | 580 | // Abort socket._httpMessage ? | |
| 588 | 581 | } | |
@@ -741,14 +734,9 @@ function clearIncoming(req) { | |||
| 741 | 734 | if (parser && parser.incoming === req) { | |
| 742 | 735 | if (req.readableEnded) { | |
| 743 | 736 | parser.incoming = null; | |
| 744 | - req.destroyed = true; | ||
| 745 | - req.emit('close'); | ||
| 746 | 737 | } else { | |
| 747 | 738 | req.on('end', clearIncoming); | |
| 748 | 739 | } | |
| 749 | - } else { | ||
| 750 | - req.destroyed = true; | ||
| 751 | - req.emit('close'); | ||
| 752 | 740 | } | |
| 753 | 741 | } | |
| 754 | 742 | ||
0 commit comments