After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 711807 - gtestdbus: Properly close server connections
gtestdbus: Properly close server connections
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gdbus
unspecified
Other All
: Normal normal
: ---
Assigned To: David Zeuthen (not reading bugmail)
gtkdev
Depends on:
Blocks: 711799
 
 
Reported: 2013-11-10 21:18 UTC by Stef Walter
Modified: 2018-05-24 16:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtestdbus: Properly close server connections (1.62 KB, patch)
2013-11-10 21:18 UTC, Stef Walter
committed Details | Review
Work around test failure in gdbus-names (863 bytes, patch)
2014-02-17 23:18 UTC, Allison Karlitskaya (desrt)
committed Details | Review
Revert "gtestdbus: Properly close server connections" (1.70 KB, patch)
2014-05-01 04:18 UTC, Stef Walter
none Details | Review

Description Stef Walter 2013-11-10 21:18:48 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?
Comment 1 Stef Walter 2013-11-10 21:18:52 UTC
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.
Comment 2 Stef Walter 2013-11-11 07:55:36 UTC
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.
Comment 3 Colin Walters 2013-11-11 15:59:54 UTC
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.
Comment 4 Allison Karlitskaya (desrt) 2014-02-17 23:10:15 UTC
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.
Comment 5 Allison Karlitskaya (desrt) 2014-02-17 23:18:31 UTC
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.
Comment 6 Allison Karlitskaya (desrt) 2014-02-17 23:19:13 UTC
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.
Comment 7 Xavier Claessens 2014-02-18 20:44:57 UTC
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?
Comment 8 Stef Walter 2014-02-19 07:56:31 UTC
(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.
Comment 9 Simon McVittie 2014-03-24 10:38:14 UTC
(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
  • #0 _g_dbus_worker_send_message
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusprivate.c line 1626
  • #1 g_dbus_connection_send_message_unlocked
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 1710
  • #2 g_dbus_connection_send_message_with_reply_unlocked
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 1993
  • #3 g_dbus_connection_send_message_with_reply
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 2097
  • #4 g_dbus_connection_send_message_with_reply_sync
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 2244
  • #5 g_dbus_connection_call_sync_internal
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 5723
  • #6 g_dbus_connection_call_with_unix_fd_list_sync
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusconnection.c line 6069
  • #7 g_dbus_proxy_call_sync_internal
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusproxy.c line 2901
  • #8 g_dbus_proxy_call_sync
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gio/gdbusproxy.c line 3093
  • #9 ??
    from /usr/lib/libedataserver-1.2.so.17
  • #10 e_gdbus_proxy_method_call_sync_void__void
    from /usr/lib/libedataserver-1.2.so.17
  • #11 e_book_client_view_stop
    from /usr/lib/libebook-1.2.so.14
  • #12 edsf_persona_store_finalize
    at /home/smcv/src/gnome/folks-tp/backends/eds/lib/edsf-persona-store.vala line 346
  • #13 g_object_unref
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gobject/gobject.c line 3112
  • #14 ??
    from /usr/lib/x86_64-linux-gnu/libgee-0.8.so.2
  • #15 ??
    from /usr/lib/x86_64-linux-gnu/libgee-0.8.so.2
  • #16 g_object_unref
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gobject/gobject.c line 3112
  • #17 folks_individual_aggregator_finalize
    at /home/smcv/src/gnome/folks-tp/folks/individual-aggregator.vala line 128
  • #18 g_object_unref
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gobject/gobject.c line 3112
  • #19 individual_retrieval_tests_finalize
    at /home/smcv/src/gnome/folks-tp/tests/eds/individual-retrieval.vala line 28
  • #20 g_object_unref
    at /build/glib2.0-V5GbKs/glib2.0-2.39.92/./gobject/gobject.c line 3112
  • #21 _vala_main
    at /home/smcv/src/gnome/folks-tp/tests/eds/individual-retrieval.vala line 128
  • #22 __libc_start_main
    at libc-start.c line 287
  • #23 _start

Comment 10 Simon McVittie 2014-03-24 10:39:10 UTC
(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..."
Comment 11 Simon McVittie 2014-03-24 16:14:58 UTC
The equivalent of this patch might be worthwhile:

https://bugzilla.gnome.org/show_bug.cgi?id=726973#c1
Comment 12 Simon McVittie 2014-03-24 16:38:41 UTC
See also Bug #726978
Comment 13 Stef Walter 2014-05-01 04:18:14 UTC
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:
Comment 14 Stef Walter 2014-05-01 04:20:05 UTC
I agree with Ryan, lets revert this for now.

The glib tests still fail for me due to the unrelated #727920 test problem.
Comment 15 Guillaume Desmottes 2016-08-15 15:18:31 UTC
Shouldn't this patch be reverted now?
Comment 16 GNOME Infrastructure Team 2018-05-24 16:00:17 UTC
-- 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.