GNOME Bugzilla – Bug 711807
gtestdbus: Properly close server connections
Last modified: 2018-05-24 16:00:17 UTC
g_test_dbus_down() following a g_test_dbus_stop() just hangs until a timeout. This a documented sequence of calls. Patch fixes the issue. May be a better way to accomplish this?
Created attachment 259494 [details] [review] gtestdbus: Properly close server connections Otherwise g_test_dbus_down() following a g_test_dbus_stop() will hang until it times out.
This is causing a few tests to fail. dbus-appinfo was failing before I started my cleanup work, but now more are failing that they try to shutdown properly.
Review of attachment 259494 [details] [review]: It's a little unusual to call g_object_run_dispose(), but this API looks like it's explicitly trying to accomplish that effect.
This patch misses the mark. Calling g_object_run_dispose() here is just about the worst possible thing that we can do. We're using weak notifies to wait for the last reference to drop, but those will also be fired in response to dispose(). In effect, this patch always makes g_test_dbus_down() exit more or less immediately.
Created attachment 269480 [details] [review] Work around test failure in gdbus-names This is caused by g_test_dbus_down() returning too soon. Add a sleep for now.
Attachment 269480 [details] pushed as c37cd19 - Work around test failure in gdbus-names Pushing this so I can do a release. We should revert it once we have a proper fix for this bug.
If I revert both Stef's and Ryan's patches, while true; do ./gdbus-names; done works perfectly well even if I do background work to change the load. I don't understand what was the initial issue. Maybe already fixed in another way?
(In reply to comment #7) > I don't understand what was the initial issue. Maybe already fixed in another > way? The original issue was that it leaks the GDBusConnection and worker. See the blocker bug.
(In reply to comment #4) > Calling g_object_run_dispose() here is just about the worst possible thing that > we can do. It appears that GDBusConnection is designed to survive use-after-dispose in a semi-graceful way. If anything that was not yet torn down still has a ref to the GDBusConnection, and tries to send a message after it was disposed, we'll get a segfault by trying to send a message to a NULL GDBusWorker. I've seen this in a version of Folks patched to work with Evolution 3.8 (long story), and a version of telepathy-glib patched to use GDBus instead of dbus-glib. I realise that this is not necessarily considered to be a bug, because g_test_dbus_down() is already effectively an assertion that there are no more refs to the GDBusConnection, unless your tests run with non-fatal warnings and you don't mind a 30-second delay. The traceback below is from Folks, for instance: Program received signal SIGSEGV, Segmentation fault. _g_dbus_worker_send_message (worker=0x0, message=message@entry=0x2aaabc03aca0, blob=blob@entry=0x6634b0 "l\001", blob_len=168) at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusprivate.c:1626 1626 /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusprivate.c: No such file or directory. (gdb) bt
+ Trace 233386
(In reply to comment #9) > It appears that GDBusConnection is designed to survive use-after-dispose in a > semi-graceful way. Sorry, that should say "... is *not* designed to..."
The equivalent of this patch might be worthwhile: https://bugzilla.gnome.org/show_bug.cgi?id=726973#c1
See also Bug #726978
Created attachment 275515 [details] [review] Revert "gtestdbus: Properly close server connections" This reverts commit baf92d09d69de0d9f9b2d0f77fc62c21fdef4da8. GDBusConnection isn't ready to have its connections disposed, they just crash. See comments here:
I agree with Ryan, lets revert this for now. The glib tests still fail for me due to the unrelated #727920 test problem.
Shouldn't this patch be reverted now?
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/787.