Skip to content

Commit

Permalink
http: Only send connection:keep-alive if necessary
Browse files Browse the repository at this point in the history
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
  • Loading branch information
isaacs committed Sep 4, 2013
1 parent 689e5c9 commit 15a5a4a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
15 changes: 12 additions & 3 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions test/simple/test-http-raw-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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'
];
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions test/simple/test-http-should-keep-alive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down

0 comments on commit 15a5a4a

Please sign in to comment.