From cc3cf6b8b2ad12d54f3474113f0ccfa7dcf66b7b Mon Sep 17 00:00:00 2001
From: Michael Catanzaro <mcatanzaro@gnome.org>
Date: Sat, 4 Jan 2020 20:46:25 -0600
Subject: [PATCH] gsocketclient: run timeout source on the task's main context

This shouldn't make any difference, because this code should only ever
be running in the main context that was thread-default at the time the
task was created, so it should already match the task's context. But
let's make sure, just in case.
---
 gio/gsocketclient.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c
index 6adeee299..81721795b 100644
--- a/gio/gsocketclient.c
+++ b/gio/gsocketclient.c
@@ -1794,7 +1794,7 @@ g_socket_client_enumerator_callback (GObject      *object,
   attempt->connection = (GIOStream *)g_socket_connection_factory_create_connection (socket);
   attempt->timeout_source = g_timeout_source_new (HAPPY_EYEBALLS_CONNECTION_ATTEMPT_TIMEOUT_MS);
   g_source_set_callback (attempt->timeout_source, on_connection_attempt_timeout, attempt, NULL);
-  g_source_attach (attempt->timeout_source, g_main_context_get_thread_default ());
+  g_source_attach (attempt->timeout_source, g_task_get_context (data->task));
   data->connection_attempts = g_slist_append (data->connection_attempts, attempt);
 
   if (g_task_get_cancellable (data->task))
-- 
2.24.1

From d4fcf91460696b09bb2b55c352a023f6dd71c7fe Mon Sep 17 00:00:00 2001
From: Patrick Griffis <tingping@tingping.se>
Date: Thu, 23 Jan 2020 19:58:41 -0800
Subject: [PATCH] Refactor g_socket_client_connect_async()

This is a fairly large refactoring. The highlights are:

- Removing in-progress connections/addresses from GSocketClientAsyncConnectData:

  This caused issues where multiple ConnectionAttempt's would step over eachother
  and modify shared state causing bugs like accidentally bypassing a set proxy.

  Fixes #1871
  Fixes #1989
  Fixes #1902

- Cancelling address enumeration on error/completion

- Queuing successful TCP connections and doing application layer work serially:

  This is more in the spirit of Happy Eyeballs but it also greatly simplifies
  the flow of connection handling so fewer tasks are happening in parallel
  when they don't need to be.

  The behavior also should more closely match that of g_socket_client_connect().

- Better track the state of address enumeration:

  Previously we were over eager to treat enumeration finishing as an error.

  Fixes #1872
  See also #1982

- Add more detailed documentation and logging.

Closes #1995
---
 gio/gsocketclient.c | 459 ++++++++++++++++++++++++++++----------------
 1 file changed, 296 insertions(+), 163 deletions(-)

diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c
index 81721795b..c9943309c 100644
--- a/gio/gsocketclient.c
+++ b/gio/gsocketclient.c
@@ -1337,13 +1337,15 @@ typedef struct
 
   GSocketConnectable *connectable;
   GSocketAddressEnumerator *enumerator;
-  GProxyAddress *proxy_addr;
-  GSocket *socket;
-  GIOStream *connection;
+  GCancellable *enumeration_cancellable;
 
   GSList *connection_attempts;
+  GSList *successful_connections;
   GError *last_error;
 
+  gboolean enumerated_at_least_once;
+  gboolean enumeration_completed;
+  gboolean connection_in_progress;
   gboolean completed;
 } GSocketClientAsyncConnectData;
 
@@ -1355,10 +1357,9 @@ g_socket_client_async_connect_data_free (GSocketClientAsyncConnectData *data)
   data->task = NULL;
   g_clear_object (&data->connectable);
   g_clear_object (&data->enumerator);
-  g_clear_object (&data->proxy_addr);
-  g_clear_object (&data->socket);
-  g_clear_object (&data->connection);
+  g_clear_object (&data->enumeration_cancellable);
   g_slist_free_full (data->connection_attempts, connection_attempt_unref);
+  g_slist_free_full (data->successful_connections, connection_attempt_unref);
 
   g_clear_error (&data->last_error);
 
