Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Keep-Alive by default in global agents and close idle connections in server #43522

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion doc/api/http.md
Expand Up @@ -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()`

Expand Down Expand Up @@ -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}
Expand Down
5 changes: 5 additions & 0 deletions doc/api/https.md
Expand Up @@ -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.
Expand Down
23 changes: 21 additions & 2 deletions lib/_http_agent.js
Expand Up @@ -31,10 +31,12 @@ const {
ArrayPrototypeSplice,
FunctionPrototypeCall,
NumberIsNaN,
NumberParseInt,
ObjectCreate,
ObjectKeys,
ObjectSetPrototypeOf,
ObjectValues,
RegExpPrototypeExec,
StringPrototypeIndexOf,
StringPrototypeSplit,
StringPrototypeStartsWith,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -542,5 +561,5 @@ function asyncResetHandle(socket) {

module.exports = {
Agent,
globalAgent: new Agent()
globalAgent: new Agent({ keepAlive: true, scheduling: 'lifo', timeout: 5000 })
};
1 change: 1 addition & 0 deletions lib/_http_server.js
Expand Up @@ -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);
};
Expand Down
2 changes: 1 addition & 1 deletion lib/https.js
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions test/async-hooks/test-graph.http.js
Expand Up @@ -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());
}));
Expand Down
24 changes: 24 additions & 0 deletions 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();
12 changes: 7 additions & 5 deletions test/parallel/test-http-client-agent.js
Expand Up @@ -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') {
Expand All @@ -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);
}
}));
Expand Down
23 changes: 23 additions & 0 deletions 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();
3 changes: 2 additions & 1 deletion test/parallel/test-http-client-headers-array.js
Expand Up @@ -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
Expand All @@ -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, {
Expand Down
27 changes: 27 additions & 0 deletions 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();
2 changes: 1 addition & 1 deletion test/parallel/test-http-client-spurious-aborted.js
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-http-client-timeout-on-connect.js
Expand Up @@ -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) => {
Expand Down
18 changes: 11 additions & 7 deletions test/parallel/test-http-content-length.js
Expand Up @@ -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',
};

Expand All @@ -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':
Expand Down Expand Up @@ -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({
Expand All @@ -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({
Expand All @@ -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();
});

});
2 changes: 1 addition & 1 deletion test/parallel/test-http-default-encoding.js
Expand Up @@ -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();
});

});
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-max-headers-count.js
Expand Up @@ -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;
Expand Down
Expand Up @@ -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'));
}));

Expand All @@ -37,7 +39,6 @@ events.captureRejections = true;
code: 'ECONNRESET'
}));
res.resume();
server.close();
}));
}));
}
Expand Down