From 1ce5db4d95ef8d522592db33a131780754935fc3 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Wed, 6 Nov 2013 19:58:03 +0800 Subject: [PATCH] http: cleanup freeSockets when socket destroyed If the socket was destroyed, we need to remove it from the agent's `freeSockets` list, otherwise dead socket could be reused by new request. --- lib/_http_agent.js | 25 ++-- test/simple/test-http-agent-keepalive.js | 144 +++++++++++++++++++++++ 2 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 test/simple/test-http-agent-keepalive.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 2c578295a393ff..a6987dcf2e3f63 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -229,17 +229,24 @@ Agent.prototype.createSocket = function(req, options) { Agent.prototype.removeSocket = function(s, options) { var name = this.getName(options); - debug('removeSocket', name); - if (this.sockets[name]) { - var index = this.sockets[name].indexOf(s); - if (index !== -1) { - this.sockets[name].splice(index, 1); - if (this.sockets[name].length === 0) { - // don't leak - delete this.sockets[name]; + debug('removeSocket', name, 'destroyed:', s.destroyed); + var sets = [this.sockets]; + + // If the socket was destroyed, remove it from the free buffers too. + if (s.destroyed) + sets.push(this.freeSockets); + + sets.forEach(function(sockets) { + if (sockets[name]) { + var index = sockets[name].indexOf(s); + if (index !== -1) { + sockets[name].splice(index, 1); + // Don't leak + if (sockets[name].length === 0) + delete sockets[name]; } } - } + }); if (this.requests[name] && this.requests[name].length) { debug('removeSocket, have a request, make a socket'); var req = this.requests[name][0]; diff --git a/test/simple/test-http-agent-keepalive.js b/test/simple/test-http-agent-keepalive.js new file mode 100644 index 00000000000000..79722a0b13deb5 --- /dev/null +++ b/test/simple/test-http-agent-keepalive.js @@ -0,0 +1,144 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var http = require('http'); +var Agent = require('_http_agent').Agent; +var EventEmitter = require('events').EventEmitter; + +var agent = new Agent({ + keepAlive: true, + keepAliveMsecs: 1000, + maxSockets: 5, + maxFreeSockets: 5, +}); + +var server = http.createServer(function (req, res) { + if (req.url === '/error') { + res.destroy(); + return; + } else if (req.url === '/remote_close') { + // cache the socket, close it after 100ms + var socket = res.connection; + setTimeout(function () { + socket.end(); + }, 100); + } + res.end('hello world'); +}); + +function get(path, callback) { + return agent.get({ + host: 'localhost', + port: common.PORT, + path: path + }, callback); +} + +var name = 'localhost:' + common.PORT + '::'; + +function checkDataAndSockets(body) { + assert.equal(body.toString(), 'hello world'); + assert.equal(agent.sockets[name].length, 1); + assert.equal(agent.freeSockets[name], undefined); +} + +function second() { + // request second, use the same socket + get('/second', function (res) { + assert.equal(res.statusCode, 200); + res.on('data', checkDataAndSockets); + res.on('end', function () { + assert.equal(agent.sockets[name].length, 1); + assert.equal(agent.freeSockets[name], undefined); + process.nextTick(function () { + assert.equal(agent.sockets[name], undefined); + assert.equal(agent.freeSockets[name].length, 1); + remoteClose(); + }); + }); + }); +} + +function remoteClose() { + // mock remote server close the socket + get('/remote_close', function (res) { + assert.deepEqual(res.statusCode, 200); + res.on('data', checkDataAndSockets); + res.on('end', function () { + assert.equal(agent.sockets[name].length, 1); + assert.equal(agent.freeSockets[name], undefined); + process.nextTick(function () { + assert.equal(agent.sockets[name], undefined); + assert.equal(agent.freeSockets[name].length, 1); + // waitting remote server close the socket + setTimeout(function () { + assert.equal(agent.sockets[name], undefined); + assert.equal(agent.freeSockets[name], undefined, + 'freeSockets is not empty'); + remoteError(); + }, 200); + }); + }); + }); +} + +function remoteError() { + // remove server will destroy ths socket + var req = get('/error', function (res) { + throw new Error('should not call this function'); + }); + req.on('error', function (err) { + assert.ok(err); + assert.equal(err.message, 'socket hang up'); + assert.equal(agent.sockets[name].length, 1); + assert.equal(agent.freeSockets[name], undefined); + // Wait socket 'close' event emit + setTimeout(function () { + assert.equal(agent.sockets[name], undefined); + assert.equal(agent.freeSockets[name], undefined); + done(); + }, 1); + }); +} + +function done() { + console.log('http keepalive agent test success.'); + process.exit(0); +} + +server.listen(common.PORT, function() { + // request first, and keep alive + get('/first', function (res) { + assert.equal(res.statusCode, 200); + res.on('data', checkDataAndSockets); + res.on('end', function () { + assert.equal(agent.sockets[name].length, 1); + assert.equal(agent.freeSockets[name], undefined); + process.nextTick(function () { + assert.equal(agent.sockets[name], undefined); + assert.equal(agent.freeSockets[name].length, 1); + second(); + }); + }); + }); +});