Skip to content

Commit

Permalink
http: remove stale timeout listeners
Browse files Browse the repository at this point in the history
In order to prevent a memory leak when using keep alive, ensure that the
timeout listener for the request is removed when the response has ended.

PR-URL: #9440
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Karl Böhlmark authored and Fishrock123 committed Dec 13, 2016
1 parent 53e8e96 commit dded482
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,13 @@ function tickOnSocket(req, socket) {
socket.on('close', socketCloseListener);

if (req.timeout) {
socket.once('timeout', () => req.emit('timeout'));
const emitRequestTimeout = () => req.emit('timeout');
socket.once('timeout', emitRequestTimeout);
req.once('response', (res) => {
res.once('end', () => {
socket.removeListener('timeout', emitRequestTimeout);
});
});
}
req.emit('socket', socket);
}
Expand Down
44 changes: 44 additions & 0 deletions test/parallel/test-http-client-timeout-option-listeners.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';
const common = require('../common');
const http = require('http');
const assert = require('assert');

const agent = new http.Agent({ keepAlive: true });

const server = http.createServer((req, res) => {
res.end('');
});

const options = {
agent,
method: 'GET',
port: undefined,
host: common.localhostIPv4,
path: '/',
timeout: common.platformTimeout(100)
};

server.listen(0, options.host, common.mustCall(() => {
options.port = server.address().port;
doRequest(common.mustCall((numListeners) => {
assert.strictEqual(numListeners, 1);
doRequest(common.mustCall((numListeners) => {
assert.strictEqual(numListeners, 1);
server.close();
agent.destroy();
}));
}));
}));

function doRequest(cb) {
http.request(options, common.mustCall((response) => {
const sockets = agent.sockets[`${options.host}:${options.port}:`];
assert.strictEqual(sockets.length, 1);
const socket = sockets[0];
const numListeners = socket.listeners('timeout').length;
response.resume();
response.once('end', common.mustCall(() => {
process.nextTick(cb, numListeners);
}));
})).end();
}

0 comments on commit dded482

Please sign in to comment.