luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-03-06 07:54:46 +01:00
Code Issues Packages Projects Releases Wiki Activity

Revert "gtestdbus: Properly close server connections"

Browse Source 
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
This commit is contained in:
Matthew Leeds 2019-07-01 16:26:57 -07:00
parent a656555f76
commit d03025ba10
1 changed files with 4 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
gio/gtestdbus.c
View File

@ -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);