diff options
author | Philip Withnall <withnall@endlessm.com> | 2017-02-06 09:41:10 +0100 |
---|---|---|
committer | Philip Withnall <withnall@endlessm.com> | 2017-06-01 11:39:40 +0100 |
commit | 4dd1b17c2487470831f03d7ee52e3cc1a0c9e0bd (patch) | |
tree | 2c6e95109fd45fde2322dc313d1ca2074f4ba4c8 /gio/gdbusnamewatching.c | |
parent | 97068f363efd40893ff902201390b0efe92d3293 (diff) | |
download | glib-4dd1b17c2487470831f03d7ee52e3cc1a0c9e0bd.tar.gz |
gdbus: Fix race in name watching on connection teardown
If g_dbus_unwatch_name() is called from one thread at the same time as
the GDBusConnection is emitting ::disconnected in another thread, there
will be a race and the handler for ::disconnected may end up using
memory after it’s freed.
Fix this by serialising through the map_id_to_client, so that
on_connection_disconnected() atomically gets a strong reference to the
Client, or NULL.
https://bugzilla.gnome.org/show_bug.cgi?id=777307
Diffstat (limited to 'gio/gdbusnamewatching.c')
-rw-r--r-- | gio/gdbusnamewatching.c | 48 |
1 files changed, 43 insertions, 5 deletions
diff --git a/gio/gdbusnamewatching.c b/gio/gdbusnamewatching.c index a295587fa..9fb6dd1ca 100644 --- a/gio/gdbusnamewatching.c +++ b/gio/gdbusnamewatching.c @@ -249,13 +249,43 @@ call_vanished_handler (Client *client, /* ---------------------------------------------------------------------------------------------------- */ +/* Return a reference to the #Client for @watcher_id, or %NULL if it’s been + * unwatched. This is safe to call from any thread. */ +static Client * +dup_client (guint watcher_id) +{ + Client *client; + + G_LOCK (lock); + + g_assert (watcher_id != 0); + g_assert (map_id_to_client != NULL); + + client = g_hash_table_lookup (map_id_to_client, GUINT_TO_POINTER (watcher_id)); + + if (client != NULL) + client_ref (client); + + G_UNLOCK (lock); + + return client; +} + +/* Could be called from any thread, so it could be called after client_unref() + * has started finalising the #Client. Avoid that by looking up the #Client + * atomically. */ static void on_connection_disconnected (GDBusConnection *connection, gboolean remote_peer_vanished, GError *error, gpointer user_data) { - Client *client = user_data; + guint watcher_id = GPOINTER_TO_UINT (user_data); + Client *client = NULL; + + client = dup_client (watcher_id); + if (client == NULL) + return; if (client->name_owner_changed_subscription_id > 0) g_dbus_connection_signal_unsubscribe (client->connection, client->name_owner_changed_subscription_id); @@ -267,10 +297,13 @@ on_connection_disconnected (GDBusConnection *connection, client->connection = NULL; call_vanished_handler (client, FALSE); + + client_unref (client); } /* ---------------------------------------------------------------------------------------------------- */ +/* Will always be called from the thread which acquired client->main_context. */ static void on_name_owner_changed (GDBusConnection *connection, const gchar *sender_name, @@ -280,11 +313,16 @@ on_name_owner_changed (GDBusConnection *connection, GVariant *parameters, gpointer user_data) { - Client *client = user_data; + guint watcher_id = GPOINTER_TO_UINT (user_data); + Client *client = NULL; const gchar *name; const gchar *old_owner; const gchar *new_owner; + client = dup_client (watcher_id); + if (client == NULL) + return; + if (!client->initialized) goto out; @@ -319,7 +357,7 @@ on_name_owner_changed (GDBusConnection *connection, } out: - ; + client_unref (client); } /* ---------------------------------------------------------------------------------------------------- */ @@ -444,7 +482,7 @@ has_connection (Client *client) client->disconnected_signal_handler_id = g_signal_connect (client->connection, "closed", G_CALLBACK (on_connection_disconnected), - client); + GUINT_TO_POINTER (client->id)); /* start listening to NameOwnerChanged messages immediately */ client->name_owner_changed_subscription_id = g_dbus_connection_signal_subscribe (client->connection, @@ -455,7 +493,7 @@ has_connection (Client *client) client->name, G_DBUS_SIGNAL_FLAGS_NONE, on_name_owner_changed, - client, + GUINT_TO_POINTER (client->id), NULL); if (client->flags & G_BUS_NAME_WATCHER_FLAGS_AUTO_START) |