net: rework autoSelectFamily implementation

Author: ShogunPanda

lib/_tls_wrap.js

@@ -54,7 +54,7 @@ const EE = require('events');
 const net = require('net');
 const tls = require('tls');
 const common = require('_tls_common');
-const { kWrapConnectedHandle } = require('internal/net');
+const { kReinitializeHandle } = require('internal/net');
 const JSStreamSocket = require('internal/js_stream_socket');
 const { Buffer } = require('buffer');
 let debug = require('internal/util/debuglog').debuglog('tls', (fn) => {
@@ -633,9 +633,11 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle) {
   return res;
 };
 
-TLSSocket.prototype[kWrapConnectedHandle] = function(handle) {
+TLSSocket.prototype[kReinitializeHandle] = function reinitializeHandle(handle) {
   this._handle = this._wrapHandle(null, handle);
   this.ssl = this._handle;
+
+  net.Socket.prototype[kReinitializeHandle].call(this, this._handle);
   this._init();
 
   if (this._tlsOptions.enableTrace) {

lib/internal/net.js

@@ -67,7 +67,7 @@ function makeSyncWrite(fd) {
 }
 
 module.exports = {
-  kWrapConnectedHandle: Symbol('wrapConnectedHandle'),
+  kReinitializeHandle: Symbol('reinitializeHandle'),
   isIP,
   isIPv4,
   isIPv6,

lib/net.js

@@ -42,7 +42,7 @@ let debug = require('internal/util/debuglog').debuglog('net', (fn) => {
   debug = fn;
 });
 const {
-  kWrapConnectedHandle,
+  kReinitializeHandle,
   isIP,
   isIPv4,
   isIPv6,
@@ -53,7 +53,8 @@ const assert = require('internal/assert');
 const {
   UV_EADDRINUSE,
   UV_EINVAL,
-  UV_ENOTCONN
+  UV_ENOTCONN,
+  UV_ECANCELED
 } = internalBinding('uv');
 
 const { Buffer } = require('buffer');
@@ -1064,17 +1065,18 @@ function internalConnect(
 }
 
 
-function internalConnectMultiple(context) {
+function internalConnectMultiple(context, canceled) {
   clearTimeout(context[kTimeout]);
   const self = context.socket;
-  assert(self.connecting);
 
   // All connections have been tried without success, destroy with error
-  if (context.current === context.addresses.length) {
+  if (canceled || context.current === context.addresses.length) {
     self.destroy(aggregateErrors(context.errors));
     return;
   }
 
+  assert(self.connecting);
+
   const { localPort, port, flags } = context;
   const { address, family: addressType } = context.addresses[context.current++];
   const handle = new TCP(TCPConstants.SOCKET);
@@ -1101,6 +1103,8 @@ function internalConnectMultiple(context) {
     }
   }
 
+  debug('connect/multiple: attempting to connect to %s:%d (addressType: %d)', address, port, addressType);
+
   const req = new TCPConnectWrap();
   req.oncomplete = FunctionPrototypeBind(afterConnectMultiple, undefined, context);
   req.address = address;
@@ -1190,6 +1194,13 @@ Socket.prototype.connect = function(...args) {
   return this;
 };
 
+Socket.prototype[kReinitializeHandle] = function reinitializeHandle(handle) {
+  this._handle = handle;
+  this._handle[owner_symbol] = this;
+
+  initSocketHandle(this);
+}
+
 function socketToDnsFamily(family) {
   switch (family) {
     case 'IPv4':
@@ -1337,6 +1348,8 @@ function lookupAndConnectMultiple(self, async_id_symbol, lookup, host, options,
       if (!self.connecting) {
         return;
       } else if (err) {
+        self.emit('lookup', err, undefined, undefined, host);
+
         // net.createConnection() creates a net.Socket object and immediately
         // calls net.Socket.connect() on it (that's us). There are no event
         // listeners registered yet so defer the error event to the next tick.
@@ -1529,7 +1542,7 @@ function afterConnectMultiple(context, status, handle, req, readable, writable)
     ArrayPrototypePush(context.errors, ex);
 
     // Try the next address
-    internalConnectMultiple(context);
+    internalConnectMultiple(context, status === UV_ECANCELED);
     return;
   }
 
@@ -1540,13 +1553,9 @@ function afterConnectMultiple(context, status, handle, req, readable, writable)
     return;
   }
 
-  // Perform initialization sequence on the handle, then move on with the regular callback
-  self._handle = handle;
-  initSocketHandle(self);
-
-  if (self[kWrapConnectedHandle]) {
-    self[kWrapConnectedHandle](handle);
-    initSocketHandle(self); // This is called again to initialize the TLSWrap
+  if(self[kReinitializeHandle]) {
+    self[kReinitializeHandle](handle);
+    handle = self._handle;
   }
 
   if (hasObserver('net')) {

test/parallel/test-http2-ping-settings-heapdump.js

@@ -11,6 +11,14 @@ const v8 = require('v8');
 // after it is destroyed, either because they are detached from it or have been
 // destroyed themselves.
 
+// We use an higher autoSelectFamilyAttemptTimeout in this test as the v8.getHeapSnapshot().resume()
+// will slow the connection flow and we don't want the second connection attempt to start.
+let autoSelectFamilyAttemptTimeout = common.platformTimeout(1000);
+if (common.isWindows) {
+  // Some of the windows machines in the CI need more time to establish connection
+  autoSelectFamilyAttemptTimeout = common.platformTimeout(10000);
+}
+
 for (const variant of ['ping', 'settings']) {
   const server = http2.createServer();
   server.on('session', common.mustCall((session) => {
@@ -30,7 +38,7 @@ for (const variant of ['ping', 'settings']) {
   }));
 
   server.listen(0, common.mustCall(() => {
-    const client = http2.connect(`http://localhost:${server.address().port}`,
+    const client = http2.connect(`http://localhost:${server.address().port}`, { autoSelectFamilyAttemptTimeout },
                                  common.mustCall());
     client.on('error', (err) => {
       // We destroy the session so it's possible to get ECONNRESET here.

test/parallel/test-net-dns-custom-lookup.js

@@ -26,13 +26,22 @@ function check(addressType, cb) {
 
   function lookup(host, dnsopts, cb) {
     dnsopts.family = addressType;
+
     if (addressType === 4) {
       process.nextTick(function() {
-        cb(null, common.localhostIPv4, 4);
+        if (dnsopts.all) {
+          cb(null, [{ address: common.localhostIPv4, family: 4 }]);
+        } else {
+          cb(null, common.localhostIPv4, 4);
+        }
       });
     } else {
       process.nextTick(function() {
-        cb(null, '::1', 6);
+        if (dnsopts.all) {
+          cb(null, [{ address: '::1', family: 6 }]);
+        } else {
+          cb(null, '::1', 6);
+        }
       });
     }
   }
@@ -48,7 +57,11 @@ check(4, function() {
     host: 'localhost',
     port: 80,
     lookup(host, dnsopts, cb) {
-      cb(null, undefined, 4);
+      if (dnsopts.all) {
+        cb(null, [{ address: undefined, family: 4 }]);
+      } else {
+        cb(null, undefined, 4);
+      }
     }
   }).on('error', common.expectsError({ code: 'ERR_INVALID_IP_ADDRESS' }));
 }

test/parallel/test-net-dns-lookup.js

@@ -31,10 +31,10 @@ const server = net.createServer(function(client) {
 
 server.listen(0, common.mustCall(function() {
   net.connect(this.address().port, 'localhost')
-    .on('lookup', common.mustCall(function(err, ip, type, host) {
+    .on('lookup', common.mustCallAtLeast(function(err, ip, type, host) {
       assert.strictEqual(err, null);
       assert.match(ip, /^(127\.0\.0\.1|::1)$/);
       assert.match(type.toString(), /^(4|6)$/);
       assert.strictEqual(host, 'localhost');
-    }));
+    }, 1));
 }));

test/parallel/test-net-options-lookup.js

@@ -36,7 +36,11 @@ function connectDoesNotThrow(input) {
 {
   // Verify that an error is emitted when an invalid address family is returned.
   const s = connectDoesNotThrow((host, options, cb) => {
-    cb(null, '127.0.0.1', 100);
+    if (options.all) {
+      cb(null, [{ address: '127.0.0.1', family: 100 }]);
+    } else {
+      cb(null, '127.0.0.1', 100);
+    }
   });
 
   s.on('error', common.expectsError({

test/parallel/test-net-server-reset.js

@@ -20,17 +20,11 @@ server.on('close', common.mustCall());
 
 assert.strictEqual(server, server.listen(0, () => {
   net.createConnection(server.address().port)
-    .on('error', common.mustCall(
-      common.expectsError({
-        code: 'ECONNRESET',
-        name: 'Error'
-      }))
-    );
+    .on('error', common.mustCall((error) => {
+      assert.strictEqual(error.code, 'ECONNRESET');
+    }));
   net.createConnection(server.address().port)
-    .on('error', common.mustCall(
-      common.expectsError({
-        code: 'ECONNRESET',
-        name: 'Error'
-      }))
-    );
+    .on('error', common.mustCall((error) => {
+      assert.strictEqual(error.code, 'ECONNRESET');
+    }));
 }));