Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 60a585e

Browse filesBrowse files
ronagaduh95
authored andcommitted
http: cleanup pipeline queue
When a socket with pipelined requests is destroyed then some requests will leak. PR-URL: #62534 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 9047ec1 commit 60a585e
Copy full SHA for 60a585e

4 files changed

+92-4Lines changed: 92 additions & 4 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+8-3Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,14 +328,19 @@ OutgoingMessage.prototype.destroy = function destroy(error) {
328328
if (this[kSocket]) {
329329
this[kSocket].destroy(error);
330330
} else {
331-
this.once('socket', function socketDestroyOnConnect(socket) {
332-
socket.destroy(error);
333-
});
331+
process.nextTick(emitDestroyNT, this);
334332
}
335333

336334
return this;
337335
};
338336

337+
function emitDestroyNT(self) {
338+
if (!self._closed) {
339+
self._closed = true;
340+
self.emit('close');
341+
}
342+
}
343+
339344

340345
// This abstract either writing directly to the socket or buffering it.
341346
OutgoingMessage.prototype._send = function _send(data, encoding, callback, byteLength) {
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,14 +841,21 @@ function socketOnClose(socket, state) {
841841
debug('server socket close');
842842
freeParser(socket.parser, null, socket);
843843
abortIncoming(state.incoming);
844+
abortOutgoing(state.outgoing);
844845
}
845846

846847
function abortIncoming(incoming) {
847848
while (incoming.length) {
848849
const req = incoming.shift();
849850
req.destroy(new ConnResetException('aborted'));
850851
}
851-
// Abort socket._httpMessage ?
852+
}
853+
854+
function abortOutgoing(outgoing) {
855+
while (outgoing.length) {
856+
const req = outgoing.shift();
857+
req.destroy(new ConnResetException('aborted'));
858+
}
852859
}
853860

854861
function socketOnEnd(server, socket, parser, state) {
Collapse file

‎test/parallel/test-http-outgoing-destroyed.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-outgoing-destroyed.js
+26Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,32 @@
22
const common = require('../common');
33
const http = require('http');
44
const assert = require('assert');
5+
const { OutgoingMessage } = require('http');
6+
7+
// OutgoingMessage.destroy() with no socket should emit 'close' and set closed.
8+
{
9+
const msg = new OutgoingMessage();
10+
assert.strictEqual(msg.destroyed, false);
11+
assert.strictEqual(msg.closed, false);
12+
msg.on('close', common.mustCall(() => {
13+
assert.strictEqual(msg.destroyed, true);
14+
assert.strictEqual(msg.closed, true);
15+
}));
16+
msg.destroy();
17+
assert.strictEqual(msg.destroyed, true);
18+
}
19+
20+
// OutgoingMessage.destroy(err) with no socket should set errored and emit 'close'.
21+
{
22+
const msg = new OutgoingMessage();
23+
const err = new Error('test destroy');
24+
msg.on('close', common.mustCall(() => {
25+
assert.strictEqual(msg.closed, true);
26+
assert.strictEqual(msg.errored, err);
27+
}));
28+
msg.destroy(err);
29+
assert.strictEqual(msg.errored, err);
30+
}
531

632
{
733
const server = http.createServer(common.mustCall((req, res) => {
Collapse file
+50Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
// Test that queued (pipelined) outgoing responses are destroyed when the
3+
// socket closes before the first response has finished. Previously,
4+
// socketOnClose only aborted state.incoming (pending requests) but left
5+
// state.outgoing responses with socket=null alive forever.
6+
7+
const common = require('../common');
8+
const http = require('http');
9+
const net = require('net');
10+
const assert = require('assert');
11+
12+
let requestCount = 0;
13+
14+
const server = http.createServer(common.mustCall((req, res) => {
15+
requestCount++;
16+
17+
if (requestCount === 1) {
18+
// Keep the first response open so the second response is queued in
19+
// state.outgoing with socket === null.
20+
res.writeHead(200);
21+
res.write('start');
22+
// Intentionally do not call res.end().
23+
} else {
24+
// The second response should be queued — no socket assigned yet.
25+
assert.strictEqual(res.socket, null);
26+
assert.strictEqual(res.destroyed, false);
27+
assert.strictEqual(res.closed, false);
28+
29+
res.on('close', common.mustCall(() => {
30+
assert.strictEqual(res.destroyed, true);
31+
assert.strictEqual(res.closed, true);
32+
server.close();
33+
}));
34+
35+
// Simulate client dying while first response is still in flight.
36+
req.socket.destroy();
37+
}
38+
}, 2));
39+
40+
server.listen(0, common.mustCall(function() {
41+
const port = this.address().port;
42+
const client = net.connect(port);
43+
44+
// Send two pipelined HTTP/1.1 requests at once.
45+
client.write(
46+
`GET /1 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n` +
47+
`GET /2 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n`,
48+
);
49+
client.resume();
50+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.