Skip to content

Commit 70e6a88

Browse files
committed
http: add missing async_hooks destroy before asyncReset
Emit a destroy before calling asyncReset because otherwise the old async_id never gets destroyed. Refs: nodejs#19859
1 parent ff3fd3b commit 70e6a88

File tree

2 files changed

+93
-1
lines changed

2 files changed

+93
-1
lines changed

lib/_http_agent.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ const net = require('net');
2525
const util = require('util');
2626
const EventEmitter = require('events');
2727
const debug = util.debuglog('http');
28-
const { async_id_symbol } = require('internal/async_hooks').symbols;
28+
const {
29+
destroyHooksExist,
30+
emitDestroy,
31+
symbols
32+
} = require('internal/async_hooks');
33+
const async_id_symbol = symbols.async_id_symbol;
2934

3035
// New Agent code.
3136

@@ -167,6 +172,9 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
167172
var socket = this.freeSockets[name].shift();
168173
// Guard against an uninitialized or user supplied Socket.
169174
if (socket._handle && typeof socket._handle.asyncReset === 'function') {
175+
if (destroyHooksExist() && socket._handle.getAsyncId()) {
176+
emitDestroy(socket._handle.getAsyncId());
177+
}
170178
// Assign the handle a new asyncId and run any init() hooks.
171179
socket._handle.asyncReset();
172180
socket[async_id_symbol] = socket._handle.getAsyncId();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
'use strict';
2+
// Flags: --expose-internals
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { async_id_symbol } = require('internal/async_hooks').symbols;
6+
const async_hooks = require('async_hooks');
7+
const http = require('http');
8+
9+
// Regression test for https://github.com/nodejs/node/issues/19859
10+
// Checks that an http.Agent emits a destroy for the old asyncId before calling
11+
// asyncReset()s when reusing a socket handle. The setup is nearly identical to
12+
// parallel/test-async-hooks-http-agent (which focuses on the assertion that
13+
// a fresh asyncId is assigned to the net.Socket instance).
14+
15+
const destroyedIds = new Set();
16+
async_hooks.createHook({
17+
destroy: common.mustCallAtLeast((asyncId) => {
18+
destroyedIds.add(asyncId);
19+
}, 1)
20+
}).enable();
21+
22+
// Make sure a single socket is transparently reused for 2 requests.
23+
const agent = new http.Agent({
24+
keepAlive: true,
25+
keepAliveMsecs: Infinity,
26+
maxSockets: 1
27+
});
28+
29+
const server = http.createServer(common.mustCall((req, res) => {
30+
req.once('data', common.mustCallAtLeast(() => {
31+
res.writeHead(200, { 'Content-Type': 'text/plain' });
32+
res.write('foo');
33+
}));
34+
req.on('end', common.mustCall(() => {
35+
res.end('bar');
36+
}));
37+
}, 2)).listen(0, common.mustCall(() => {
38+
const port = server.address().port;
39+
const payload = 'hello world';
40+
41+
// First request. This is useless except for adding a socket to the
42+
// agent’s pool for reuse.
43+
const r1 = http.request({
44+
agent, port, method: 'POST'
45+
}, common.mustCall((res) => {
46+
// Remember which socket we used.
47+
const socket = res.socket;
48+
const asyncIdAtFirstRequest = socket[async_id_symbol];
49+
assert.ok(asyncIdAtFirstRequest > 0, `${asyncIdAtFirstRequest} > 0`);
50+
// Check that request and response share their socket.
51+
assert.strictEqual(r1.socket, socket);
52+
53+
res.on('data', common.mustCallAtLeast(() => {}));
54+
res.on('end', common.mustCall(() => {
55+
// setImmediate() to give the agent time to register the freed socket.
56+
setImmediate(common.mustCall(() => {
57+
// The socket is free for reuse now.
58+
assert.strictEqual(socket[async_id_symbol], -1);
59+
60+
// second request:
61+
const r2 = http.request({
62+
agent, port, method: 'POST'
63+
}, common.mustCall((res) => {
64+
assert.ok(destroyedIds.has(asyncIdAtFirstRequest));
65+
66+
// Empty payload, to hit the “right” code path.
67+
r2.end('');
68+
69+
res.on('data', common.mustCallAtLeast(() => {}));
70+
res.on('end', common.mustCall(() => {
71+
// Clean up to let the event loop stop.
72+
server.close();
73+
agent.destroy();
74+
}));
75+
}));
76+
77+
// Schedule a payload to be written immediately, but do not end the
78+
// request just yet.
79+
r2.write(payload);
80+
}));
81+
}));
82+
}));
83+
r1.end(payload);
84+
}));

0 commit comments

Comments
 (0)