[glib/glib-2-30] GDBusWorker: combine num_writes_pending with flush_pending
- From: David Zeuthen <davidz src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib/glib-2-30] GDBusWorker: combine num_writes_pending with flush_pending
- Date: Fri, 16 Sep 2011 16:13:02 +0000 (UTC)
commit 068160789ace422002de1c251d374065fd6fa0dc
Author: Simon McVittie <simon mcvittie collabora co uk>
Date: Tue Sep 13 17:37:33 2011 +0100
GDBusWorker: combine num_writes_pending with flush_pending
num_writes_pending was a counter, but it only took values 0 or 1, so make
it a boolean: it would never make sense to be trying to write out two
messages at the same time (they'd get interleaved).
Similarly, we can never be writing and flushing at the same time (that'd
mean we were flushing halfway through a message, which would be pointless)
so combine it with flush_pending too, calling the result output_pending.
Also assert that it takes the expected value whenever we change it,
and document the locking discipline used for it, including a subtle
case in write_message_in_idle_cb where it's not obvious at first glance
why we don't need the lock.
(Having the combined boolean at the top of the block of write-related
struct members improves struct packing on 64-bit platforms, by packing
read_num_ancillary_messages and output_pending into one word.)
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=651268
Bug-NB: NB#271520
Signed-off-by: Simon McVittie <simon mcvittie collabora co uk>
Signed-off-by: David Zeuthen <davidz redhat com>
gio/gdbusprivate.c | 36 +++++++++++++++++++++++++-----------
1 files changed, 25 insertions(+), 11 deletions(-)
---
diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c
index aa2a16e..86af72d 100644
--- a/gio/gdbusprivate.c
+++ b/gio/gdbusprivate.c
@@ -370,13 +370,17 @@ struct GDBusWorker
GSocketControlMessage **read_ancillary_messages;
gint read_num_ancillary_messages;
+ /* TRUE if an async write or flush is pending.
+ * Only the worker thread may change its value, and only with the write_lock.
+ * Other threads may read its value when holding the write_lock.
+ * The worker thread may read its value at any time.
+ */
+ gboolean output_pending;
/* used for writing */
- gint num_writes_pending;
GMutex *write_lock;
GQueue *write_queue;
guint64 write_num_messages_written;
GList *write_pending_flushes;
- gboolean flush_pending;
};
/* ---------------------------------------------------------------------------------------------------- */
@@ -1107,7 +1111,8 @@ ostream_flush_cb (GObject *source_object,
/* Make sure we tell folks that we don't have additional
flushes pending */
g_mutex_lock (data->worker->write_lock);
- data->worker->flush_pending = FALSE;
+ g_assert (data->worker->output_pending);
+ data->worker->output_pending = FALSE;
g_mutex_unlock (data->worker->write_lock);
/* OK, cool, finally kick off the next write */
@@ -1164,7 +1169,8 @@ message_written (GDBusWorker *worker,
}
if (flushers != NULL)
{
- worker->flush_pending = TRUE;
+ g_assert (!worker->output_pending);
+ worker->output_pending = TRUE;
}
g_mutex_unlock (worker->write_lock);
@@ -1198,7 +1204,8 @@ write_message_cb (GObject *source_object,
GError *error;
g_mutex_lock (data->worker->write_lock);
- data->worker->num_writes_pending -= 1;
+ g_assert (data->worker->output_pending);
+ data->worker->output_pending = FALSE;
g_mutex_unlock (data->worker->write_lock);
error = NULL;
@@ -1225,15 +1232,17 @@ maybe_write_next_message (GDBusWorker *worker)
MessageToWriteData *data;
write_next:
+ /* we mustn't try to write two things at once */
+ g_assert (!worker->output_pending);
g_mutex_lock (worker->write_lock);
data = g_queue_pop_head (worker->write_queue);
if (data != NULL)
- worker->num_writes_pending += 1;
+ worker->output_pending = TRUE;
g_mutex_unlock (worker->write_lock);
/* Note that write_lock is only used for protecting the @write_queue
- * and @num_writes_pending fields of the GDBusWorker struct ... which we
+ * and @output_pending fields of the GDBusWorker struct ... which we
* need to modify from arbitrary threads in _g_dbus_worker_send_message().
*
* Therefore, it's fine to drop it here when calling back into user
@@ -1257,7 +1266,7 @@ maybe_write_next_message (GDBusWorker *worker)
{
/* filters dropped message */
g_mutex_lock (worker->write_lock);
- worker->num_writes_pending -= 1;
+ worker->output_pending = FALSE;
g_mutex_unlock (worker->write_lock);
message_to_write_data_free (data);
goto write_next;
@@ -1300,8 +1309,13 @@ static gboolean
write_message_in_idle_cb (gpointer user_data)
{
GDBusWorker *worker = user_data;
- if (worker->num_writes_pending == 0 && !worker->flush_pending)
+
+ /* Because this is the worker thread, we can read this struct member
+ * without holding the lock: no other thread ever modifies it.
+ */
+ if (!worker->output_pending)
maybe_write_next_message (worker);
+
return FALSE;
}
@@ -1328,7 +1342,7 @@ _g_dbus_worker_send_message (GDBusWorker *worker,
g_mutex_lock (worker->write_lock);
g_queue_push_tail (worker->write_queue, data);
- if (worker->num_writes_pending == 0)
+ if (!worker->output_pending)
{
GSource *idle_source;
idle_source = g_idle_source_new ();
@@ -1373,7 +1387,7 @@ _g_dbus_worker_new (GIOStream *stream,
worker->stream = g_object_ref (stream);
worker->capabilities = capabilities;
worker->cancellable = g_cancellable_new ();
- worker->flush_pending = FALSE;
+ worker->output_pending = FALSE;
worker->frozen = initially_frozen;
worker->received_messages_while_frozen = g_queue_new ();
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]