Skip to content

Commit

Permalink
http: refactor agent 'free' handler
Browse files Browse the repository at this point in the history
Remove nesting in favor of early returns.

PR-URL: #32801
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
ronag authored and addaleax committed Apr 15, 2020
1 parent e0a7fd7 commit 613d421
Showing 1 changed file with 49 additions and 48 deletions.
97 changes: 49 additions & 48 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const {
ERR_INVALID_ARG_TYPE,
},
} = require('internal/errors');
const { once } = require('internal/util');

const kOnKeylog = Symbol('onkeylog');
// New Agent code.

Expand Down Expand Up @@ -94,52 +96,55 @@ function Agent(options) {
// case of socket.destroy() below this 'error' has no handler
// and could cause unhandled exception.

if (socket.writable &&
this.requests[name] && this.requests[name].length) {
const req = this.requests[name].shift();
if (!socket.writable) {
socket.destroy();
return;
}

const requests = this.requests[name];
if (requests && requests.length) {
const req = requests.shift();
setRequestSocket(this, req, socket);
if (this.requests[name].length === 0) {
// don't leak
if (requests.length === 0) {
delete this.requests[name];
}
} else {
// If there are no pending requests, then put it in
// the freeSockets pool, but only if we're allowed to do so.
const req = socket._httpMessage;
if (req &&
req.shouldKeepAlive &&
socket.writable &&
this.keepAlive) {
let freeSockets = this.freeSockets[name];
const freeLen = freeSockets ? freeSockets.length : 0;
let count = freeLen;
if (this.sockets[name])
count += this.sockets[name].length;

if (count > this.maxSockets || freeLen >= this.maxFreeSockets) {
socket.destroy();
} else if (this.keepSocketAlive(socket)) {
freeSockets = freeSockets || [];
this.freeSockets[name] = freeSockets;
socket[async_id_symbol] = -1;
socket._httpMessage = null;
this.removeSocket(socket, options);

const agentTimeout = this.options.timeout || 0;
if (socket.timeout !== agentTimeout) {
socket.setTimeout(agentTimeout);
}

socket.once('error', freeSocketErrorListener);
freeSockets.push(socket);
} else {
// Implementation doesn't want to keep socket alive
socket.destroy();
}
} else {
socket.destroy();
}
return;
}

// If there are no pending requests, then put it in
// the freeSockets pool, but only if we're allowed to do so.
const req = socket._httpMessage;
if (!req || !req.shouldKeepAlive || !this.keepAlive) {
socket.destroy();
return;
}

let freeSockets = this.freeSockets[name];
const freeLen = freeSockets ? freeSockets.length : 0;
let count = freeLen;
if (this.sockets[name])
count += this.sockets[name].length;

if (count > this.maxSockets ||
freeLen >= this.maxFreeSockets ||
!this.keepSocketAlive(socket)) {
socket.destroy();
return;
}

freeSockets = freeSockets || [];
this.freeSockets[name] = freeSockets;
socket[async_id_symbol] = -1;
socket._httpMessage = null;
this.removeSocket(socket, options);

const agentTimeout = this.options.timeout || 0;
if (socket.timeout !== agentTimeout) {
socket.setTimeout(agentTimeout);
}

socket.once('error', freeSocketErrorListener);
freeSockets.push(socket);
});

// Don't emit keylog events unless there is a listener for them.
Expand Down Expand Up @@ -266,12 +271,8 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) {

debug('createConnection', name, options);
options.encoding = null;
let called = false;

const oncreate = (err, s) => {
if (called)
return;
called = true;
const oncreate = once((err, s) => {
if (err)
return cb(err);
if (!this.sockets[name]) {
Expand All @@ -281,7 +282,7 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) {
debug('sockets', name, this.sockets[name].length);
installListeners(this, s, options);
cb(null, s);
};
});

const newSocket = this.createConnection(options, oncreate);
if (newSocket)
Expand Down

0 comments on commit 613d421

Please sign in to comment.