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

GDBusWorker: tolerate read errors while closing

Browse Source 
My previous fix for GNOME#662100 was incomplete: it seems that with some
timings, the stream can be closed with an async read in-flight. This
can make the read fail immediately with G_IO_ERROR_CLOSED instead of
becoming cancelled.

This happens reliably on an embedded device, and rarely on my laptop;
repeating the test 100 times in quick succession reliably reproduces
the bug on my laptop.

It seems as though what we really want is to ignore read errors, once
we've established that we want to close the connection anyway - this
means that after asking to close, you're immune to exit-on-close,
which seems like a good rule.

An additional subtlety is that continuing to read after we know we
want to close is still required, otherwise we'll never emit ::closed.

Bug: https://bugzilla.gnome.org/show_bug.cgi?id=662100
Bug-NB: NB#287088
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Colin Walters <walters@verbum.org>
This commit is contained in:
Simon McVittie 2011-11-11 14:41:50 +00:00
parent 214d249f40
commit 0f01bef4b4
1 changed files with 21 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
gio/gdbusprivate.c
View File

@ -384,6 +384,8 @@ struct GDBusWorker
GList *write_pending_flushes;
/* list of CloseData, protected by write_lock */
GList *pending_close_attempts;
/* no lock - only used from the worker thread */
gboolean close_expected;
};
static void _g_dbus_worker_unref (GDBusWorker *worker);
@ -665,8 +667,18 @@ _g_dbus_worker_do_read_cb (GInputStream *input_stream,
* if the GDBusConnection tells us to close (either via
* _g_dbus_worker_stop, which is called on last-unref, or directly),
* so a cancelled read must mean our connection was closed locally.
*
* If we're closing, other errors are possible - notably,
* G_IO_ERROR_CLOSED can be seen if we close the stream with an async
* read in-flight. It seems sensible to treat all read errors during
* closing as an expected thing that doesn't trip exit-on-close.
*
* Because close_expected can't be set until we get into the worker
* thread, but the cancellable is signalled sooner (from another
* thread), we do still need to check the error.
*/
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
if (worker->close_expected ||
g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
_g_dbus_worker_emit_disconnected (worker, FALSE, NULL);
else
_g_dbus_worker_emit_disconnected (worker, TRUE, error);
@ -806,6 +818,10 @@ _g_dbus_worker_do_read_cb (GInputStream *input_stream,
static void
_g_dbus_worker_do_read_unlocked (GDBusWorker *worker)
{
/* Note that we do need to keep trying to read even if close_expected is
* true, because only failing a read causes us to signal 'closed'.
*/
/* if bytes_wanted is zero, it means start reading a message */
if (worker->read_buffer_bytes_wanted == 0)
{
@ -1396,6 +1412,7 @@ maybe_write_next_message (GDBusWorker *worker)
/* if we want to close the connection, that takes precedence */
if (worker->pending_close_attempts != NULL)
{
worker->close_expected = TRUE;
worker->output_pending = TRUE;
g_io_stream_close_async (worker->stream, G_PRIORITY_DEFAULT,
@ -1638,6 +1655,9 @@ _g_dbus_worker_close (GDBusWorker *worker,
(cancellable == NULL ? NULL : g_object_ref (cancellable));
close_data->result = (result == NULL ? NULL : g_object_ref (result));
/* Don't set worker->close_expected here - we're in the wrong thread.
* It'll be set before the actual close happens.
*/
g_cancellable_cancel (worker->cancellable);
schedule_write_in_worker_thread (worker, NULL, close_data);
}