diff options
| author | Philip Withnall <pwithnall@endlessos.org> | 2023-02-22 14:55:40 +0000 |
|---|---|---|
| committer | Philip Withnall <pwithnall@endlessos.org> | 2023-04-14 15:37:21 +0100 |
| commit | 0a84c182e28f50be2263e42e0bc21074dd523701 (patch) | |
| tree | 2e20874e1ed0df63dcf9db71948e83baac4a74dd /gio | |
| parent | b84ec21f9c4c57990309e691206582c589f59770 (diff) | |
| download | glib-0a84c182e28f50be2263e42e0bc21074dd523701.tar.gz | |
gdbusconnection: Improve refcount handling of timeout source
The ref on the timeout source owned by `SendMessageData` was being
dropped just after attaching the source to the main context, leaving it
unowned in that struct. That meant the only ref on the source was held
by the `GMainContext` it was attached to.
This ref was dropped when returning `G_SOURCE_REMOVE` from
`send_message_with_reply_timeout_cb()`. Before that happens,
`send_message_data_deliver_error()` is called, which normally calls
`send_message_with_reply_cleanup()` and destroys the source.
However, if `send_message_data_deliver_error()` is called when the
message has already been delivered, calling
`send_message_with_reply_cleanup()` will be skipped. This leaves the
source pointer in `SendMessageData` dangling, which will cause problems
when `g_source_destroy()` is subsequently called on it.
I’m not sure if it’s possible in practice for this situation to occur,
but the code certainly does nothing to prevent it, and it’s easy enough
to avoid by keeping a strong ref on the source in `SendMessageData`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1264
Diffstat (limited to 'gio')
| -rw-r--r-- | gio/gdbusconnection.c | 8 |
1 files changed, 4 insertions, 4 deletions
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index f4bc21bb3..bed1ff284 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -1751,7 +1751,7 @@ typedef struct gulong cancellable_handler_id; - GSource *timeout_source; + GSource *timeout_source; /* (owned) (nullable) */ gboolean delivered; } SendMessageData; @@ -1760,6 +1760,7 @@ typedef struct static void send_message_data_free (SendMessageData *data) { + /* These should already have been cleared by send_message_with_reply_cleanup(). */ g_assert (data->timeout_source == NULL); g_assert (data->cancellable_handler_id == 0); @@ -1784,7 +1785,7 @@ send_message_with_reply_cleanup (GTask *task, gboolean remove) if (data->timeout_source != NULL) { g_source_destroy (data->timeout_source); - data->timeout_source = NULL; + g_clear_pointer (&data->timeout_source, g_source_unref); } if (data->cancellable_handler_id > 0) { @@ -1888,7 +1889,7 @@ send_message_with_reply_timeout_cb (gpointer user_data) send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_TIMED_OUT, _("Timeout was reached")); - return FALSE; + return G_SOURCE_REMOVE; } /* ---------------------------------------------------------------------------------------------------- */ @@ -1949,7 +1950,6 @@ g_dbus_connection_send_message_with_reply_unlocked (GDBusConnection *connect g_source_set_static_name (data->timeout_source, "[gio] send_message_with_reply_unlocked"); g_task_attach_source (task, data->timeout_source, (GSourceFunc) send_message_with_reply_timeout_cb); - g_source_unref (data->timeout_source); } g_hash_table_insert (connection->map_method_serial_to_task, |