[dart:io] Fix _connecting counting in HttpClient

Resolves comments in previous cl and fix a problem where _connecting
decrements twice.

Bug: #34477
Change-Id: Icf5a98f4cf9e3f6ca582fe69f41074d482e00f0e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148841
Commit-Queue: Zichang Guo <[email protected]>
Reviewed-by: Jonas Termansen <[email protected]>

Author: zichangg

sdk/lib/_http/http_impl.dart

@@ -2081,7 +2081,6 @@ class _ConnectionTarget {
       if (connectionTimeout != null) {
         socketFuture = socketFuture.timeout(connectionTimeout, onTimeout: () {
           _socketTasks.remove(task);
-          _connecting--;
           task.cancel();
           return null;
         });
@@ -2092,13 +2091,13 @@ class _ConnectionTarget {
         // is completed with 'null' by the onTimeout callback above. In this
         // case, propagate a SocketException as specified by the
         // HttpClient.connectionTimeout docs.
+        _connecting--;
         if (socket == null) {
           assert(connectionTimeout != null);
           throw new SocketException(
               "HTTP connection timed out after ${connectionTimeout}, "
               "host: ${host}, port: ${port}");
         }
-        _connecting--;
         socket.setOption(SocketOption.tcpNoDelay, true);
         var connection =
             new _HttpClientConnection(key, socket, client, false, context);
@@ -2119,7 +2118,6 @@ class _ConnectionTarget {
           return new _ConnectionInfo(connection, proxy);
         }
       }, onError: (error) {
-        _connecting--;
         _socketTasks.remove(task);
         _checkPending();
         throw error;

sdk_nnbd/lib/_http/http_impl.dart

@@ -2080,7 +2080,6 @@ class _ConnectionTarget {
       if (connectionTimeout != null) {
         socketFuture = socketFuture.timeout(connectionTimeout, onTimeout: () {
           _socketTasks.remove(task);
-          _connecting--;
           task.cancel();
           return null;
         });
@@ -2091,13 +2090,13 @@ class _ConnectionTarget {
         // is completed with 'null' by the onTimeout callback above. In this
         // case, propagate a SocketException as specified by the
         // HttpClient.connectionTimeout docs.
+        _connecting--;
         if (socket == null) {
           assert(connectionTimeout != null);
           throw new SocketException(
               "HTTP connection timed out after ${connectionTimeout}, "
               "host: ${host}, port: ${port}");
         }
-        _connecting--;
         socket.setOption(SocketOption.tcpNoDelay, true);
         var connection =
             new _HttpClientConnection(key, socket, client, false, context);
@@ -2118,7 +2117,6 @@ class _ConnectionTarget {
           return new _ConnectionInfo(connection, proxy);
         }
       }, onError: (error) {
-        _connecting--;
         _socketTasks.remove(task);
         _checkPending();
         throw error;

tests/standalone/io/http_client_connect_test.dart

@@ -285,19 +285,27 @@ void testMaxConnectionsPerHost(int connectionCap, int connections) {
   });
 }
 
-void testMaxConnectionsWithFailure() async {
-  var client = new HttpClient();
+Future<void> testMaxConnectionsWithFailure() async {
+  // When DNS lookup failed, counter for connecting doesn't decrement which
+  // prevents the following connections.
+  final client = HttpClient();
   client.maxConnectionsPerHost = 1;
   try {
-    await client.getUrl(Uri.parse('http://mydomainismissing'));
-  } catch (e) {}
+    await client.getUrl(Uri.parse('http://domain.invalid'));
+  } catch (e) {
+    if (e is! SocketException) {
+      Expect.fail("Unexpected exception $e is thrown");
+    }
+  }
   try {
-    await client.getUrl(Uri.parse('http://mydomainismissing'));
+    await client.getUrl(Uri.parse('http://domain.invalid'));
+    Expect.fail("Calls exceed client's maxConnectionsPerHost should throw "
+        "exceptions as well");
   } catch (e) {
-    return;
+    if (e is! SocketException) {
+      Expect.fail("Unexpected exception $e is thrown");
+    }
   }
-
-  Expect.fail('second call should also fail');
 }
 
 void main() {

tests/standalone_2/io/http_client_connect_test.dart

@@ -283,19 +283,27 @@ void testMaxConnectionsPerHost(int connectionCap, int connections) {
   });
 }
 
-void testMaxConnectionsWithFailure() async {
-  var client = new HttpClient();
+Future<void> testMaxConnectionsWithFailure() async {
+  // When DNS lookup failed, counter for connecting doesn't decrement which
+  // prevents the following connections.
+  final client = HttpClient();
   client.maxConnectionsPerHost = 1;
   try {
-    await client.getUrl(Uri.parse('http://mydomainismissing'));
-  } catch (e) {}
+    await client.getUrl(Uri.parse('http://domain.invalid'));
+  } catch (e) {
+    if (e is! SocketException) {
+      Expect.fail("Unexpected exception $e is thrown");
+    }
+  }
   try {
-    await client.getUrl(Uri.parse('http://mydomainismissing'));
+    await client.getUrl(Uri.parse('http://domain.invalid'));
+    Expect.fail("Calls exceed client's maxConnectionsPerHost should throw "
+        "exceptions as well");
   } catch (e) {
-    return;
+    if (e is! SocketException) {
+      Expect.fail("Unexpected exception $e is thrown");
+    }
   }
-
-  Expect.fail('second call should also fail');
 }
 
 void main() {