[glib: 1/5] Revert "gtestdbus: Properly close server connections"

[Philip Withnall <pwithnall src gnome org>]

commit d03025ba1097ec41c90fa0279bafd3ab47e1a546
Author: Matthew Leeds <matthew leeds endlessm com>
Date:   Mon Jul 1 16:26:57 2019 -0700

    Revert "gtestdbus: Properly close server connections"
    
    This reverts commit baf92d09d69de0d9f9b2d0f77fc62c21fdef4da8.
    
    Closes #787
    
    According to the original commit, this change was made because otherwise
    g_test_dbus_down() following a g_test_dbus_stop() hangs until it times
    out. The timeout being referred to is the 30 seconds which are waited by
    _g_object_unref_and_wait_weak_notify() for the GWeakNotify to be
    triggered when the last strong reference to the singleton
    GDBusConnection object is dropped. But the patch was not correct and the
    leak should have instead been fixed by having the last strong reference
    holder drop their reference on the GDBusConnection before calling
    g_test_dbus_down(). Timing out after 30 seconds is the desired behavior
    in the case where someone holds a reference to the singleton for that
    entire period.
    
    There are a few problems with this patch. First, as pointed out here[1],
    calling g_object_run_dispose() in the idle callback means we are causing
    the GWeakNotify to trigger ~immediately rather than waiting 30 seconds
    to give another owner a chance to unref. Second, since someone else may
    still hold a reference on the object being disposed, they may call
    methods on it after it's been disposed which can seg fault as documented
    here[2] and as I also saw recently in another project.
    
    It's unclear what the original leak being fixed was, but many have been
    fixed between 2013 and now. I ran all the unit tests under valgrind, and
    some do fail (some consistently and some intermittently) but none of the
    failures seem to only happen after this reversion commit. I also
    couldn't find anywhere in the valgrind output where any GDBusConnection
    objects are definitely being lost.
    
    [1] https://gitlab.gnome.org/GNOME/glib/issues/787#note_214226
    [2] https://gitlab.gnome.org/GNOME/glib/issues/787#note_214237

 gio/gtestdbus.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
---
diff --git a/gio/gtestdbus.c b/gio/gtestdbus.c
index 99c9cf8ea..6c060aa7c 100644
--- a/gio/gtestdbus.c
+++ b/gio/gtestdbus.c
@@ -67,15 +67,14 @@ on_weak_notify_timeout (gpointer user_data)
 }
 
 static gboolean
-dispose_on_idle (gpointer object)
+unref_on_idle (gpointer object)
 {
-  g_object_run_dispose (object);
   g_object_unref (object);
   return FALSE;
 }
 
 static gboolean
-_g_object_dispose_and_wait_weak_notify (gpointer object)
+_g_object_unref_and_wait_weak_notify (gpointer object)
 {
   WeakNotifyData data;
   guint timeout_id;
@@ -87,7 +86,7 @@ _g_object_dispose_and_wait_weak_notify (gpointer object)
 
   /* Drop the ref in an idle callback, this is to make sure the mainloop
    * is already running when weak notify happens */
-  g_idle_add (dispose_on_idle, object);
+  g_idle_add (unref_on_idle, object);
 
   /* Make sure we don't block forever */
   timeout_id = g_timeout_add (30 * 1000, on_weak_notify_timeout, &data);
@@ -820,7 +819,7 @@ g_test_dbus_down (GTestDBus *self)
     stop_daemon (self);
 
   if (connection != NULL)
-    _g_object_dispose_and_wait_weak_notify (connection);
+    _g_object_unref_and_wait_weak_notify (connection);
 
   g_test_dbus_unset ();
   _g_bus_forget_singleton (G_BUS_TYPE_SESSION);