async_hooks,http: fix socket reuse with Agent
Under very specific circumstances the `http` implementation could be brought to crash, because the Agent did not re-assign the async id field properly after setting up a socket for reuse. Fixes: https://github.com/nodejs/node/issues/13325 PR-URL: https://github.com/nodejs/node/pull/13348 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
This commit is contained in:
parent
9b9a25c5eb
commit
3e02636bcb
@ -168,6 +168,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/,
|
|||||||
var socket = this.freeSockets[name].shift();
|
var socket = this.freeSockets[name].shift();
|
||||||
// Assign the handle a new asyncId and run any init() hooks.
|
// Assign the handle a new asyncId and run any init() hooks.
|
||||||
socket._handle.asyncReset();
|
socket._handle.asyncReset();
|
||||||
|
socket[async_id_symbol] = socket._handle.getAsyncId();
|
||||||
debug('have free socket');
|
debug('have free socket');
|
||||||
|
|
||||||
// don't leak
|
// don't leak
|
||||||
|
79
test/parallel/test-async-hooks-http-agent.js
Normal file
79
test/parallel/test-async-hooks-http-agent.js
Normal file
@ -0,0 +1,79 @@
|
|||||||
|
'use strict';
|
||||||
|
const common = require('../common');
|
||||||
|
const assert = require('assert');
|
||||||
|
const async_id_symbol = process.binding('async_wrap').async_id_symbol;
|
||||||
|
const http = require('http');
|
||||||
|
|
||||||
|
// Regression test for https://github.com/nodejs/node/issues/13325
|
||||||
|
// Checks that an http.Agent properly asyncReset()s a reused socket handle, and
|
||||||
|
// re-assigns the fresh async id to the reused `net.Socket` instance.
|
||||||
|
|
||||||
|
// Make sure a single socket is transpartently reused for 2 requests.
|
||||||
|
const agent = new http.Agent({
|
||||||
|
keepAlive: true,
|
||||||
|
keepAliveMsecs: Infinity,
|
||||||
|
maxSockets: 1
|
||||||
|
});
|
||||||
|
|
||||||
|
const server = http.createServer(common.mustCall((req, res) => {
|
||||||
|
req.once('data', common.mustCallAtLeast(() => {
|
||||||
|
res.writeHead(200, {'Content-Type': 'text/plain'});
|
||||||
|
res.write('foo');
|
||||||
|
}));
|
||||||
|
req.on('end', common.mustCall(() => {
|
||||||
|
res.end('bar');
|
||||||
|
}));
|
||||||
|
}, 2)).listen(0, common.mustCall(() => {
|
||||||
|
const port = server.address().port;
|
||||||
|
const payload = 'hello world';
|
||||||
|
|
||||||
|
// First request. This is useless except for adding a socket to the
|
||||||
|
// agent’s pool for reuse.
|
||||||
|
const r1 = http.request({
|
||||||
|
agent, port, method: 'POST'
|
||||||
|
}, common.mustCall((res) => {
|
||||||
|
// Remember which socket we used.
|
||||||
|
const socket = res.socket;
|
||||||
|
const asyncIdAtFirstRequest = socket[async_id_symbol];
|
||||||
|
assert.ok(asyncIdAtFirstRequest > 0, `${asyncIdAtFirstRequest} > 0`);
|
||||||
|
// Check that request and response share their socket.
|
||||||
|
assert.strictEqual(r1.socket, socket);
|
||||||
|
|
||||||
|
res.on('data', common.mustCallAtLeast(() => {}));
|
||||||
|
res.on('end', common.mustCall(() => {
|
||||||
|
// setImmediate() to give the agent time to register the freed socket.
|
||||||
|
setImmediate(common.mustCall(() => {
|
||||||
|
// The socket is free for reuse now.
|
||||||
|
assert.strictEqual(socket[async_id_symbol], -1);
|
||||||
|
|
||||||
|
// Second request. To re-create the exact conditions from the
|
||||||
|
// referenced issue, we use a POST request without chunked encoding
|
||||||
|
// (hence the Content-Length header) and call .end() after the
|
||||||
|
// response header has already been received.
|
||||||
|
const r2 = http.request({
|
||||||
|
agent, port, method: 'POST', headers: {
|
||||||
|
'Content-Length': payload.length
|
||||||
|
}
|
||||||
|
}, common.mustCall((res) => {
|
||||||
|
const asyncId = res.socket[async_id_symbol];
|
||||||
|
assert.ok(asyncId > 0, `${asyncId} > 0`);
|
||||||
|
assert.strictEqual(r2.socket, socket);
|
||||||
|
// Empty payload, to hit the “right” code path.
|
||||||
|
r2.end('');
|
||||||
|
|
||||||
|
res.on('data', common.mustCallAtLeast(() => {}));
|
||||||
|
res.on('end', common.mustCall(() => {
|
||||||
|
// Clean up to let the event loop stop.
|
||||||
|
server.close();
|
||||||
|
agent.destroy();
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
|
||||||
|
// Schedule a payload to be written immediately, but do not end the
|
||||||
|
// request just yet.
|
||||||
|
r2.write(payload);
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
r1.end(payload);
|
||||||
|
}));
|
Loading…
x
Reference in New Issue
Block a user