From a13bbdc1dc0cd4ec16c3e75d0cc1c07eb791b3bc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 17 Jan 2020 19:56:58 +0000 Subject: [PATCH] gdbusconnection: Tidy up unsubscription code This just removes a now-redundant intermediate array. This means that the `SignalSubscriber` instances are now potentially freed a little sooner, inside the locked segment, but they are already careful to only call their `user_data_free_func` in the right thread. So that should not deadlock. Signed-off-by: Philip Withnall Helps: #978 --- gio/gdbusconnection.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 4cae53b29..4d99f8570 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -3576,15 +3576,16 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, /* ---------------------------------------------------------------------------------------------------- */ /* called in any thread */ -/* must hold lock when calling this (except if connection->finalizing is TRUE) */ -static void +/* must hold lock when calling this (except if connection->finalizing is TRUE) + * returns the number of removed subscribers */ +static guint unsubscribe_id_internal (GDBusConnection *connection, - guint subscription_id, - GPtrArray *out_removed_subscribers) + guint subscription_id) { SignalData *signal_data; GPtrArray *signal_data_array; guint n; + guint n_removed = 0; signal_data = g_hash_table_lookup (connection->map_id_to_signal_data, GUINT_TO_POINTER (subscription_id)); @@ -3607,7 +3608,7 @@ unsubscribe_id_internal (GDBusConnection *connection, * guaranteed to be unique. */ g_warn_if_fail (g_hash_table_remove (connection->map_id_to_signal_data, GUINT_TO_POINTER (subscription_id))); - g_ptr_array_add (out_removed_subscribers, signal_subscriber_ref (subscriber)); + n_removed++; g_ptr_array_remove_index_fast (signal_data->subscribers, n); if (signal_data->subscribers->len == 0) @@ -3649,7 +3650,7 @@ unsubscribe_id_internal (GDBusConnection *connection, g_assert_not_reached (); out: - ; + return n_removed; } /** @@ -3666,23 +3667,17 @@ void g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, guint subscription_id) { - GPtrArray *subscribers; + guint n_subscribers_removed G_GNUC_UNUSED /* when compiling with G_DISABLE_ASSERT */; g_return_if_fail (G_IS_DBUS_CONNECTION (connection)); g_return_if_fail (check_initialized (connection)); - subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); - CONNECTION_LOCK (connection); - unsubscribe_id_internal (connection, - subscription_id, - subscribers); + n_subscribers_removed = unsubscribe_id_internal (connection, subscription_id); CONNECTION_UNLOCK (connection); /* invariant */ - g_assert (subscribers->len == 0 || subscribers->len == 1); - - g_ptr_array_unref (subscribers); + g_assert (n_subscribers_removed == 0 || n_subscribers_removed == 1); } /* ---------------------------------------------------------------------------------------------------- */ @@ -3945,7 +3940,6 @@ purge_all_signal_subscriptions (GDBusConnection *connection) GHashTableIter iter; gpointer key; GArray *ids; - GPtrArray *subscribers; guint n; ids = g_array_new (FALSE, FALSE, sizeof (guint)); @@ -3956,17 +3950,12 @@ purge_all_signal_subscriptions (GDBusConnection *connection) g_array_append_val (ids, subscription_id); } - subscribers = g_ptr_array_new_with_free_func ((GDestroyNotify) signal_subscriber_unref); for (n = 0; n < ids->len; n++) { guint subscription_id = g_array_index (ids, guint, n); - unsubscribe_id_internal (connection, - subscription_id, - subscribers); + unsubscribe_id_internal (connection, subscription_id); } g_array_free (ids, TRUE); - - g_ptr_array_unref (subscribers); } /* ---------------------------------------------------------------------------------------------------- */