From 1effe5e7d5e481bc95fc4fc848d4ae850e3547cd Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Mon, 28 Nov 2016 13:38:50 -0800 Subject: [PATCH] http: remove stale timeout listeners --- lib/_http_client.js | 23 ++++------ ...st-http-client-timeout-option-listeners.js | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 15 deletions(-) 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 6837c94df98eea..bd64763f48bc65 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -562,7 +562,7 @@ function tickOnSocket(req, socket) { socket.on('close', socketCloseListener); if (req.timeout) { - socket.once('timeout', () => req.emit('timeout')); + req.setTimeout(req.timeout); } req.emit('socket', socket); } @@ -620,28 +620,21 @@ ClientRequest.prototype.setTimeout = function setTimeout(msecs, callback) { self.emit('timeout'); } - if (this.socket && this.socket.writable) { - if (this.timeoutCb) - this.socket.setTimeout(0, this.timeoutCb); - this.timeoutCb = emitTimeout; - this.socket.setTimeout(msecs, emitTimeout); - return this; - } + // Remove any existing timeout listener + if (this.timeoutCb) + this.socket.setTimeout(0, this.timeoutCb); // Set timeoutCb so that it'll get cleaned up on request end this.timeoutCb = emitTimeout; + if (this.socket) { - var sock = this.socket; - this.socket.once('connect', function() { + this.socket.setTimeout(msecs, emitTimeout); + } else { + this.once('socket', function(sock) { sock.setTimeout(msecs, emitTimeout); }); - return this; } - this.once('socket', function(sock) { - sock.setTimeout(msecs, emitTimeout); - }); - return this; }; 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(); +}