From 987338fe31b39b7f57a8f5645593425722ba40af Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 14 Feb 2013 15:37:12 -0800 Subject: http: Do not let Agent hand out destroyed sockets Fix #4373 --- lib/http.js | 3 +- test/simple/test-http-agent-destroyed-socket.js | 106 ++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 test/simple/test-http-agent-destroyed-socket.js diff --git a/lib/http.js b/lib/http.js index 6f3eff1ba..2446545ba 100644 --- a/lib/http.js +++ b/lib/http.js @@ -1131,7 +1131,8 @@ function Agent(options) { name += ':' + localAddress; } - if (self.requests[name] && self.requests[name].length) { + if (!socket.destroyed && + self.requests[name] && self.requests[name].length) { self.requests[name].shift().onSocket(socket); if (self.requests[name].length === 0) { // don't leak diff --git a/test/simple/test-http-agent-destroyed-socket.js b/test/simple/test-http-agent-destroyed-socket.js new file mode 100644 index 000000000..674ab5328 --- /dev/null +++ b/test/simple/test-http-agent-destroyed-socket.js @@ -0,0 +1,106 @@ +// 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 server = http.createServer(function(req, res) { + res.writeHead(200, {'Content-Type': 'text/plain'}); + res.end('Hello World\n'); +}).listen(common.PORT); + +var agent = new http.Agent({maxSockets: 1}); + +agent.on('free', function(socket, host, port) { + console.log('freeing socket. destroyed? ', socket.destroyed); +}); + +var requestOptions = { + agent: agent, + host: 'localhost', + port: common.PORT, + path: '/' +}; + +var request1 = http.get(requestOptions, function(response) { + // assert request2 is queued in the agent + var key = 'localhost:' + common.PORT; + assert(agent.requests[key].length === 1); + console.log('got response1'); + request1.socket.on('close', function() { + console.log('request1 socket closed'); + }); + response.pipe(process.stdout); + response.on('end', function() { + console.log('response1 done'); + ///////////////////////////////// + // + // THE IMPORTANT PART + // + // It is possible for the socket to get destroyed and other work + // to run before the 'close' event fires because it happens on + // nextTick. This example is contrived because it destroys the + // socket manually at just the right time, but at Voxer we have + // seen cases where the socket is destroyed by non-user code + // then handed out again by an agent *before* the 'close' event + // is triggered. + request1.socket.destroy(); + + process.nextTick(function() { + // assert request2 was removed from the queue + assert(!agent.requests[key]); + console.log("waiting for request2.onSocket's nextTick"); + process.nextTick(function() { + // assert that the same socket was not assigned to request2, + // since it was destroyed. + assert(request1.socket !== request2.socket); + assert(!request2.socket.destroyed, 'the socket is destroyed'); + }); + }); + }); +}); + +var request2 = http.get(requestOptions, function(response) { + assert(!request2.socket.destroyed); + assert(request1.socket.destroyed); + // assert not reusing the same socket, since it was destroyed. + assert(request1.socket !== request2.socket); + console.log('got response2'); + var gotClose = false; + var gotResponseEnd = false; + request2.socket.on('close', function() { + console.log('request2 socket closed'); + gotClose = true; + done(); + }); + response.pipe(process.stdout); + response.on('end', function() { + console.log('response2 done'); + gotResponseEnd = true; + done(); + }); + + function done() { + if (gotResponseEnd && gotClose) + server.close(); + } +}); -- cgit v1.2.1