From 15a5a4a9453a056b783c7ff0966e31c3c6b7d7e1 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 17 Aug 2013 18:50:59 -0700 Subject: [PATCH] http: Only send connection:keep-alive if necessary In cases where the Agent has maxSockets=Infinity, and keepAlive=false, there's no case where we won't immediately close the connection after the response is completed. Since we're going to close it anyway, send a `connection:close` header rather than a `connection:keep-alive` header. Still send the `connection:keep-alive` if the agent will actually reuse the socket, however. Closes #5838 --- lib/_http_client.js | 15 ++++++++++++--- test/simple/test-http-raw-headers.js | 10 +++++----- test/simple/test-http-should-keep-alive.js | 1 + 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index 63f5ee9565..a1dc3bef0a 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -107,9 +107,18 @@ function ClientRequest(options, cb) { var conn = self.agent.createConnection({ path: self.socketPath }); self.onSocket(conn); } else if (self.agent) { - // If there is an agent we should default to Connection:keep-alive. - self._last = false; - self.shouldKeepAlive = true; + // If there is an agent we should default to Connection:keep-alive, + // but only if the Agent will actually reuse the connection! + // If it's not a keepAlive agent, and the maxSockets==Infinity, then + // there's never a case where this socket will actually be reused + var agent = self.agent; + if (!agent.keepAlive && !Number.isFinite(agent.maxSockets)) { + self._last = true; + self.shouldKeepAlive = false; + } else { + self._last = false; + self.shouldKeepAlive = true; + } self.agent.addRequest(self, options); } else { // No agent, default to Connection:close. diff --git a/test/simple/test-http-raw-headers.js b/test/simple/test-http-raw-headers.js index a65c106489..0d31d8f613 100644 --- a/test/simple/test-http-raw-headers.js +++ b/test/simple/test-http-raw-headers.js @@ -34,13 +34,13 @@ http.createServer(function(req, res) { 'x-BaR', 'yoyoyo', 'Connection', - 'keep-alive' + 'close' ]; var expectHeaders = { host: 'localhost:' + common.PORT, 'transfer-encoding': 'CHUNKED', 'x-bar': 'yoyoyo', - connection: 'keep-alive' + connection: 'close' }; var expectRawTrailers = [ @@ -77,7 +77,7 @@ http.createServer(function(req, res) { 'Date', 'Tue, 06 Aug 2013 01:31:54 GMT', 'Connection', - 'keep-alive', + 'close', 'Transfer-Encoding', 'chunked' ]; @@ -96,13 +96,13 @@ http.createServer(function(req, res) { 'Date', null, 'Connection', - 'keep-alive', + 'close', 'Transfer-Encoding', 'chunked' ]; var expectHeaders = { date: null, - connection: 'keep-alive', + connection: 'close', 'transfer-encoding': 'chunked' }; res.rawHeaders[1] = null; diff --git a/test/simple/test-http-should-keep-alive.js b/test/simple/test-http-should-keep-alive.js index e2303be7bf..943c34bb43 100644 --- a/test/simple/test-http-should-keep-alive.js +++ b/test/simple/test-http-should-keep-alive.js @@ -42,6 +42,7 @@ var SHOULD_KEEP_ALIVE = [ ]; var requests = 0; var responses = 0; +http.globalAgent.maxSockets = 5; var server = net.createServer(function(socket) { socket.write(SERVER_RESPONSES[requests]);