GeezerVison is showing github.com — net: fix family autoselection timeout handling · nodejs/node@750e53c
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/net.js +19-13Lines changed: 19 additions & 13 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -134,7 +134,6 @@ let autoSelectFamilyAttemptTimeoutDefault = 250; | 134 | 134 |
| 135 | 135 | const { clearTimeout, setTimeout } = require('timers'); | 136 | 136 | const { kTimeout } = require('internal/timers'); | 137 | | -const kTimeoutTriggered = Symbol('kTimeoutTriggered'); | 138 | 137 |
| 139 | 138 | const DEFAULT_IPV4_ADDR = '0.0.0.0'; | 140 | 139 | const DEFAULT_IPV6_ADDR = '::'; | @@ -1106,9 +1105,10 @@ function internalConnectMultiple(context, canceled) { | 1106 | 1105 |
| 1107 | 1106 | assert(self.connecting); | 1108 | 1107 |
| 1109 | | - const handle = context.current === 0 ? self._handle : new TCP(TCPConstants.SOCKET); | | 1108 | + const current = context.current++; | | 1109 | + const handle = current === 0 ? self._handle : new TCP(TCPConstants.SOCKET); | 1110 | 1110 | const { localPort, port, flags } = context; | 1111 | | - const { address, family: addressType } = context.addresses[context.current++]; | | 1111 | + const { address, family: addressType } = context.addresses[current]; | 1112 | 1112 | let localAddress; | 1113 | 1113 | let err; | 1114 | 1114 |
| @@ -1135,7 +1135,7 @@ function internalConnectMultiple(context, canceled) { | 1135 | 1135 | debug('connect/multiple: attempting to connect to %s:%d (addressType: %d)', address, port, addressType); | 1136 | 1136 |
| 1137 | 1137 | const req = new TCPConnectWrap(); | 1138 | | - req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context); | | 1138 | + req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context, current); | 1139 | 1139 | req.address = address; | 1140 | 1140 | req.port = port; | 1141 | 1141 | req.localAddress = localAddress; | @@ -1162,8 +1162,12 @@ function internalConnectMultiple(context, canceled) { | 1162 | 1162 | return; | 1163 | 1163 | } | 1164 | 1164 |
| 1165 | | - // If the attempt has not returned an error, start the connection timer | 1166 | | - context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req); | | 1165 | + if (current < context.addresses.length - 1) { | | 1166 | + debug('connect/multiple: setting the attempt timeout to %d ms', context.timeout); | | 1167 | + | | 1168 | + // If the attempt has not returned an error, start the connection timer | | 1169 | + context[kTimeout] = setTimeout(internalConnectMultipleTimeout, context.timeout, context, req, handle); | | 1170 | + } | 1167 | 1171 | } | 1168 | 1172 |
| 1169 | 1173 | Socket.prototype.connect = function(...args) { | @@ -1478,7 +1482,6 @@ function lookupAndConnectMultiple( | 1478 | 1482 | localPort, | 1479 | 1483 | timeout, | 1480 | 1484 | [kTimeout]: null, | 1481 | | - [kTimeoutTriggered]: false, | 1482 | 1485 | errors: [], | 1483 | 1486 | }; | 1484 | 1487 |
| @@ -1581,18 +1584,19 @@ function afterConnect(status, handle, req, readable, writable) { | 1581 | 1584 | } | 1582 | 1585 | } | 1583 | 1586 |
| 1584 | | -function afterConnectMultiple(context, status, handle, req, readable, writable) { | | 1587 | +function afterConnectMultiple(context, current, status, handle, req, readable, writable) { | | 1588 | + // Make sure another connection is not spawned | | 1589 | + clearTimeout(context[kTimeout]); | | 1590 | + | 1585 | 1591 | // One of the connection has completed and correctly dispatched but after timeout, ignore this one | 1586 | | - if (context[kTimeoutTriggered]) { | | 1592 | + if (status === 0 && current !== context.current - 1) { | 1587 | 1593 | debug('connect/multiple: ignoring successful but timedout connection to %s:%s', req.address, req.port); | 1588 | 1594 | handle.close(); | 1589 | 1595 | return; | 1590 | 1596 | } | 1591 | 1597 |
| 1592 | 1598 | const self = context.socket; | 1593 | 1599 |
| 1594 | | - // Make sure another connection is not spawned | 1595 | | - clearTimeout(context[kTimeout]); | 1596 | 1600 |
| 1597 | 1601 | // Some error occurred, add to the list of exceptions | 1598 | 1602 | if (status !== 0) { | @@ -1633,8 +1637,10 @@ function afterConnectMultiple(context, status, handle, req, readable, writable) | 1633 | 1637 | afterConnect(status, handle, req, readable, writable); | 1634 | 1638 | } | 1635 | 1639 |
| 1636 | | -function internalConnectMultipleTimeout(context, req) { | 1637 | | - context[kTimeoutTriggered] = true; | | 1640 | +function internalConnectMultipleTimeout(context, req, handle) { | | 1641 | + debug('connect/multiple: connection to %s:%s timed out', req.address, req.port); | | 1642 | + req.oncomplete = undefined; | | 1643 | + handle.close(); | 1638 | 1644 | internalConnectMultiple(context); | 1639 | 1645 | } | 1640 | 1646 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-net-autoselectfamily.js +61-3Lines changed: 61 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -36,7 +36,15 @@ function _lookup(resolver, hostname, options, cb) { | 36 | 36 | }); | 37 | 37 | } | 38 | 38 |
| 39 | | -function createDnsServer(ipv6Addr, ipv4Addr, cb) { | | 39 | +function createDnsServer(ipv6Addrs, ipv4Addrs, cb) { | | 40 | + if (!Array.isArray(ipv6Addrs)) { | | 41 | + ipv6Addrs = [ipv6Addrs]; | | 42 | + } | | 43 | + | | 44 | + if (!Array.isArray(ipv4Addrs)) { | | 45 | + ipv4Addrs = [ipv4Addrs]; | | 46 | + } | | 47 | + | 40 | 48 | // Create a DNS server which replies with a AAAA and a A record for the same host | 41 | 49 | const socket = dgram.createSocket('udp4'); | 42 | 50 |
| @@ -49,8 +57,8 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) { | 49 | 57 | id: parsed.id, | 50 | 58 | questions: parsed.questions, | 51 | 59 | answers: [ | 52 | | - { type: 'AAAA', address: ipv6Addr, ttl: 123, domain: 'example.org' }, | 53 | | - { type: 'A', address: ipv4Addr, ttl: 123, domain: 'example.org' }, | | 60 | + ...ipv6Addrs.map((address) => ({ type: 'AAAA', address, ttl: 123, domain: 'example.org' })), | | 61 | + ...ipv4Addrs.map((address) => ({ type: 'A', address, ttl: 123, domain: 'example.org' })), | 54 | 62 | ] | 55 | 63 | }), port, address); | 56 | 64 | })); | @@ -106,6 +114,56 @@ function createDnsServer(ipv6Addr, ipv4Addr, cb) { | 106 | 114 | })); | 107 | 115 | } | 108 | 116 |
| | 117 | +// Test that only the last successful connection is established. | | 118 | +{ | | 119 | + createDnsServer( | | 120 | + '::1', | | 121 | + ['104.20.22.46', '104.20.23.46', '127.0.0.1'], | | 122 | + common.mustCall(function({ dnsServer, lookup }) { | | 123 | + const ipv4Server = createServer((socket) => { | | 124 | + socket.on('data', common.mustCall(() => { | | 125 | + socket.write('response-ipv4'); | | 126 | + socket.end(); | | 127 | + })); | | 128 | + }); | | 129 | + | | 130 | + ipv4Server.listen(0, '127.0.0.1', common.mustCall(() => { | | 131 | + const port = ipv4Server.address().port; | | 132 | + | | 133 | + const connection = createConnection({ | | 134 | + host: 'example.org', | | 135 | + port: port, | | 136 | + lookup, | | 137 | + autoSelectFamily: true, | | 138 | + autoSelectFamilyAttemptTimeout, | | 139 | + }); | | 140 | + | | 141 | + let response = ''; | | 142 | + connection.setEncoding('utf-8'); | | 143 | + | | 144 | + connection.on('ready', common.mustCall(() => { | | 145 | + assert.deepStrictEqual( | | 146 | + connection.autoSelectFamilyAttemptedAddresses, | | 147 | + [`::1:${port}`, `104.20.22.46:${port}`, `104.20.23.46:${port}`, `127.0.0.1:${port}`] | | 148 | + ); | | 149 | + })); | | 150 | + | | 151 | + connection.on('data', (chunk) => { | | 152 | + response += chunk; | | 153 | + }); | | 154 | + | | 155 | + connection.on('end', common.mustCall(() => { | | 156 | + assert.strictEqual(response, 'response-ipv4'); | | 157 | + ipv4Server.close(); | | 158 | + dnsServer.close(); | | 159 | + })); | | 160 | + | | 161 | + connection.write('request'); | | 162 | + })); | | 163 | + }) | | 164 | + ); | | 165 | +} | | 166 | + | 109 | 167 | // Test that IPV4 is NOT reached if IPV6 is reachable | 110 | 168 | if (common.hasIPv6) { | 111 | 169 | createDnsServer('::1', '127.0.0.1', common.mustCall(function({ dnsServer, lookup }) { | |
|