@@ -1370,6 +1371,7 @@ typedef struct
   GSocketAddress *address;
   GSocket *socket;
   GIOStream *connection;
+  GProxyAddress *proxy_addr;
   GSocketClientAsyncConnectData *data; /* unowned */
   GSource *timeout_source;
   GCancellable *cancellable;
@@ -1401,6 +1403,7 @@ connection_attempt_unref (gpointer pointer)
       g_clear_object (&attempt->socket);
       g_clear_object (&attempt->connection);
       g_clear_object (&attempt->cancellable);
+      g_clear_object (&attempt->proxy_addr);
       if (attempt->timeout_source)
         {
           g_source_destroy (attempt->timeout_source);
@@ -1418,37 +1421,59 @@ connection_attempt_remove (ConnectionAttempt *attempt)
 }
 
 static void
-g_socket_client_async_connect_complete (GSocketClientAsyncConnectData *data)
+cancel_all_attempts (GSocketClientAsyncConnectData *data)
 {
-  g_assert (data->connection);
+  GSList *l;
 
-  if (!G_IS_SOCKET_CONNECTION (data->connection))
+  for (l = data->connection_attempts; l; l = g_slist_next (l))
     {
-      GSocketConnection *wrapper_connection;
-
-      wrapper_connection = g_tcp_wrapper_connection_new (data->connection, data->socket);
-      g_object_unref (data->connection);
-      data->connection = (GIOStream *)wrapper_connection;
+      ConnectionAttempt *attempt_entry = l->data;
+      g_cancellable_cancel (attempt_entry->cancellable);
+      connection_attempt_unref (attempt_entry);
     }
+  g_slist_free (data->connection_attempts);
+  data->connection_attempts = NULL;
 
-  if (!data->completed)
+  g_slist_free_full (data->successful_connections, connection_attempt_unref);
+  data->successful_connections = NULL;
+
+  g_cancellable_cancel (data->enumeration_cancellable);
+}
+
+static void
+g_socket_client_async_connect_complete (ConnectionAttempt *attempt)
+{
+  GSocketClientAsyncConnectData *data = attempt->data;
+  GError *error = NULL;
+  g_assert (attempt->connection);
+  g_assert (!data->completed);
+
+  if (!G_IS_SOCKET_CONNECTION (attempt->connection))
     {
-      GError *error = NULL;
+      GSocketConnection *wrapper_connection;
 
-      if (g_cancellable_set_error_if_cancelled (g_task_get_cancellable (data->task), &error))
-        {
-          g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
-          g_task_return_error (data->task, g_steal_pointer (&error));
-        }
-      else
-        {
-          g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, data->connection);
-          g_task_return_pointer (data->task, g_steal_pointer (&data->connection), g_object_unref);
-        }
+      wrapper_connection = g_tcp_wrapper_connection_new (attempt->connection, attempt->socket);
+      g_object_unref (attempt->connection);
+      attempt->connection = (GIOStream *)wrapper_connection;
+    }
 
-      data->completed = TRUE;
+  data->completed = TRUE;
+  cancel_all_attempts (data);
+
+  if (g_cancellable_set_error_if_cancelled (g_task_get_cancellable (data->task), &error))
+    {
+      g_debug ("GSocketClient: Connection cancelled!");
+      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
+      g_task_return_error (data->task, g_steal_pointer (&error));
+    }
+  else
+    {
+      g_debug ("GSocketClient: Connection successful!");
+      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, attempt->connection);
+      g_task_return_pointer (data->task, g_steal_pointer (&attempt->connection), g_object_unref);
     }
 
+  connection_attempt_unref (attempt);
   g_object_unref (data->task);
 }
 
@@ -1470,59 +1495,63 @@ static void
 enumerator_next_async (GSocketClientAsyncConnectData *data,
                        gboolean                       add_task_ref)
 {
-  /* We need to cleanup the state */
-  g_clear_object (&data->socket);
-  g_clear_object (&data->proxy_addr);
-  g_clear_object (&data->connection);
-
   /* Each enumeration takes a ref. This arg just avoids repeated unrefs when
      an enumeration starts another enumeration */
   if (add_task_ref)
     g_object_ref (data->task);
 
   g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL);
