http: use Keep-Alive by default in global agents

PR-URL: #43522 Fixes: #37184 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Jun 29, 2022 · 4267b92 · 4267b92
1 parent 8e19dab
commit 4267b92
Show file tree

Hide file tree

Showing 27 changed files with 173 additions and 35 deletions.
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -1449,11 +1449,20 @@ type other than {net.Socket}.
 
 <!-- YAML
 added: v0.1.90
+changes:
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43522
+    description: The method closes idle connections before returning.
+
 -->
 
 * `callback` {Function}
 
-Stops the server from accepting new connections. See [`net.Server.close()`][].
+Stops the server from accepting new connections and closes all connections
+connected to this server which are not sending a request or waiting for
+a response.
+See [`net.Server.close()`][].
 
 ### `server.closeAllConnections()`
 
@@ -3214,6 +3223,11 @@ server.listen(8000);
 
 <!-- YAML
 added: v0.5.9
+changes:
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43522
+    description: The agent now uses HTTP Keep-Alive by default.
 -->
 
 * {http.Agent}

diff --git a/doc/api/https.md b/doc/api/https.md
@@ -309,6 +309,11 @@ https.get('https://encrypted.google.com/', (res) => {
 
 <!-- YAML
 added: v0.5.9
+changes:
+  - version:
+      - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43522
+    description: The agent now uses HTTP Keep-Alive by default.
 -->
 
 Global instance of [`https.Agent`][] for all HTTPS client requests.

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
@@ -31,10 +31,12 @@ const {
   ArrayPrototypeSplice,
   FunctionPrototypeCall,
   NumberIsNaN,
+  NumberParseInt,
   ObjectCreate,
   ObjectKeys,
   ObjectSetPrototypeOf,
   ObjectValues,
+  RegExpPrototypeExec,
   StringPrototypeIndexOf,
   StringPrototypeSplit,
   StringPrototypeStartsWith,
@@ -492,7 +494,24 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
   socket.setKeepAlive(true, this.keepAliveMsecs);
   socket.unref();
 
-  const agentTimeout = this.options.timeout || 0;
+  let agentTimeout = this.options.timeout || 0;
+
+  if (socket._httpMessage?.res) {
+    const keepAliveHint = socket._httpMessage.res.headers['keep-alive'];
+
+    if (keepAliveHint) {
+      const hint = RegExpPrototypeExec(/^timeout=(\d+)/, keepAliveHint)?.[1];
+
+      if (hint) {
+        const serverHintTimeout = NumberParseInt(hint) * 1000;
+
+        if (serverHintTimeout < agentTimeout) {
+          agentTimeout = serverHintTimeout;
+        }
+      }
+    }
+  }
+
   if (socket.timeout !== agentTimeout) {
     socket.setTimeout(agentTimeout);
   }
@@ -542,5 +561,5 @@ function asyncResetHandle(socket) {
 
 module.exports = {
   Agent,
-  globalAgent: new Agent()
+  globalAgent: new Agent({ keepAlive: true, scheduling: 'lifo', timeout: 5000 })
 };
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -478,6 +478,7 @@ ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
 
 Server.prototype.close = function() {
+  this.closeIdleConnections();
   clearInterval(this[kConnectionsCheckingInterval]);
   ReflectApply(net.Server.prototype.close, this, arguments);
 };

diff --git a/lib/https.js b/lib/https.js
@@ -331,7 +331,7 @@ Agent.prototype._evictSession = function _evictSession(key) {
   delete this._sessionCache.map[key];
 };
 
-const globalAgent = new Agent();
+const globalAgent = new Agent({ keepAlive: true, scheduling: 'lifo', timeout: 5000 });
 
 /**
  * Makes a request to a secure web server.

diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js
@@ -12,6 +12,7 @@ const hooks = initHooks();
 hooks.enable();
 
 const server = http.createServer(common.mustCall((req, res) => {
+  res.writeHead(200, { 'Connection': 'close' });
   res.end();
   server.close(common.mustCall());
 }));

diff --git a/test/parallel/test-http-agent-no-wait.js b/test/parallel/test-http-agent-no-wait.js
@@ -0,0 +1,24 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(function(req, res) {
+  res.writeHead(200);
+  res.end();
+});
+
+server.listen(0, common.mustCall(() => {
+  const req = http.get({ port: server.address().port }, (res) => {
+    assert.strictEqual(res.statusCode, 200);
+
+    res.resume();
+    server.close();
+  });
+
+  req.end();
+}));
+
+// This timer should never go off as the server will close the socket
+setTimeout(common.mustNotCall(), 1000).unref();
diff --git a/test/parallel/test-http-client-agent.js b/test/parallel/test-http-client-agent.js
@@ -27,6 +27,7 @@ const Countdown = require('../common/countdown');
 
 let name;
 const max = 3;
+const agent = new http.Agent();
 
 const server = http.Server(common.mustCall((req, res) => {
   if (req.url === '/0') {
@@ -40,27 +41,28 @@ const server = http.Server(common.mustCall((req, res) => {
   }
 }, max));
 server.listen(0, common.mustCall(() => {
-  name = http.globalAgent.getName({ port: server.address().port });
+  name = agent.getName({ port: server.address().port });
   for (let i = 0; i < max; ++i)
     request(i);
 }));
 
 const countdown = new Countdown(max, () => {
-  assert(!(name in http.globalAgent.sockets));
-  assert(!(name in http.globalAgent.requests));
+  assert(!(name in agent.sockets));
+  assert(!(name in agent.requests));
   server.close();
 });
 
 function request(i) {
   const req = http.get({
     port: server.address().port,
-    path: `/${i}`
+    path: `/${i}`,
+    agent
   }, function(res) {
     const socket = req.socket;
     socket.on('close', common.mustCall(() => {
       countdown.dec();
       if (countdown.remaining > 0) {
-        assert.strictEqual(http.globalAgent.sockets[name].includes(socket),
+        assert.strictEqual(agent.sockets[name].includes(socket),
                            false);
       }
     }));

diff --git a/test/parallel/test-http-client-close-with-default-agent.js b/test/parallel/test-http-client-close-with-default-agent.js
@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(function(req, res) {
+  res.writeHead(200);
+  res.end();
+});
+
+server.listen(0, common.mustCall(() => {
+  const req = http.get({ port: server.address().port }, (res) => {
+    assert.strictEqual(res.statusCode, 200);
+    res.resume();
+    server.close();
+  });
+
+  req.end();
+}));
+
+// This timer should never go off as the server will close the socket
+setTimeout(common.mustNotCall(), common.platformTimeout(10000)).unref();
diff --git a/test/parallel/test-http-client-headers-array.js b/test/parallel/test-http-client-headers-array.js
@@ -10,7 +10,7 @@ function execute(options) {
     const expectHeaders = {
       'x-foo': 'boom',
       'cookie': 'a=1; b=2; c=3',
-      'connection': 'close'
+      'connection': 'keep-alive'
     };
 
     // no Host header when you set headers an array
@@ -28,6 +28,7 @@ function execute(options) {
 
     assert.deepStrictEqual(req.headers, expectHeaders);
 
+    res.writeHead(200, { 'Connection': 'close' });
     res.end();
   }).listen(0, function() {
     options = Object.assign(options, {

diff --git a/test/parallel/test-http-client-keep-alive-hint.js b/test/parallel/test-http-client-keep-alive-hint.js
@@ -0,0 +1,27 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(
+  { keepAliveTimeout: common.platformTimeout(60000) },
+  function(req, res) {
+    req.resume();
+    res.writeHead(200, { 'Connection': 'keep-alive', 'Keep-Alive': 'timeout=1' });
+    res.end('FOO');
+  }
+);
+
+server.listen(0, common.mustCall(() => {
+  http.get({ port: server.address().port }, (res) => {
+    assert.strictEqual(res.statusCode, 200);
+
+    res.resume();
+    server.close();
+  });
+}));
+
+
+// This timer should never go off as the agent will parse the hint and terminate earlier
+setTimeout(common.mustNotCall(), common.platformTimeout(3000)).unref();
diff --git a/test/parallel/test-http-client-spurious-aborted.js b/test/parallel/test-http-client-spurious-aborted.js
@@ -10,7 +10,7 @@ const N = 2;
 let abortRequest = true;
 
 const server = http.Server(common.mustCall((req, res) => {
-  const headers = { 'Content-Type': 'text/plain' };
+  const headers = { 'Content-Type': 'text/plain', 'Connection': 'close' };
   headers['Content-Length'] = 50;
   const socket = res.socket;
   res.writeHead(200, headers);

diff --git a/test/parallel/test-http-client-timeout-on-connect.js b/test/parallel/test-http-client-timeout-on-connect.js
@@ -13,7 +13,10 @@ const server = http.createServer((req, res) => {
 
 server.listen(0, common.localhostIPv4, common.mustCall(() => {
   const port = server.address().port;
-  const req = http.get(`http://${common.localhostIPv4}:${port}`);
+  const req = http.get(
+    `http://${common.localhostIPv4}:${port}`,
+    { agent: new http.Agent() }
+  );
 
   req.setTimeout(1);
   req.on('socket', common.mustCall((socket) => {

diff --git a/test/parallel/test-http-content-length.js b/test/parallel/test-http-content-length.js
@@ -5,17 +5,17 @@ const http = require('http');
 const Countdown = require('../common/countdown');
 
 const expectedHeadersMultipleWrites = {
-  'connection': 'close',
+  'connection': 'keep-alive',
   'transfer-encoding': 'chunked',
 };
 
 const expectedHeadersEndWithData = {
-  'connection': 'close',
-  'content-length': String('hello world'.length)
+  'connection': 'keep-alive',
+  'content-length': String('hello world'.length),
 };
 
 const expectedHeadersEndNoData = {
-  'connection': 'close',
+  'connection': 'keep-alive',
   'content-length': '0',
 };
 
@@ -24,6 +24,7 @@ const countdown = new Countdown(3, () => server.close());
 
 const server = http.createServer(function(req, res) {
   res.removeHeader('Date');
+  res.setHeader('Keep-Alive', 'timeout=1');
 
   switch (req.url.substr(1)) {
     case 'multiple-writes':
@@ -59,7 +60,8 @@ server.listen(0, function() {
   req.write('hello ');
   req.end('world');
   req.on('response', function(res) {
-    assert.deepStrictEqual(res.headers, expectedHeadersMultipleWrites);
+    assert.deepStrictEqual(res.headers, { ...expectedHeadersMultipleWrites, 'keep-alive': 'timeout=1' });
+    res.resume();
   });
 
   req = http.request({
@@ -71,7 +73,8 @@ server.listen(0, function() {
   req.removeHeader('Host');
   req.end('hello world');
   req.on('response', function(res) {
-    assert.deepStrictEqual(res.headers, expectedHeadersEndWithData);
+    assert.deepStrictEqual(res.headers, { ...expectedHeadersEndWithData, 'keep-alive': 'timeout=1' });
+    res.resume();
   });
 
   req = http.request({
@@ -83,7 +86,8 @@ server.listen(0, function() {
   req.removeHeader('Host');
   req.end();
   req.on('response', function(res) {
-    assert.deepStrictEqual(res.headers, expectedHeadersEndNoData);
+    assert.deepStrictEqual(res.headers, { ...expectedHeadersEndNoData, 'keep-alive': 'timeout=1' });
+    res.resume();
   });
 
 });
diff --git a/test/parallel/test-http-default-encoding.js b/test/parallel/test-http-default-encoding.js
@@ -32,9 +32,9 @@ const server = http.Server((req, res) => {
   req.on('data', (chunk) => {
     result += chunk;
   }).on('end', () => {
-    server.close();
     res.writeHead(200);
     res.end('hello world\n');
+    server.close();
   });
 
 });

diff --git a/test/parallel/test-http-max-headers-count.js b/test/parallel/test-http-max-headers-count.js
@@ -48,7 +48,7 @@ const server = http.createServer(function(req, res) {
     expected = maxAndExpected[requests][1];
     server.maxHeadersCount = max;
   }
-  res.writeHead(200, headers);
+  res.writeHead(200, { ...headers, 'Connection': 'close' });
   res.end();
 });
 server.maxHeadersCount = max;

diff --git a/test/parallel/test-http-outgoing-message-capture-rejection.js b/test/parallel/test-http-outgoing-message-capture-rejection.js
@@ -16,9 +16,11 @@ events.captureRejections = true;
 
     res.socket.on('error', common.mustCall((err) => {
       assert.strictEqual(err, _err);
+      server.close();
     }));
 
     // Write until there is space in the buffer
+    res.writeHead(200, { 'Connection': 'close' });
     while (res.write('hello'));
   }));
 
@@ -37,7 +39,6 @@ events.captureRejections = true;
         code: 'ECONNRESET'
       }));
       res.resume();
-      server.close();
     }));
   }));
 }