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 9f3bc70

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 aa1d8a9 commit 9f3bc70
Copy full SHA for 9f3bc70

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
@@ -840,14 +840,21 @@ function socketOnClose(socket, state) {
840840
debug('server socket close');
841841
freeParser(socket.parser, null, socket);
842842
abortIncoming(state.incoming);
843+
abortOutgoing(state.outgoing);
843844
}
844845

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

853860
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.