[glib/wip/tingping/network-address-fixes] gnetworkaddress: Never end enumeration before resolving completes

[Patrick Griffis <pgriffis src gnome org>]

commit 533209ffacab58c30a20cf4ca27317a679c80187
Author: Patrick Griffis <pgriffis igalia com>
Date:   Fri Feb 8 12:16:14 2019 -0500

    gnetworkaddress: Never end enumeration before resolving completes
    
    Previously once the end of addresses was reached it would return
    NULL even if it was waiting on a dns response. Now it will keep
    waiting so all addresses are received.
    
    Fixes #1680

 gio/gnetworkaddress.c       | 122 ++++++++++++++++++++++++++++----------------
 gio/tests/network-address.c |   8 +--
 2 files changed, 82 insertions(+), 48 deletions(-)
---
diff --git a/gio/gnetworkaddress.c b/gio/gnetworkaddress.c
index e81d311ce..504afc115 100644
--- a/gio/gnetworkaddress.c
+++ b/gio/gnetworkaddress.c
@@ -883,6 +883,12 @@ g_network_address_get_scheme (GNetworkAddress *addr)
 #define G_TYPE_NETWORK_ADDRESS_ADDRESS_ENUMERATOR (_g_network_address_address_enumerator_get_type ())
 #define G_NETWORK_ADDRESS_ADDRESS_ENUMERATOR(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), 
G_TYPE_NETWORK_ADDRESS_ADDRESS_ENUMERATOR, GNetworkAddressAddressEnumerator))
 
+typedef enum {
+  RESOLVE_STATE_NONE = 0,
+  RESOLVE_STATE_WAITING_ON_IPV4 = 1 << 0,
+  RESOLVE_STATE_WAITING_ON_IPV6 = 1 << 1,
+} ResolveState;
+
 typedef struct {
   GSocketAddressEnumerator parent_instance;
 
@@ -891,9 +897,11 @@ typedef struct {
   GList *last_tail; /* (unowned) (nullable) */
   GList *current_item; /* (unowned) (nullable) */
   GTask *queued_task; /* (owned) (nullable) */
+  GTask *waiting_task; /* (owned) (nullable) */
   GError *last_error; /* (owned) (nullable) */
   GSource *wait_source; /* (owned) (nullable) */
   GMainContext *context; /* (owned) (nullable) */
+  ResolveState state;
 } GNetworkAddressAddressEnumerator;
 
 typedef struct {
@@ -916,6 +924,7 @@ g_network_address_address_enumerator_finalize (GObject *object)
       g_clear_pointer (&addr_enum->wait_source, g_source_unref);
     }
   g_clear_object (&addr_enum->queued_task);
+  g_clear_object (&addr_enum->waiting_task);
   g_clear_error (&addr_enum->last_error);
   g_object_unref (addr_enum->addr);
   g_clear_pointer (&addr_enum->addresses, g_list_free);
@@ -1097,20 +1106,49 @@ g_network_address_address_enumerator_next (GSocketAddressEnumerator  *enumerator
   return g_object_ref (sockaddr);
 }
 
+static GSocketAddress *
+init_and_query_next_address (GNetworkAddressAddressEnumerator *addr_enum)
+{
+  GNetworkAddress *addr = addr_enum->addr;
+  GSocketAddress *sockaddr;
+
+  if (addr_enum->addresses == NULL)
+    {
+      addr_enum->current_item = addr_enum->addresses = list_copy_interleaved (addr->priv->sockaddrs);
+      addr_enum->last_tail = g_list_last (addr_enum->addr->priv->sockaddrs);
+      if (addr_enum->current_item)
+        sockaddr = g_object_ref (addr_enum->current_item->data);
+      else
+        sockaddr = NULL;
+    }
+  else
+    {
+      GList *parent_tail = g_list_last (addr_enum->addr->priv->sockaddrs);
+
+      if (addr_enum->last_tail != parent_tail)
+        {
+          addr_enum->current_item = list_concat_interleaved (addr_enum->current_item, g_list_next 
(addr_enum->last_tail));
+          addr_enum->last_tail = parent_tail;
+        }
+
+      if (addr_enum->current_item->next)
+        {
+          addr_enum->current_item = g_list_next (addr_enum->current_item);
+          sockaddr = g_object_ref (addr_enum->current_item->data);
+        }
+      else
+        sockaddr = NULL;
+    }
+
+  return sockaddr;
+}
+
 static void
 complete_queued_task (GNetworkAddressAddressEnumerator *addr_enum,
                       GTask                            *task,
                       GError                           *error)
 {
-  GSocketAddress *sockaddr;
-
-  addr_enum->current_item = addr_enum->addresses = list_copy_interleaved (addr_enum->addr->priv->sockaddrs);
-  addr_enum->last_tail = g_list_last (addr_enum->addr->priv->sockaddrs);
-
-  if (addr_enum->current_item)
-    sockaddr = g_object_ref (addr_enum->current_item->data);
-  else
-    sockaddr = NULL;
+  GSocketAddress *sockaddr = init_and_query_next_address (addr_enum);
 
   if (error)
     g_task_return_error (task, error);
@@ -1127,10 +1165,12 @@ on_address_timeout (gpointer user_data)
   /* Upon completion it may get unref'd by the owner */
   g_object_ref (addr_enum);
 
-  /* If ipv6 didn't come in yet, just complete the task */
   if (addr_enum->queued_task != NULL)
-    complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task),
-                          g_steal_pointer (&addr_enum->last_error));
+      complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task),
+                            g_steal_pointer (&addr_enum->last_error));
+  else if (addr_enum->waiting_task != NULL)
+      complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->waiting_task),
+                            NULL);
 
   g_clear_pointer (&addr_enum->wait_source, g_source_unref);
   g_object_unref (addr_enum);
