From af6dbece870a38dcce89ef55922ecfa085925823 Mon Sep 17 00:00:00 2001
From: Philip Withnall <withnall@endlessm.com>
Date: Fri, 21 Feb 2020 12:07:53 +0000
Subject: [PATCH] tests: Wait until unwatching the gdbus-testserver name has
 completed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, the code in `ensure_gdbus_testserver_up()` created a proxy
object and watched its `name-owner` to see when the
`com.example.TestService` name appeared.

This ended up subscribing to three signals (one of them for name
ownership, and two unused for properties of the proxy), and was racy. In
particular, the `name-owner` property could be set before all D-Bus
messages had been processed — it could have been derived from getting
the owner of the name, for example.

This left unprocessed messages hanging around in the `context`, but that
context was never iterated again, which essentially leaked the
references held by those messages. That included a reference to the
`GDBusConnection`.

The first part of the fix is to simplify the code to use
`g_bus_watch_name_on_connection()`, so there’s only one signal
subscription to worry about.

The second part of the fix is to use the `GDestroyNotify` callback for
the watch data to be notified of when all D-Bus traffic has been
processed and the signal unsubscription is complete. At this point, it’s
guaranteed that there are no idle callbacks pending in the
`GMainContext`, since the `GDestroyNotify` callback is the last one
invoked on the `GMainContext`.

Essentially, this commit uses the `GDestroyNotify` callback as a
synchronisation message between the D-Bus worker thread and the thread
calling `ensure_gdbus_testserver_up()`.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #1515
---
 gio/tests/gdbus-tests.c | 65 ++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/gio/tests/gdbus-tests.c b/gio/tests/gdbus-tests.c
index 66f16e02f..bed0d24f7 100644
--- a/gio/tests/gdbus-tests.c
+++ b/gio/tests/gdbus-tests.c
@@ -86,26 +86,52 @@ _give_up (gpointer data)
   g_return_val_if_reached (TRUE);
 }
 
+typedef struct
+{
+  GMainContext *context;
+  gboolean name_appeared;
+  gboolean unwatch_complete;
+} WatchData;
+
+static void
+name_appeared_cb (GDBusConnection *connection,
+                  const gchar     *name,
+                  const gchar     *name_owner,
+                  gpointer         user_data)
+{
+  WatchData *data = user_data;
+
+  g_assert (name_owner != NULL);
+  data->name_appeared = TRUE;
+  g_main_context_wakeup (data->context);
+}
+
+static void
+watch_free_cb (gpointer user_data)
+{
+  WatchData *data = user_data;
+
+  data->unwatch_complete = TRUE;
+  g_main_context_wakeup (data->context);
+}
+
 void
 ensure_gdbus_testserver_up (GDBusConnection *connection,
                             GMainContext    *context)
 {
-  gchar *name_owner;
   GSource *timeout_source = NULL;
-  GDBusProxy *proxy;
-  GError *error = NULL;
+  guint watch_id;
+  WatchData data = { context, FALSE, FALSE };
 
   g_main_context_push_thread_default (context);
 
-  proxy = g_dbus_proxy_new_sync (connection,
-                                 G_DBUS_PROXY_FLAGS_NONE,
-                                 NULL,                      /* GDBusInterfaceInfo */
-                                 "com.example.TestService", /* name */
-                                 "/com/example/TestObject", /* object path */
-                                 "com.example.Frob",        /* interface */
-                                 NULL, /* GCancellable */
-                                 &error);
-  g_assert_no_error (error);
+  watch_id = g_bus_watch_name_on_connection (connection,
+                                             "com.example.TestService",
+                                             G_BUS_NAME_WATCHER_FLAGS_NONE,
+                                             name_appeared_cb,
+                                             NULL,
+                                             &data,
+                                             watch_free_cb);
 
   timeout_source = g_timeout_source_new_seconds (60);
   g_source_set_callback (timeout_source, _give_up,
@@ -113,20 +139,17 @@ ensure_gdbus_testserver_up (GDBusConnection *connection,
                          NULL);
   g_source_attach (timeout_source, context);
 
-  while (TRUE)
-    {
-      name_owner = g_dbus_proxy_get_name_owner (proxy);
+  while (!data.name_appeared)
+    g_main_context_iteration (context, TRUE);
 
-      if (name_owner != NULL)
-        break;
+  g_bus_unwatch_name (watch_id);
+  watch_id = 0;
 
-      g_main_context_iteration (context, TRUE);
-    }
+  while (!data.unwatch_complete)
+    g_main_context_iteration (context, TRUE);
 
   g_source_destroy (timeout_source);
   g_source_unref (timeout_source);
-  g_free (name_owner);
-  g_object_unref (proxy);
 
   g_main_context_pop_thread_default (context);
 }