GNOME Bugzilla – Bug 723719
Close the bus stream gracefully
Last modified: 2016-01-31 12:30:22 UTC
The GDBusWorker's state machine in gdbusprivate.c uses a pending read on the bus socket or some other GIOStream to wait for incoming messages from the bus. When closing the connection, the GIOStream is closed with the read still pending, expecting the read to get cancelled or otherwise somehow fail. As shown in bug #723655 (attachment #268134 [details]), the latter does not happen cleanly. Furthermore, once bug #707912 is fixed, this will stop working and cause g_dbus_connection_close() to fail. Instead, the read should be allowed to finish with the cancellation, and then the closing requests can be fulfilled.
Created attachment 268233 [details] [review] gdbus: Let the pending read finish before closing the connection This prevents the GIOStream from being closed with a read pending on the input substream. g_io_stream_close_async() is actually supposed to fail in this case; see https://bugzilla.gnome.org/show_bug.cgi?id=707912
Bumping up the severity from minor to normal since I'm reasonably sure this will fix Bug #723788, a rare test failure.
Review of attachment 268233 [details] [review]: I'm not really a GIO maintainer, but this looks correct to me. I'd appreciate a double-check from someone else who knows this module (or if someone wants to say "yes you're a maintainer for GDBus" I'll just mark it accepted-commit-now).
*** Bug 723788 has been marked as a duplicate of this bug. ***
(In reply to comment #2) > Bumping up the severity from minor to normal since I'm reasonably sure this > will fix Bug #723788, a rare test failure. Yes, it does. On one of Debian's mipsel test machines, I could reproduce Bug #723788 in GLib 2.38.2 on 2 runs out of 100; with Mikhail's patch, the test has passed 300 times.
I don't really know this code, but if it looks right to you and seems to fix some bugs and not introduce others, then I'd say commit it. (I'd like to get a fix for bug 707912 in too, which is blocked by this.)
The issue seems fixed by bug #743990.