GeezerVison is showing github.com — http: cleanup pipeline queue · nodejs/node@9f3bc70
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/_http_outgoing.js +8-3Lines changed: 8 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -328,14 +328,19 @@ OutgoingMessage.prototype.destroy = function destroy(error) { | 328 | 328 | if (this[kSocket]) { | 329 | 329 | this[kSocket].destroy(error); | 330 | 330 | } else { | 331 | | - this.once('socket', function socketDestroyOnConnect(socket) { | 332 | | - socket.destroy(error); | 333 | | - }); | | 331 | + process.nextTick(emitDestroyNT, this); | 334 | 332 | } | 335 | 333 |
| 336 | 334 | return this; | 337 | 335 | }; | 338 | 336 |
| | 337 | +function emitDestroyNT(self) { | | 338 | + if (!self._closed) { | | 339 | + self._closed = true; | | 340 | + self.emit('close'); | | 341 | + } | | 342 | +} | | 343 | + | 339 | 344 |
| 340 | 345 | // This abstract either writing directly to the socket or buffering it. | 341 | 346 | OutgoingMessage.prototype._send = function _send(data, encoding, callback, byteLength) { | |
Collapse file Copy file name to clipboardExpand all lines: lib/_http_server.js +8-1Lines changed: 8 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -840,14 +840,21 @@ function socketOnClose(socket, state) { | 840 | 840 | debug('server socket close'); | 841 | 841 | freeParser(socket.parser, null, socket); | 842 | 842 | abortIncoming(state.incoming); | | 843 | + abortOutgoing(state.outgoing); | 843 | 844 | } | 844 | 845 |
| 845 | 846 | function abortIncoming(incoming) { | 846 | 847 | while (incoming.length) { | 847 | 848 | const req = incoming.shift(); | 848 | 849 | req.destroy(new ConnResetException('aborted')); | 849 | 850 | } | 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 | + } | 851 | 858 | } | 852 | 859 |
| 853 | 860 | function socketOnEnd(server, socket, parser, state) { | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-http-outgoing-destroyed.js +26Lines changed: 26 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 2 | 2 | const common = require('../common'); | 3 | 3 | const http = require('http'); | 4 | 4 | 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 | +} | 5 | 31 |
| 6 | 32 | { | 7 | 33 | const server = http.createServer(common.mustCall((req, res) => { | |
Collapse file Copy file name to clipboard +50Lines changed: 50 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 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 | +})); |
|