Skip to content

net: race family autoselection attempts #66229

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.

Sign up for GitHub

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

Open
mcollina wants to merge 1 commit into
base: main
Choose a base branch
from
Open

net: race family autoselection attempts #66229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 24 additions & 16 deletions doc/api/net.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,9 +1143,12 @@ added:
* `port` {number} The port which the socket attempted to connect to.
* `family` {number} The family of the IP. It can be `6` for IPv6 or `4` for IPv4.

Emitted when a connection attempt timed out. This is only emitted (and may be
emitted multiple times) if the family autoselection algorithm is enabled
in [`socket.connect(options)`][].
Emitted when a connection attempt is still pending after the configured
`autoSelectFamilyAttemptTimeout` and another attempt is about to start. The
pending attempt remains active and may still establish the connection, unless
`localPort` requires sequential attempts. This is only emitted if the family
autoselection algorithm is enabled in
[`socket.connect(options)`][].

### Event: `'data'`

Expand Down Expand Up @@ -1396,21 +1399,26 @@ For TCP connections, available `options` are:

* `autoSelectFamily` {boolean}: If set to `true`, it enables a family
autodetection algorithm that loosely implements section 5 of [RFC 8305][]. The
`all` option passed to lookup is set to `true` and the sockets attempts to
connect to all obtained IPv6 and IPv4 addresses, in sequence, until a
connection is established. The first returned AAAA address is tried first,
then the first returned A address, then the second returned AAAA address and
so on. Each connection attempt (but the last one) is given the amount of time
specified by the `autoSelectFamilyAttemptTimeout` option before timing out and
trying the next address. Ignored if the `family` option is not `0` or if
`localAddress` is set. Connection errors are not emitted if at least one
connection succeeds. If all connections attempts fails, a single
`all` option passed to lookup is set to `true` and the socket attempts to
connect to all obtained IPv6 and IPv4 addresses until a connection is
established. The first valid address is tried first, followed by addresses
from alternating families in their original order. After
`autoSelectFamilyAttemptTimeout` milliseconds, the next attempt starts without
canceling any pending attempts. The first successful TCP connection wins and
the other attempts are canceled. When `localPort` is set, attempts are made
sequentially because multiple connections cannot portably bind the same local
port. The option is ignored if `family` is not `0` or if `localAddress` is set.
Connection errors are not emitted if at least one
connection succeeds. If all connection attempts fail, a single
`AggregateError` with all failed attempts is emitted. **Default:**
[`net.getDefaultAutoSelectFamily()`][].
* `autoSelectFamilyAttemptTimeout` {number}: The amount of time in milliseconds
to wait for a connection attempt to finish before trying the next address when
using the `autoSelectFamily` option. If set to a positive integer less than
`10`, then the value `10` will be used instead. **Default:**
* `autoSelectFamilyAttemptTimeout` {number}: The delay in milliseconds before
starting the next connection attempt while the previous one is pending when
using the `autoSelectFamily` option. A failed attempt can start the next one
sooner. A pending attempt is not canceled when this delay elapses, except when
`localPort` requires sequential attempts. If set to a positive integer less
than `10`, then the value `10` will be used instead.
**Default:**
[`net.getDefaultAutoSelectFamilyAttemptTimeout()`][].
* `family` {number}: Version of IP stack. Must be `4`, `6`, or `0`. The value
`0` indicates that both IPv4 and IPv6 addresses are allowed. **Default:** `0`.
Expand Down
194 changes: 114 additions & 80 deletions lib/net.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const {
ArrayPrototypeIncludes,
ArrayPrototypeIndexOf,
ArrayPrototypePush,
ArrayPrototypeSplice,
Boolean,
FunctionPrototypeBind,
FunctionPrototypeCall,
Expand Down Expand Up @@ -66,7 +67,6 @@ const {
UV_EBADF,
UV_EINVAL,
UV_ENOTCONN,
UV_ECANCELED,
UV_ETIMEDOUT,
} = internalBinding('uv');
const { convertIpv6StringToBuffer } = internalBinding('cares_wrap');
Expand Down Expand Up @@ -172,6 +172,7 @@ const DEFAULT_IPV6_ADDR = '::';
const noop = () => {};

const kPerfHooksNetConnectContext = Symbol('kPerfHooksNetConnectContext');
const kAutoSelectFamilyContext = Symbol('kAutoSelectFamilyContext');

const dc = require('diagnostics_channel');
const netClientSocketChannel = dc.channel('net.client.socket');
Expand Down Expand Up @@ -1165,6 +1166,12 @@ Socket.prototype._destroy = function(exception, cb) {

this.connecting = false;

const context = this[kAutoSelectFamilyContext];
if (context) {
closeConnectionAttempts(context);
this[kAutoSelectFamilyContext] = undefined;
}

// `_parent` may be null; we use a loose `!= null` check in case external
// code sets it to undefined.
for (let s = this; s != null; s = s._parent) {
Expand Down Expand Up @@ -1477,107 +1484,124 @@ function internalConnect(
}


function internalConnectMultiple(context, canceled) {
function closeConnectionAttempts(context) {
clearTimeout(context[kTimeout]);
const self = context.socket;

// We were requested to abort. Stop all operations
if (self._aborted) {
return;
context[kTimeout] = null;
context.done = true;
for (let i = 0; i < context.pending.length; i++) {
const { handle, req } = context.pending[i];
req.oncomplete = undefined;
handle.close();
}
context.pending.length = 0;
}

// All connections have been tried without success, destroy with error
if (canceled || context.current === context.addresses.length) {
if (context.errors.length === 0) {
self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT());
return;
}
function scheduleConnectionAttempt(context, delay, attempt) {
clearTimeout(context[kTimeout]);
context[kTimeout] = attempt ?
setTimeout(internalConnectMultipleTimeout, delay, context, attempt) :
setTimeout(internalConnectMultiple, delay, context);
if (context.socket._handle?.hasRef?.() === false) context[kTimeout].unref();

@pimterry pimterry Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was surprised by the ?. after hasRef here - I think that's been added as a workaround that hides a worse bug:

All of this will also be called as part of tlssocket.connect, where _handle is a TLSWrap, not a TCPWrap, and it looks like TLSWrap doesn't currently have hasRef, so this'll breaks unref behaviour for all TLS.

I think that's just a straight TLSWrap bug, and so we should add hasRef there and then we can drop the ?. here (if the handle is set, hasRef should work).

}

self.destroy(new NodeAggregateError(context.errors));
return;
function internalConnectMultipleTimeout(context, attempt) {
const { handle, req } = attempt;
if (!context.done && ArrayPrototypeIncludes(context.pending, attempt)) {
debug('connect/multiple: connection to %s:%s is still pending', req.address, req.port);
context.socket.emit('connectionAttemptTimeout', req.address, req.port, req.addressType);
if (context.localPort && !context.done) {
// Two attempts cannot portably bind to the same fixed source port.
const index = ArrayPrototypeIndexOf(context.pending, attempt);
if (index !== -1) {
ArrayPrototypeSplice(context.pending, index, 1);
req.oncomplete = undefined;
handle.close();
ArrayPrototypePush(context.errors, createConnectionError(req, UV_ETIMEDOUT));
}
}
}
if (!context.done && context.socket.connecting) internalConnectMultiple(context);
}

assert(self.connecting);

const current = context.current++;
function internalConnectMultiple(context) {
context[kTimeout] = null;
const self = context.socket;
if (context.done || !self.connecting || self._aborted) return;

if (current > 0) {
self[kReinitializeHandle](new TCP(TCPConstants.SOCKET));
if (context.current === context.addresses.length) {
if (context.pending.length === 0) {
self.destroy(context.errors.length === 0 ?
new ERR_SOCKET_CONNECTION_TIMEOUT() : new NodeAggregateError(context.errors));
}
return;

@pimterry pimterry Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the final candidate fails explicitly but there are still other previous requests pending, we can get stuck.

Two routes:

  • A final call to scheduleConnectionAttempt/internalConnectMultiple after sync failure (e.g. blocklist) ends up here. Given preceeding requests still pending it skips the destroy() call.
  • After async final failure with pending requests, in afterConnectMultiple we similarly return without any action at 2205.

If we hit either case, we just do nothing. We've cleared all the timeouts, but we don't clean up.

If the pending requests never resolve (no response at all) then in practice I think we wait here until the OS times out for us - for Linux defaults for example it looks like this will wait for a little over 2 minutes.

I think we need a final step here. Maybe wait one more timeout and then kill everything? Would be nice to bound that extra timeout tighter somehow (use the correct remaining timeout from the latest of the pending requests somehow) but probably not worth the extra complexity.

}

const current = context.current++;
const { localPort, port, flags } = context;
const { address, family: addressType } = context.addresses[current];
const handle = new TCP(TCPConstants.SOCKET);
if (self._handle?.hasRef?.() === false) handle.unref();
let localAddress;
let err;

if (localPort) {
if (addressType === 4) {
localAddress = DEFAULT_IPV4_ADDR;
err = self._handle.bind(localAddress, localPort);
err = handle.bind(localAddress, localPort);
} else { // addressType === 6
localAddress = DEFAULT_IPV6_ADDR;
err = self._handle.bind6(localAddress, localPort, flags);
err = handle.bind6(localAddress, localPort, flags);
}

debug('connect/multiple: binding to localAddress: %s and localPort: %d (addressType: %d)',
localAddress, localPort, addressType);

err = checkBindError(err, localPort, self._handle);
err = checkBindError(err, localPort, handle);
if (err) {
handle.close();
ArrayPrototypePush(context.errors, new ExceptionWithHostPort(err, 'bind', localAddress, localPort));
internalConnectMultiple(context);
scheduleConnectionAttempt(context, 10);

@pimterry pimterry Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This adds a 10ms delay after explicit connection rejections.

With this PR as is, for the common case of IPv4-only localhost server (where we try IPv6, fail with ECONNRESET, then fallback to IPv4) connection time jumps from sub millisecond currently to 10ms minimum.

The RFC does suggest we should use a 10ms minimum, but all the explanation around it makes it clear that this is worrying about packet loss & timeouts, and refers to simple implementations only. Non-timeout cases should clearly bypass this imo.

Imo we should keep the old behaviour, and instantly move forward after errors.

return;
}
}

if (self.blockList?.check(address, `ipv${addressType}`)) {
handle.close();
const ex = new ERR_IP_BLOCKED(address);
ArrayPrototypePush(context.errors, ex);
self.emit('connectionAttemptFailed', address, port, addressType, ex);
internalConnectMultiple(context);
if (self.connecting && !context.done) scheduleConnectionAttempt(context, 10);
return;
}

debug('connect/multiple: attempting to connect to %s:%d (addressType: %d)', address, port, addressType);
self.emit('connectionAttempt', address, port, addressType);
if (!self.connecting || context.done) {
handle.close();
return;
}

const req = new TCPConnectWrap();
req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context, current);
req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context);
req.address = address;
req.port = port;
req.localAddress = localAddress;
req.localPort = localPort;
req.addressType = addressType;

ArrayPrototypePush(self.autoSelectFamilyAttemptedAddresses, `${address}:${port}`);

if (addressType === 4) {
err = self._handle.connect(req, address, port);
} else {
err = self._handle.connect6(req, address, port);
}
err = addressType === 4 ? handle.connect(req, address, port) : handle.connect6(req, address, port);

if (err) {
const sockname = self._getsockname();
let details;

if (sockname) {
details = sockname.address + ':' + sockname.port;
}

const ex = new ExceptionWithHostPort(err, 'connect', address, port, details);
handle.close();
const ex = createConnectionError(req, err);
ArrayPrototypePush(context.errors, ex);

self.emit('connectionAttemptFailed', address, port, addressType, ex);
internalConnectMultiple(context);
if (self.connecting && !context.done) scheduleConnectionAttempt(context, 10);
return;
}

if (current < context.addresses.length - 1) {
debug('connect/multiple: setting the attempt timeout to %d ms', context.timeout);

// If the attempt has not returned an error, start the connection timer
context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req, self._handle);
const attempt = { handle, req };
ArrayPrototypePush(context.pending, attempt);
if (context.current < context.addresses.length) {
scheduleConnectionAttempt(context, context.timeout, attempt);
}
}

Expand Down Expand Up @@ -1985,7 +2009,10 @@ function lookupAndConnectMultiple(
timeout,
[kTimeout]: null,
errors: [],
pending: [],
done: false,
};
self[kAutoSelectFamilyContext] = context;

self._unrefTimer();
defaultTriggerAsyncIdScope(self[async_id_symbol], internalConnectMultiple, context);
Expand All @@ -2007,6 +2034,13 @@ Socket.prototype.ref = function() {
if (typeof this._handle.ref === 'function') {
this._handle.ref();
}
const context = this[kAutoSelectFamilyContext];
if (context) {
context[kTimeout]?.ref();
for (let i = 0; i < context.pending.length; i++) {
context.pending[i].handle.ref();
}
}

return this;
};
Expand All @@ -2021,6 +2055,13 @@ Socket.prototype.unref = function() {
if (typeof this._handle.unref === 'function') {
this._handle.unref();
}
const context = this[kAutoSelectFamilyContext];
if (context) {
context[kTimeout]?.unref();
for (let i = 0; i < context.pending.length; i++) {
context.pending[i].handle.unref();
}
}

return this;
};
Expand Down Expand Up @@ -2138,61 +2179,54 @@ function createConnectionError(req, status) {
return ex;
}

function afterConnectMultiple(context, current, status, handle, req, readable, writable) {
function afterConnectMultiple(context, status, handle, req, readable, writable) {
debug('connect/multiple: connection attempt to %s:%s completed with status %s', req.address, req.port, status);
let index = -1;
for (let i = 0; i < context.pending.length; i++) {
if (context.pending[i].handle === handle) {
index = i;
break;
}
}
if (index === -1) return;
ArrayPrototypeSplice(context.pending, index, 1);

// Make sure another connection is not spawned
clearTimeout(context[kTimeout]);

// One of the connection has completed and correctly dispatched but after timeout, ignore this one
if (status === 0 && current !== context.current - 1) {
debug('connect/multiple: ignoring successful but timedout connection to %s:%s', req.address, req.port);
if (context.done || !context.socket.connecting) {
handle.close();
return;
}

const self = context.socket;

// Some error occurred, add to the list of exceptions
if (status !== 0) {
handle.close();
const ex = createConnectionError(req, status);
ArrayPrototypePush(context.errors, ex);

self.emit('connectionAttemptFailed', req.address, req.port, req.addressType, ex);

// Try the next address, unless we were aborted
if (context.socket.connecting) {
internalConnectMultiple(context, status === UV_ECANCELED);
if (self.connecting && !context.done) {
if (context.current < context.addresses.length) {
scheduleConnectionAttempt(context, 10);
} else if (context.pending.length === 0) {
internalConnectMultiple(context);
}
}

return;
}

closeConnectionAttempts(context);
self[kAutoSelectFamilyContext] = undefined;
const unrefed = self._handle?.hasRef?.() === false;
self[kReinitializeHandle](handle);
if (unrefed) self._handle.unref();
if (hasObserver('net')) {
startPerf(
self,
kPerfHooksNetConnectContext,
{ type: 'net', name: 'connect', detail: { host: req.address, port: req.port } },
);
}

afterConnect(status, self._handle, req, readable, writable);
}

function internalConnectMultipleTimeout(context, req, handle) {
debug('connect/multiple: connection to %s:%s timed out', req.address, req.port);
context.socket.emit('connectionAttemptTimeout', req.address, req.port, req.addressType);

req.oncomplete = undefined;
ArrayPrototypePush(context.errors, createConnectionError(req, UV_ETIMEDOUT));
handle.close();

// Try the next address, unless we were aborted
if (context.socket.connecting) {
internalConnectMultiple(context);
}
}

function addServerAbortSignalOption(self, options) {
if (options?.signal === undefined) {
return;
Expand Down
Loading
Loading