+  g_debug ("GSocketClient: Starting new address enumeration");
   g_socket_address_enumerator_next_async (data->enumerator,
-					  g_task_get_cancellable (data->task),
+					  data->enumeration_cancellable,
 					  g_socket_client_enumerator_callback,
 					  data);
 }
 
+static void try_next_connection_or_finish (GSocketClientAsyncConnectData *, gboolean);
+
 static void
 g_socket_client_tls_handshake_callback (GObject      *object,
 					GAsyncResult *result,
 					gpointer      user_data)
 {
-  GSocketClientAsyncConnectData *data = user_data;
+  ConnectionAttempt *attempt = user_data;
+  GSocketClientAsyncConnectData *data = attempt->data;
 
   if (g_tls_connection_handshake_finish (G_TLS_CONNECTION (object),
 					 result,
 					 &data->last_error))
     {
-      g_object_unref (data->connection);
-      data->connection = G_IO_STREAM (object);
+      g_object_unref (attempt->connection);
+      attempt->connection = G_IO_STREAM (object);
 
-      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_TLS_HANDSHAKED, data->connectable, data->connection);
-      g_socket_client_async_connect_complete (data);
+      g_debug ("GSocketClient: TLS handshake succeeded");
+      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_TLS_HANDSHAKED, data->connectable, attempt->connection);
+      g_socket_client_async_connect_complete (attempt);
     }
   else
     {
       g_object_unref (object);
-      enumerator_next_async (data, FALSE);
+      connection_attempt_unref (attempt);
+      g_debug ("GSocketClient: TLS handshake failed: %s", data->last_error->message);
+      try_next_connection_or_finish (data, TRUE);
     }
 }
 
 static void
-g_socket_client_tls_handshake (GSocketClientAsyncConnectData *data)
+g_socket_client_tls_handshake (ConnectionAttempt *attempt)
 {
+  GSocketClientAsyncConnectData *data = attempt->data;
   GIOStream *tlsconn;
 
   if (!data->client->priv->tls)
     {
-      g_socket_client_async_connect_complete (data);
+      g_socket_client_async_connect_complete (attempt);
       return;
     }
 
-  tlsconn = g_tls_client_connection_new (data->connection,
+  g_debug ("GSocketClient: Starting TLS handshake");
+  tlsconn = g_tls_client_connection_new (attempt->connection,
 					 data->connectable,
 					 &data->last_error);
   if (tlsconn)
@@ -1534,11 +1563,12 @@ g_socket_client_tls_handshake (GSocketClientAsyncConnectData *data)
 					G_PRIORITY_DEFAULT,
 					g_task_get_cancellable (data->task),
 					g_socket_client_tls_handshake_callback,
-					data);
+					attempt);
     }
   else
     {
-      enumerator_next_async (data, FALSE);
+      connection_attempt_unref (attempt);
+      try_next_connection_or_finish (data, TRUE);
     }
 }
 
@@ -1547,23 +1577,38 @@ g_socket_client_proxy_connect_callback (GObject      *object,
 					GAsyncResult *result,
 					gpointer      user_data)
 {
-  GSocketClientAsyncConnectData *data = user_data;
+  ConnectionAttempt *attempt = user_data;
+  GSocketClientAsyncConnectData *data = attempt->data;
 
-  g_object_unref (data->connection);
-  data->connection = g_proxy_connect_finish (G_PROXY (object),
-					     result,
-					     &data->last_error);
-  if (data->connection)
+  g_object_unref (attempt->connection);
+  attempt->connection = g_proxy_connect_finish (G_PROXY (object),
+                                                result,
+                                                &data->last_error);
+  if (attempt->connection)
     {
-      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_PROXY_NEGOTIATED, data->connectable, data->connection);
+      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_PROXY_NEGOTIATED, data->connectable, attempt->connection);
     }
   else
     {
-      enumerator_next_async (data, FALSE);
+      connection_attempt_unref (attempt);
+      try_next_connection_or_finish (data, TRUE);
       return;
     }
 