@@ -1147,7 +1187,8 @@ got_ipv6_addresses (GObject      *source_object,
   GResolver *resolver = G_RESOLVER (source_object);
   GList *addresses;
   GError *error = NULL;
-  gboolean got_ipv4_first = FALSE;
+
+  addr_enum->state ^= RESOLVE_STATE_WAITING_ON_IPV6;
 
   addresses = g_resolver_lookup_by_name_with_flags_finish (resolver, result, &error);
   if (!error)
@@ -1166,14 +1207,13 @@ got_ipv6_addresses (GObject      *source_object,
     {
       g_source_destroy (addr_enum->wait_source);
       g_clear_pointer (&addr_enum->wait_source, g_source_unref);
-      got_ipv4_first = TRUE;
     }
 
   /* If we got an error before ipv4 then let its response handle it.
    * If we get ipv6 response first or error second then
    * immediately complete the task.
    */
-  if (error != NULL && !addr_enum->last_error && !got_ipv4_first)
+  if (error != NULL && !addr_enum->last_error && (addr_enum->state & RESOLVE_STATE_WAITING_ON_IPV4))
     {
       addr_enum->last_error = g_steal_pointer (&error);
 
@@ -1183,6 +1223,10 @@ got_ipv6_addresses (GObject      *source_object,
                              addr_enum, NULL);
       g_source_attach (addr_enum->wait_source, addr_enum->context);
     }
+  else if (addr_enum->waiting_task != NULL)
+    {
+      complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->waiting_task), NULL);
+    }
   else if (addr_enum->queued_task != NULL)
     {
       GError *task_error = NULL;
@@ -1211,6 +1255,8 @@ got_ipv4_addresses (GObject      *source_object,
   GList *addresses;
   GError *error = NULL;
 
+  addr_enum->state ^= RESOLVE_STATE_WAITING_ON_IPV4;
+
   addresses = g_resolver_lookup_by_name_with_flags_finish (resolver, result, &error);
   if (!error)
     {
@@ -1227,8 +1273,9 @@ got_ipv4_addresses (GObject      *source_object,
     }
 
   /* If ipv6 already came in and errored then we return.
-   * If ipv6 returned successfully then we don't need to do anything.
-   * Otherwise we should wait a short while for ipv6 as RFC 8305 suggests.
+   * If ipv6 returned successfully then we don't need to do anything unless
+   * another enumeration was waiting on us.
+   * If ipv6 hasn't come we should wait a short while for it as RFC 8305 suggests.
    */
   if (addr_enum->last_error)
     {
@@ -1237,6 +1284,10 @@ got_ipv4_addresses (GObject      *source_object,
       complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task),
                             g_steal_pointer (&error));
     }
+  else if (addr_enum->waiting_task != NULL)
+    {
+      complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->waiting_task), NULL);
+    }
   else if (addr_enum->queued_task != NULL)
     {
       addr_enum->last_error = g_steal_pointer (&error);
@@ -1246,9 +1297,8 @@ got_ipv4_addresses (GObject      *source_object,
                              addr_enum, NULL);
       g_source_attach (addr_enum->wait_source, addr_enum->context);
     }
-  else if (error != NULL)
-    g_clear_error (&error);
 
+  g_clear_error (&error);
   g_object_unref (addr_enum);
 }
 
