Skip to content

Commit

Permalink
Fix memory leak when using socket timeout and keepalive agent (#694)
Browse files Browse the repository at this point in the history
Fixes #690
  • Loading branch information
szmarczak authored and sindresorhus committed Jan 13, 2019
1 parent 73428f9 commit 203dadc
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
11 changes: 8 additions & 3 deletions source/utils/timed-out.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,16 @@ module.exports = (request, delays, options) => {
}

if (delays.socket !== undefined) {
request.setTimeout(delays.socket, () => {
const socketTimeoutHandler = () => {
timeoutHandler(delays.socket, 'socket');
});
};

request.setTimeout(delays.socket, socketTimeoutHandler);

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

if (delays.lookup !== undefined && !request.socketPath && !net.isIP(hostname || host)) {
Expand Down
18 changes: 18 additions & 0 deletions test/timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,3 +473,21 @@ test('socket timeout is canceled on error', async t => {
// Wait a bit more to check if there are any unhandled errors
await delay(10);
});

test('no memory leak when using socket timeout and keepalive agent', async t => {
const promise = got(s.url, {
agent: keepAliveAgent,
timeout: {socket: requestDelay * 2}
});

let socket;
promise.on('request', request => {
request.on('socket', () => {
socket = request.socket;
});
});

await promise;

t.is(socket.listenerCount('timeout'), 0);
});

0 comments on commit 203dadc

Please sign in to comment.