Skip to content

Commit

Permalink
Revert "http: set IncomingMessage.destroyed"
Browse files Browse the repository at this point in the history
This reverts commit 28e6626.
PR-URL: #33686
Backport-PR-URL: #33686
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
  • Loading branch information
ronag authored and codebytere committed Jun 27, 2020
1 parent feb6e1f commit 6dbd63c
Show file tree
Hide file tree
Showing 10 changed files with 4 additions and 93 deletions.
3 changes: 0 additions & 3 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 1 addition & 8 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -714,7 +708,6 @@ function resOnFinish(req, res, socket, state, server) {
}

function emitCloseNT(self) {
self.destroyed = true;
self.emit('close');
}

Expand Down
41 changes: 0 additions & 41 deletions test/parallel/test-http-client-res-destroyed.js

This file was deleted.

5 changes: 1 addition & 4 deletions test/parallel/test-http-connect-req-res.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
5 changes: 1 addition & 4 deletions test/parallel/test-http-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 1 addition & 8 deletions test/parallel/test-http-pause-resume-one-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand All @@ -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());
}));
}));
12 changes: 0 additions & 12 deletions test/parallel/test-http-req-res-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
8 changes: 0 additions & 8 deletions test/parallel/test-http-response-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
'use strict';
const common = require('../common');
const http = require('http');
const assert = require('assert');

{
const server = http.createServer(
Expand All @@ -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();
}));
})
Expand All @@ -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();
Expand Down
3 changes: 0 additions & 3 deletions test/parallel/test-http-server-stale-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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();
});
Expand Down

0 comments on commit 6dbd63c

Please sign in to comment.