From 7fdb507ec156b03e41be498d3b5eba314cba369f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 1 Jun 2020 18:38:07 +0200 Subject: [PATCH] Revert "http: set IncomingMessage.destroyed" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 28e6626ce7020b490438e3ee8a8188a59c5f856f. PR-URL: https://github.com/nodejs/node/pull/33686 Backport-PR-URL: https://github.com/nodejs/node/pull/33686 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Juan José Arboleda --- lib/_http_client.js | 3 -- lib/_http_incoming.js | 2 - lib/_http_server.js | 9 +--- .../test-http-client-res-destroyed.js | 41 ------------------- test/parallel/test-http-connect-req-res.js | 5 +-- test/parallel/test-http-connect.js | 5 +-- .../test-http-pause-resume-one-end.js | 9 +--- test/parallel/test-http-req-res-close.js | 12 ------ test/parallel/test-http-response-close.js | 8 ---- test/parallel/test-http-server-stale-close.js | 3 -- 10 files changed, 4 insertions(+), 93 deletions(-) delete mode 100644 test/parallel/test-http-client-res-destroyed.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 5cafd5455bd717..6d2a78c0b70356 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -426,12 +426,10 @@ function socketCloseListener() { 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 { @@ -545,7 +543,6 @@ function socketOnData(d) { socket.readableFlowing = null; req.emit(eventName, res, socket, bodyHead); - req.destroyed = true; req.emit('close'); } else { // Requested Upgrade or used CONNECT method, but have no handler. diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 59136833c6b340..a33d75c3e7900e 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -119,8 +119,6 @@ IncomingMessage.prototype._read = function _read(n) { // 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; diff --git a/lib/_http_server.js b/lib/_http_server.js index 9dfeef746636c5..c8b27dcd42cb5b 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -205,10 +205,7 @@ function onServerResponseClose() { // Ergo, we need to deal with stale 'close' events and handle the case // where the ServerResponse object has already been deconstructed. // Fortunately, that requires only a single if check. :-) - if (this._httpMessage) { - this._httpMessage.destroyed = true; - this._httpMessage.emit('close'); - } + if (this._httpMessage) this._httpMessage.emit('close'); } ServerResponse.prototype.assignSocket = function assignSocket(socket) { @@ -537,7 +534,6 @@ function abortIncoming(incoming) { while (incoming.length) { const req = incoming.shift(); req.aborted = true; - req.destroyed = true; req.emit('aborted'); req.emit('close'); } @@ -664,13 +660,11 @@ 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'); } } @@ -714,7 +708,6 @@ function resOnFinish(req, res, socket, state, server) { } function emitCloseNT(self) { - self.destroyed = true; self.emit('close'); } diff --git a/test/parallel/test-http-client-res-destroyed.js b/test/parallel/test-http-client-res-destroyed.js deleted file mode 100644 index 188ab06c155731..00000000000000 --- a/test/parallel/test-http-client-res-destroyed.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const http = require('http'); - -{ - const server = http.createServer(common.mustCall((req, res) => { - res.end('asd'); - })); - - server.listen(0, common.mustCall(() => { - http.get({ - port: server.address().port - }, common.mustCall((res) => { - assert.strictEqual(res.destroyed, false); - res.destroy(); - assert.strictEqual(res.destroyed, true); - res.on('close', common.mustCall(() => { - server.close(); - })); - })); - })); -} - -{ - const server = http.createServer(common.mustCall((req, res) => { - res.end('asd'); - })); - - server.listen(0, common.mustCall(() => { - http.get({ - port: server.address().port - }, common.mustCall((res) => { - assert.strictEqual(res.destroyed, false); - res.on('close', common.mustCall(() => { - assert.strictEqual(res.destroyed, true); - server.close(); - })).resume(); - })); - })); -} diff --git a/test/parallel/test-http-connect-req-res.js b/test/parallel/test-http-connect-req-res.js index cd6e55e0898c66..893c621852be89 100644 --- a/test/parallel/test-http-connect-req-res.js +++ b/test/parallel/test-http-connect-req-res.js @@ -33,10 +33,7 @@ server.listen(0, common.mustCall(function() { path: 'example.com:443' }, common.mustNotCall()); - assert.strictEqual(req.destroyed, false); - req.on('close', common.mustCall(() => { - assert.strictEqual(req.destroyed, true); - })); + req.on('close', common.mustCall()); req.on('connect', common.mustCall(function(res, socket, firstBodyChunk) { console.error('Client got CONNECT request'); diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js index 47fd7316d0cf1d..be853de347e0cb 100644 --- a/test/parallel/test-http-connect.js +++ b/test/parallel/test-http-connect.js @@ -62,10 +62,7 @@ server.listen(0, common.mustCall(() => { assert.strictEqual(socket._httpMessage, req); })); - assert.strictEqual(req.destroyed, false); - req.on('close', common.mustCall(() => { - assert.strictEqual(req.destroyed, true); - })); + req.on('close', common.mustCall()); req.on('connect', common.mustCall((res, socket, firstBodyChunk) => { // Make sure this request got removed from the pool. diff --git a/test/parallel/test-http-pause-resume-one-end.js b/test/parallel/test-http-pause-resume-one-end.js index 34bf3be478f09c..4886a7d490e413 100644 --- a/test/parallel/test-http-pause-resume-one-end.js +++ b/test/parallel/test-http-pause-resume-one-end.js @@ -22,7 +22,6 @@ 'use strict'; const common = require('../common'); const http = require('http'); -const assert = require('assert'); const server = http.Server(function(req, res) { res.writeHead(200, { 'Content-Type': 'text/plain' }); @@ -44,12 +43,6 @@ server.listen(0, common.mustCall(function() { }); })); - res.on('end', common.mustCall(() => { - assert.strictEqual(res.destroyed, false); - })); - assert.strictEqual(res.destroyed, false); - res.on('close', common.mustCall(() => { - assert.strictEqual(res.destroyed, true); - })); + res.on('end', common.mustCall()); })); })); diff --git a/test/parallel/test-http-req-res-close.js b/test/parallel/test-http-req-res-close.js index 329515ccd76ffb..daba55f43427c2 100644 --- a/test/parallel/test-http-req-res-close.js +++ b/test/parallel/test-http-req-res-close.js @@ -8,25 +8,13 @@ const server = http.Server(common.mustCall((req, res) => { let resClosed = false; res.end(); - let resFinished = false; res.on('finish', common.mustCall(() => { - resFinished = true; - assert.strictEqual(resClosed, false); - assert.strictEqual(res.destroyed, false); assert.strictEqual(resClosed, false); })); - assert.strictEqual(req.destroyed, false); res.on('close', common.mustCall(() => { resClosed = true; - assert.strictEqual(resFinished, true); - assert.strictEqual(res.destroyed, true); - })); - assert.strictEqual(req.destroyed, false); - req.on('end', common.mustCall(() => { - assert.strictEqual(req.destroyed, false); })); req.on('close', common.mustCall(() => { - assert.strictEqual(req.destroyed, true); assert.strictEqual(req._readableState.ended, true); })); res.socket.on('close', () => server.close()); diff --git a/test/parallel/test-http-response-close.js b/test/parallel/test-http-response-close.js index 0435cd0ee37da5..4c36003357980b 100644 --- a/test/parallel/test-http-response-close.js +++ b/test/parallel/test-http-response-close.js @@ -22,7 +22,6 @@ 'use strict'; const common = require('../common'); const http = require('http'); -const assert = require('assert'); { const server = http.createServer( @@ -40,9 +39,7 @@ const assert = require('assert'); res.on('data', common.mustCall(() => { res.destroy(); })); - assert.strictEqual(res.destroyed, false); res.on('close', common.mustCall(() => { - assert.strictEqual(res.destroyed, true); server.close(); })); }) @@ -64,12 +61,7 @@ const assert = require('assert'); http.get( { port: server.address().port }, common.mustCall((res) => { - assert.strictEqual(res.destroyed, false); - res.on('end', common.mustCall(() => { - assert.strictEqual(res.destroyed, false); - })); res.on('close', common.mustCall(() => { - assert.strictEqual(res.destroyed, true); server.close(); })); res.resume(); diff --git a/test/parallel/test-http-server-stale-close.js b/test/parallel/test-http-server-stale-close.js index b9322ed9fcf83a..d67c4d31c36e37 100644 --- a/test/parallel/test-http-server-stale-close.js +++ b/test/parallel/test-http-server-stale-close.js @@ -23,7 +23,6 @@ require('../common'); const http = require('http'); const fork = require('child_process').fork; -const assert = require('assert'); if (process.env.NODE_TEST_FORK_PORT) { const req = http.request({ @@ -38,9 +37,7 @@ if (process.env.NODE_TEST_FORK_PORT) { const server = http.createServer((req, res) => { res.writeHead(200, { 'Content-Length': '42' }); req.pipe(res); - assert.strictEqual(req.destroyed, false); req.on('close', () => { - assert.strictEqual(req.destroyed, true); server.close(); res.end(); });