Skip to content

Commit 203dadc

Browse files
szmarczaksindresorhus
authored andcommittedJan 13, 2019
Fix memory leak when using socket timeout and keepalive agent (#694)
Fixes #690
1 parent 73428f9 commit 203dadc

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed
 

‎source/utils/timed-out.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,16 @@ module.exports = (request, delays, options) => {
8282
}
8383

8484
if (delays.socket !== undefined) {
85-
request.setTimeout(delays.socket, () => {
85+
const socketTimeoutHandler = () => {
8686
timeoutHandler(delays.socket, 'socket');
87-
});
87+
};
88+
89+
request.setTimeout(delays.socket, socketTimeoutHandler);
8890

89-
cancelers.push(() => request.setTimeout(0));
91+
// `request.setTimeout(0)` causes a memory leak.
92+
// We can just remove the listener and forget about the timer - it's unreffed.
93+
// See https://github.com/sindresorhus/got/issues/690
94+
cancelers.push(() => request.removeListener('timeout', socketTimeoutHandler));
9095
}
9196

9297
if (delays.lookup !== undefined && !request.socketPath && !net.isIP(hostname || host)) {

‎test/timeout.js

+18
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,21 @@ test('socket timeout is canceled on error', async t => {
473473
// Wait a bit more to check if there are any unhandled errors
474474
await delay(10);
475475
});
476+
477+
test('no memory leak when using socket timeout and keepalive agent', async t => {
478+
const promise = got(s.url, {
479+
agent: keepAliveAgent,
480+
timeout: {socket: requestDelay * 2}
481+
});
482+
483+
let socket;
484+
promise.on('request', request => {
485+
request.on('socket', () => {
486+
socket = request.socket;
487+
});
488+
});
489+
490+
await promise;
491+
492+
t.is(socket.listenerCount('timeout'), 0);
493+
});

0 commit comments

Comments
 (0)
Please sign in to comment.