-  g_socket_client_tls_handshake (data);
+  g_socket_client_tls_handshake (attempt);
+}
+
+static void
+complete_connection_with_error (GSocketClientAsyncConnectData *data,
+                                GError                        *error)
+{
+  g_debug ("GSocketClient: Connection failed: %s", error->message);
+  g_assert (!data->completed);
+
+  g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
+  data->completed = TRUE;
+  cancel_all_attempts (data);
+  g_task_return_error (data->task, error);
 }
 
 static gboolean
@@ -1577,15 +1622,114 @@ task_completed_or_cancelled (GSocketClientAsyncConnectData *data)
     return TRUE;
   else if (g_cancellable_set_error_if_cancelled (cancellable, &error))
     {
-      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
-      g_task_return_error (task, g_steal_pointer (&error));
-      data->completed = TRUE;
+      complete_connection_with_error (data, g_steal_pointer (&error));
       return TRUE;
     }
   else
     return FALSE;
 }
 
+static gboolean
+try_next_successful_connection (GSocketClientAsyncConnectData *data)
+{
+  ConnectionAttempt *attempt;
+  const gchar *protocol;
+  GProxy *proxy;
+
+  if (data->connection_in_progress)
+    return FALSE;
+
+  g_assert (data->successful_connections != NULL);
+  attempt = data->successful_connections->data;
+  g_assert (attempt != NULL);
+  data->successful_connections = g_slist_remove (data->successful_connections, attempt);
+  data->connection_in_progress = TRUE;
+
+  g_debug ("GSocketClient: Starting application layer connection");
+
+  if (!attempt->proxy_addr)
+    {
+      g_socket_client_tls_handshake (g_steal_pointer (&attempt));
+      return TRUE;
+    }
+
+  protocol = g_proxy_address_get_protocol (attempt->proxy_addr);
+
+  /* The connection should not be anything other than TCP,
+   * but let's put a safety guard in case
+   */
+  if (!G_IS_TCP_CONNECTION (attempt->connection))
+    {
+      g_critical ("Trying to proxy over non-TCP connection, this is "
+          "most likely a bug in GLib IO library.");
+
+      g_set_error_literal (&data->last_error,
+          G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+          _("Proxying over a non-TCP connection is not supported."));
+    }
+  else if (g_hash_table_contains (data->client->priv->app_proxies, protocol))
+    {
+      /* Simply complete the connection, we don't want to do TLS handshake
+       * as the application proxy handling may need proxy handshake first */
+      g_socket_client_async_connect_complete (g_steal_pointer (&attempt));
+      return TRUE;
+    }
+  else if ((proxy = g_proxy_get_default_for_protocol (protocol)))
+    {
+      GIOStream *connection = attempt->connection;
+      GProxyAddress *proxy_addr = attempt->proxy_addr;
+
+      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_PROXY_NEGOTIATING, data->connectable, attempt->connection);
+      g_debug ("GSocketClient: Starting proxy connection");
+      g_proxy_connect_async (proxy,
+                             connection,
+                             proxy_addr,
+                             g_task_get_cancellable (data->task),
+                             g_socket_client_proxy_connect_callback,
+                             g_steal_pointer (&attempt));
+      g_object_unref (proxy);
+      return TRUE;
+    }
+  else
+    {
+      g_clear_error (&data->last_error);
+
+      g_set_error (&data->last_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
+          _("Proxy protocol “%s” is not supported."),
+          protocol);
+    }
+
+  data->connection_in_progress = FALSE;
+  g_clear_pointer (&attempt, connection_attempt_unref);
+  return FALSE; /* All non-return paths are failures */
+}
+
+static void
+try_next_connection_or_finish (GSocketClientAsyncConnectData *data,
+                               gboolean                       end_current_connection)
+{
+  if (end_current_connection)
+    data->connection_in_progress = FALSE;
+
+  if (data->connection_in_progress)
+    return;
+
+  /* Keep trying successful connections until one works, each iteration pops one */
+  while (data->successful_connections)
+    {
+      if (try_next_successful_connection (data))
+        return;
+    }
+
+  if (!data->enumeration_completed)
+    {
+      enumerator_next_async (data, FALSE);
+      return;
+    }
+
+  complete_connection_with_error (data, data->last_error);
+}
+
 static void
 g_socket_client_connected_callback (GObject      *source,
 				    GAsyncResult *result,
@@ -1593,10 +1737,7 @@ g_socket_client_connected_callback (GObject      *source,
 {
   ConnectionAttempt *attempt = user_data;
   GSocketClientAsyncConnectData *data = attempt->data;
-  GSList *l;
   GError *error = NULL;
-  GProxy *proxy;
-  const gchar *protocol;
 
   if (task_completed_or_cancelled (data) || g_cancellable_is_cancelled (attempt->cancellable))
     {
@@ -1618,11 +1759,12 @@ g_socket_client_connected_callback (GObject      *source,
         {
           clarify_connect_error (error, data->connectable, attempt->address);
           set_last_error (data, error);
+          g_debug ("GSocketClient: Connection attempt failed: %s", error->message);
           connection_attempt_remove (attempt);
-          enumerator_next_async (data, FALSE);
           connection_attempt_unref (attempt);
+          try_next_connection_or_finish (data, FALSE);
         }
-      else
+      else /* Silently ignore cancelled attempts */
         {
           g_clear_error (&error);
           g_object_unref (data->task);
@@ -1632,74 +1774,21 @@ g_socket_client_connected_callback (GObject      *source,
       return;
     }
 
-  data->socket = g_steal_pointer (&attempt->socket);
-  data->connection = g_steal_pointer (&attempt->connection);
-
-  for (l = data->connection_attempts; l; l = g_slist_next (l))
-    {
-      ConnectionAttempt *attempt_entry = l->data;
-      g_cancellable_cancel (attempt_entry->cancellable);
-      connection_attempt_unref (attempt_entry);
-    }
-  g_slist_free (data->connection_attempts);
-  data->connection_attempts = NULL;
-  connection_attempt_unref (attempt);
-
-  g_socket_connection_set_cached_remote_address ((GSocketConnection*)data->connection, NULL);
-  g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_CONNECTED, data->connectable, data->connection);
+  g_socket_connection_set_cached_remote_address ((GSocketConnection*)attempt->connection, NULL);
+  g_debug ("GSocketClient: TCP connection successful");
+  g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_CONNECTED, data->connectable, attempt->connection);
 
   /* wrong, but backward compatible */
-  g_socket_set_blocking (data->socket, TRUE);
+  g_socket_set_blocking (attempt->socket, TRUE);
 
-  if (!data->proxy_addr)
-    {
-      g_socket_client_tls_handshake (data);
-      return;
-    }
-
-  protocol = g_proxy_address_get_protocol (data->proxy_addr);
-
-  /* The connection should not be anything other than TCP,
-   * but let's put a safety guard in case
+  /* This ends the parallel "happy eyeballs" portion of connecting.
+     Now that we have a successful tcp connection we will attempt to connect
+     at the TLS/Proxy layer. If those layers fail we will move on to the next
+     connection.
    */
-  if (!G_IS_TCP_CONNECTION (data->connection))
-    {
-      g_critical ("Trying to proxy over non-TCP connection, this is "
-          "most likely a bug in GLib IO library.");
-
-      g_set_error_literal (&data->last_error,
-          G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
-          _("Proxying over a non-TCP connection is not supported."));
-
-      enumerator_next_async (data, FALSE);
-    }
-  else if (g_hash_table_contains (data->client->priv->app_proxies, protocol))
-    {
-      /* Simply complete the connection, we don't want to do TLS handshake
-       * as the application proxy handling may need proxy handshake first */
-      g_socket_client_async_connect_complete (data);
-    }
-  else if ((proxy = g_proxy_get_default_for_protocol (protocol)))
-    {
-      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_PROXY_NEGOTIATING, data->connectable, data->connection);
-      g_proxy_connect_async (proxy,
-                             data->connection,
-                             data->proxy_addr,
-                             g_task_get_cancellable (data->task),
-                             g_socket_client_proxy_connect_callback,
-                             data);
-      g_object_unref (proxy);
-    }
-  else
-    {
-      g_clear_error (&data->last_error);
-
-      g_set_error (&data->last_error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
-          _("Proxy protocol “%s” is not supported."),
-          protocol);
-
-      enumerator_next_async (data, FALSE);
-    }
+  connection_attempt_remove (attempt);
+  data->successful_connections = g_slist_append (data->successful_connections, g_steal_pointer (&attempt));
+  try_next_connection_or_finish (data, FALSE);
 }
 
 static gboolean
@@ -1707,7 +1796,11 @@ on_connection_attempt_timeout (gpointer data)
 {
   ConnectionAttempt *attempt = data;
 
-  enumerator_next_async (attempt->data, TRUE);
+  if (!attempt->data->enumeration_completed)
+    {
+      g_debug ("GSocketClient: Timeout reached, trying another enumeration");
+      enumerator_next_async (attempt->data, TRUE);
+    }
 
   g_clear_pointer (&attempt->timeout_source, g_source_unref);
   return G_SOURCE_REMOVE;
@@ -1717,9 +1810,9 @@ static void
 on_connection_cancelled (GCancellable *cancellable,
                          gpointer      data)
 {
-  GCancellable *attempt_cancellable = data;
+  GCancellable *linked_cancellable = G_CANCELLABLE (data);
 
-  g_cancellable_cancel (attempt_cancellable);
+  g_cancellable_cancel (linked_cancellable);
 }
 
 static void
@@ -1743,39 +1836,49 @@ g_socket_client_enumerator_callback (GObject      *object,
 						     result, &error);
   if (address == NULL)
     {
-      if (data->connection_attempts)
+      if (G_UNLIKELY (data->enumeration_completed))
+        return;
+
+      data->enumeration_completed = TRUE;
+      g_debug ("GSocketClient: Address enumeration completed (out of addresses)");
+
+      /* As per API docs: We only care about error if its the first call,
+         after that the enumerator is done.
+
+         Note that we don't care about cancellation errors because
+         task_completed_or_cancelled() above should handle that.
+
+         If this fails and nothing is in progress then we will complete task here.
+       */
+      if ((data->enumerated_at_least_once && !data->connection_attempts && !data->connection_in_progress) ||
+          !data->enumerated_at_least_once)
         {
-          g_object_unref (data->task);
-          return;
+          g_debug ("GSocketClient: Address enumeration failed: %s", error ? error->message : NULL);
+          if (data->last_error)
+            {
+              g_clear_error (&error);
+              error = data->last_error;
+              data->last_error = NULL;
+            }
+          else if (!error)
+            {
+              g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                _("Unknown error on connect"));
+            }
+
+          complete_connection_with_error (data, error);
         }
 
-      g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
-      data->completed = TRUE;
-      if (!error)
-	{
-	  if (data->last_error)
-	    {
-	      error = data->last_error;
-	      data->last_error = NULL;
-	    }
-	  else
-	    {
-	      g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
-				   _("Unknown error on connect"));
-	    }
-	}
-      g_task_return_error (data->task, error);
+      /* Enumeration should never trigger again, drop our ref */
       g_object_unref (data->task);
       return;
     }
 
+  data->enumerated_at_least_once = TRUE;
+  g_debug ("GSocketClient: Address enumeration succeeded");
   g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED,
 			      data->connectable, NULL);
 