@@ -1262,12 +1312,11 @@ g_network_address_address_enumerator_next_async (GSocketAddressEnumerator  *enum
     G_NETWORK_ADDRESS_ADDRESS_ENUMERATOR (enumerator);
   GSocketAddress *sockaddr;
   GTask *task;
-  GNetworkAddress *addr = addr_enum->addr;
 
   task = g_task_new (addr_enum, cancellable, callback, user_data);
   g_task_set_source_tag (task, g_network_address_address_enumerator_next_async);
 
-  if (addr_enum->addresses == NULL)
+  if (addr_enum->addresses == NULL && addr_enum->state == RESOLVE_STATE_NONE)
     {
       GNetworkAddress *addr = addr_enum->addr;
       GResolver *resolver = g_resolver_get_default ();
@@ -1291,6 +1340,7 @@ g_network_address_address_enumerator_next_async (GSocketAddressEnumerator  *enum
                * times before the initial callback has been called */
               g_assert (addr_enum->queued_task == NULL);
 
+              addr_enum->state = RESOLVE_STATE_WAITING_ON_IPV4 | RESOLVE_STATE_WAITING_ON_IPV6;
               addr_enum->queued_task = g_steal_pointer (&task);
               /* Lookup in parallel as per RFC 8305 */
               g_resolver_lookup_by_name_with_flags_async (resolver,
@@ -1311,34 +1361,18 @@ g_network_address_address_enumerator_next_async (GSocketAddressEnumerator  *enum
       g_object_unref (resolver);
     }
 
-  if (addr_enum->addresses == NULL)
-    {
-      g_assert (addr->priv->sockaddrs);
+  sockaddr = init_and_query_next_address (addr_enum);
 
-      addr_enum->current_item = addr_enum->addresses = list_copy_interleaved (addr->priv->sockaddrs);
-      sockaddr = g_object_ref (addr_enum->current_item->data);
+  if (sockaddr == NULL && (addr_enum->state & RESOLVE_STATE_WAITING_ON_IPV4 ||
+                           addr_enum->state & RESOLVE_STATE_WAITING_ON_IPV6))
+    {
+      addr_enum->waiting_task = task;
     }
   else
     {
-      GList *parent_tail = g_list_last (addr_enum->addr->priv->sockaddrs);
-
-      if (addr_enum->last_tail != parent_tail)
-        {
-          addr_enum->current_item = list_concat_interleaved (addr_enum->current_item, g_list_next 
(addr_enum->last_tail));
-          addr_enum->last_tail = parent_tail;
-        }
-
-      if (addr_enum->current_item->next)
-        {
-          addr_enum->current_item = g_list_next (addr_enum->current_item);
-          sockaddr = g_object_ref (addr_enum->current_item->data);
-        }
-      else
-        sockaddr = NULL;
+      g_task_return_pointer (task, sockaddr, g_object_unref);
+      g_object_unref (task);
     }
-
-  g_task_return_pointer (task, sockaddr, g_object_unref);
-  g_object_unref (task);
 }
 
 static GSocketAddress *
diff --git a/gio/tests/network-address.c b/gio/tests/network-address.c
index 0341cccaf..adc678adf 100644
--- a/gio/tests/network-address.c
+++ b/gio/tests/network-address.c
@@ -648,7 +648,7 @@ test_happy_eyeballs_slow_ipv4 (HappyEyeballsFixture *fixture,
 {
   AsyncData data = { 0 };
 
-  /* If ipv4 dns response is a bit slow we just don't get them */
+  /* If ipv4 dns response is a bit slow we still get everything */
 
   data.loop = fixture->loop;
   mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, FAST_DELAY_LESS_THAN_TIMEOUT);
@@ -656,7 +656,7 @@ test_happy_eyeballs_slow_ipv4 (HappyEyeballsFixture *fixture,
   g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
   g_main_loop_run (fixture->loop);
 
-  assert_list_matches_expected (data.addrs, fixture->input_ipv6_results);
+  assert_list_matches_expected (data.addrs, fixture->input_all_results);
 }
 
 static void
@@ -682,7 +682,7 @@ test_happy_eyeballs_very_slow_ipv6 (HappyEyeballsFixture *fixture,
 {
   AsyncData data = { 0 };
 
-  /* If ipv6 is very slow we don't get them */
+  /* If ipv6 is very slow we still get everything */
 
   data.loop = fixture->loop;
   mock_resolver_set_ipv6_delay_ms (fixture->mock_resolver, SLOW_DELAY_MORE_THAN_TIMEOUT);
@@ -690,7 +690,7 @@ test_happy_eyeballs_very_slow_ipv6 (HappyEyeballsFixture *fixture,
   g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
   g_main_loop_run (fixture->loop);
 
-  assert_list_matches_expected (data.addrs, fixture->input_ipv4_results);
+  assert_list_matches_expected (data.addrs, fixture->input_all_results);
 }
 
 static void