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

http: overridable keep-alive behavior of Agent #13005

Closed
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
36 changes: 36 additions & 0 deletions doc/api/http.md
Expand Up @@ -157,6 +157,42 @@ socket/stream from this function, or by passing the socket/stream to `callback`.

`callback` has a signature of `(err, stream)`.

### agent.keepSocketAlive(socket)
<!-- YAML
added: REPLACEME
-->

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should list the arguments... e.g.

* `socket` {net.Socket|tls.TLSSocket} description...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

* `socket` {net.Socket}

Called when `socket` is detached from a request and could be persisted by the
Agent. Default behavior is to:

```js
socket.unref();
socket.setKeepAlive(agent.keepAliveMsecs);
```

This method can be overridden by a particular `Agent` subclass. If this
method returns a falsy value, the socket will be destroyed instead of persisting
it for use with the next request.

### agent.reuseSocket(socket, request)
<!-- YAML
added: REPLACEME
-->

* `socket` {net.Socket}
* `request` {http.ClientRequest}

Called when `socket` is attached to `request` after being persisted because of
the keep-alive options. Default behavior is to:

```js
socket.ref();
```

This method can be overridden by a particular `Agent` subclass.

### agent.destroy()
<!-- YAML
added: v0.11.4
Expand Down
22 changes: 17 additions & 5 deletions lib/_http_agent.js
Expand Up @@ -90,15 +90,16 @@ function Agent(options) {

if (count > self.maxSockets || freeLen >= self.maxFreeSockets) {
socket.destroy();
} else {
} else if (self.keepSocketAlive(socket)) {
freeSockets = freeSockets || [];
self.freeSockets[name] = freeSockets;
socket.setKeepAlive(true, self.keepAliveMsecs);
socket.unref();
socket[async_id_symbol] = -1;
socket._httpMessage = null;
self.removeSocket(socket, options);
freeSockets.push(socket);
} else {
// Implementation doesn't want to keep socket alive
socket.destroy();
}
} else {
socket.destroy();
Expand Down Expand Up @@ -169,13 +170,12 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/,
// Assign the handle a new asyncId and run any init() hooks.
socket._handle.asyncReset();
socket[async_id_symbol] = socket._handle.getAsyncId();
debug('have free socket');

// don't leak
if (!this.freeSockets[name].length)
delete this.freeSockets[name];

socket.ref();
this.reuseSocket(socket, req);
req.onSocket(socket);
this.sockets[name].push(socket);
} else if (sockLen < this.maxSockets) {
Expand Down Expand Up @@ -306,6 +306,18 @@ Agent.prototype.removeSocket = function removeSocket(s, options) {
}
};

Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
socket.setKeepAlive(true, this.keepAliveMsecs);
socket.unref();

return true;
};

Agent.prototype.reuseSocket = function reuseSocket(socket, req) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is req passed in here? It appears to be unused. Is it for agents that override the function and want to use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is correct.

debug('have free socket');
socket.ref();
};

Agent.prototype.destroy = function destroy() {
var sets = [this.freeSockets, this.sockets];
for (var s = 0; s < sets.length; s++) {
Expand Down
67 changes: 67 additions & 0 deletions test/parallel/test-http-keepalive-override.js
@@ -0,0 +1,67 @@
'use strict';
const common = require('../common');
const assert = require('assert');

const http = require('http');

const server = http.createServer((req, res) => {
res.end('ok');
}).listen(0, common.mustCall(() => {
const agent = http.Agent({
keepAlive: true,
maxSockets: 5,
maxFreeSockets: 2
});

const keepSocketAlive = agent.keepSocketAlive;
const reuseSocket = agent.reuseSocket;

let called = 0;
let expectedSocket;
agent.keepSocketAlive = common.mustCall((socket) => {
assert(socket);

called++;
if (called === 1) {
return false;
} else if (called === 2) {
expectedSocket = socket;
return keepSocketAlive.call(agent, socket);
}

assert.strictEqual(socket, expectedSocket);
return false;
}, 3);

agent.reuseSocket = common.mustCall((socket, req) => {
assert.strictEqual(socket, expectedSocket);
assert(req);

return reuseSocket.call(agent, socket, req);
}, 1);

function req(callback) {
http.request({
method: 'GET',
path: '/',
agent,
port: server.address().port
}, common.mustCall((res) => {
res.resume();
res.once('end', common.mustCall(() => {
setImmediate(callback);
}));
})).end();
}

// Should destroy socket instead of keeping it alive
req(common.mustCall(() => {
// Should keep socket alive
req(common.mustCall(() => {
// Should reuse the socket
req(common.mustCall(() => {
server.close();
}));
}));
}));
}));