-  if (G_IS_PROXY_ADDRESS (address) &&
-      data->client->priv->enable_proxy)
-    data->proxy_addr = g_object_ref (G_PROXY_ADDRESS (address));
-
   g_clear_error (&data->last_error);
 
   socket = create_socket (data->client, address, &data->last_error);
@@ -1793,6 +1896,10 @@ g_socket_client_enumerator_callback (GObject      *object,
   attempt->cancellable = g_cancellable_new ();
   attempt->connection = (GIOStream *)g_socket_connection_factory_create_connection (socket);
   attempt->timeout_source = g_timeout_source_new (HAPPY_EYEBALLS_CONNECTION_ATTEMPT_TIMEOUT_MS);
+
+  if (G_IS_PROXY_ADDRESS (address) && data->client->priv->enable_proxy)
+    attempt->proxy_addr = g_object_ref (G_PROXY_ADDRESS (address));
+
   g_source_set_callback (attempt->timeout_source, on_connection_attempt_timeout, attempt, NULL);
   g_source_attach (attempt->timeout_source, g_task_get_context (data->task));
   data->connection_attempts = g_slist_append (data->connection_attempts, attempt);
@@ -1802,6 +1909,7 @@ g_socket_client_enumerator_callback (GObject      *object,
                            g_object_ref (attempt->cancellable), g_object_unref);
 
   g_socket_connection_set_cached_remote_address ((GSocketConnection *)attempt->connection, address);
+  g_debug ("GSocketClient: Starting TCP connection attempt");
   g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_CONNECTING, data->connectable, attempt->connection);
   g_socket_connection_connect_async (G_SOCKET_CONNECTION (attempt->connection),
 				     address,
@@ -1854,24 +1962,48 @@ g_socket_client_connect_async (GSocketClient       *client,
   else
     data->enumerator = g_socket_connectable_enumerate (connectable);
 
-  /* The flow and ownership here isn't quite obvious:
-    - The task starts an async attempt to connect.
-      - Each attempt holds a single ref on task.
-      - Each attempt may create new attempts by timing out (not a failure) so
-        there are multiple attempts happening in parallel.
-      - Upon failure an attempt will start a new attempt that steals its ref
-        until there are no more attempts left and it drops its ref.
-      - Upon success it will cancel all other attempts and continue on
-        to the rest of the connection (tls, proxies, etc) which do not
-        happen in parallel and at the very end drop its ref.
-      - Upon cancellation an attempt drops its ref.
-   */
+  /* This function tries to match the behavior of g_socket_client_connect ()
+     which is simple enough but much of it is done in parallel to be as responsive
+     as possible as per Happy Eyeballs (RFC 8305). This complicates flow quite a
+     bit but we can describe it in 3 sections:
+
+     Firstly we have address enumeration (DNS):
+       - This may be triggered multiple times by enumerator_next_async().
+       - It also has its own cancellable (data->enumeration_cancellable).
+       - Enumeration is done lazily because GNetworkAddressAddressEnumerator
+         also does work in parallel and may lazily add new addresses.
+       - If the first enumeration errors then the task errors. Otherwise all enumerations
+         will potentially be used (until task or enumeration is cancelled).
+
+      Then we start attempting connections (TCP):
+        - Each connection is independent and kept in a ConnectionAttempt object.
+          - They each hold a ref on the main task and have their own cancellable.
+        - Multiple attempts may happen in parallel as per Happy Eyeballs.
+        - Upon failure or timeouts more connection attempts are made.
+          - If no connections succeed the task errors.
+        - Upon success they are kept in a list of successful connections.
+
+      Lastly we connect at the application layer (TLS, Proxies):
+        - These are done in serial.
+          - The reasoning here is that Happy Eyeballs is about making bad connections responsive
+            at the IP/TCP layers. Issues at the application layer are generally not due to
+            connectivity issues but rather misconfiguration.
+        - Upon failure it will try the next TCP connection until it runs out and
+          the task errors.
+        - Upon success it cancels everything remaining (enumeration and connections)
+          and returns the connection.
+  */
 
   data->task = g_task_new (client, cancellable, callback, user_data);
   g_task_set_check_cancellable (data->task, FALSE); /* We handle this manually */
   g_task_set_source_tag (data->task, g_socket_client_connect_async);
   g_task_set_task_data (data->task, data, (GDestroyNotify)g_socket_client_async_connect_data_free);
 
+  data->enumeration_cancellable = g_cancellable_new ();
+  if (cancellable)
+    g_cancellable_connect (cancellable, G_CALLBACK (on_connection_cancelled),
+                           g_object_ref (data->enumeration_cancellable), g_object_unref);
+
   enumerator_next_async (data, FALSE);
 }
 
@@ -1990,6 +2122,7 @@ g_socket_client_connect_to_uri_async (GSocketClient        *client,
     }
   else
     {
+      g_debug("g_socket_client_connect_to_uri_async");
       g_socket_client_connect_async (client,
 				     connectable, cancellable,
 				     callback, user_data);
-- 
2.24.1

