From b5f747187dfac9ac201af4ade6743dcc7f5dded5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20B=C3=B6hlmark?= Date: Tue, 1 Nov 2016 16:59:31 +0100 Subject: [PATCH] http: remove stale timeout listeners 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: https://github.com/nodejs/node/pull/9440 Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig --- lib/_http_client.js | 8 +++- ...st-http-client-timeout-option-listeners.js | 44 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-client-timeout-option-listeners.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 423445d6a6f091..4ca03eae6796ac 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -563,7 +563,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); } diff --git a/test/parallel/test-http-client-timeout-option-listeners.js b/test/parallel/test-http-client-timeout-option-listeners.js new file mode 100644 index 00000000000000..3ac6cd4616b496 --- /dev/null +++ b/test/parallel/test-http-client-timeout-option-listeners.js @@ -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(); +}