From d31f0777d12d22e1d27ba2bb544a9040ce51940f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 14 Mar 2024 19:51:59 +0000 Subject: [PATCH] gdbusconnection: Factor out remove_signal_data_if_unused No functional change, just removing some nesting. The check for whether signal_data->subscribers is empty changes from a conditional that tests whether it is into an early-return if it isn't. A subsequent commit will add additional conditions that make us consider a SignalData to be still in use and therefore not eligible to be removed. Signed-off-by: Simon McVittie --- gio/gdbusconnection.c | 83 +++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index c3d6e6d06..0e5cba3ea 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -3661,6 +3661,52 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection, /* ---------------------------------------------------------------------------------------------------- */ +/* + * Called in any thread. + * Must hold the connection lock when calling this, unless + * connection->finalizing is TRUE. + * May free signal_data, so do not dereference it after this. + */ +static void +remove_signal_data_if_unused (GDBusConnection *connection, + SignalData *signal_data) +{ + GPtrArray *signal_data_array; + + if (signal_data->subscribers->len != 0) + return; + + g_warn_if_fail (g_hash_table_remove (connection->map_rule_to_signal_data, signal_data->rule)); + + signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, + signal_data->sender_unique_name); + g_warn_if_fail (signal_data_array != NULL); + g_warn_if_fail (g_ptr_array_remove (signal_data_array, signal_data)); + + if (signal_data_array->len == 0) + { + g_warn_if_fail (g_hash_table_remove (connection->map_sender_unique_name_to_signal_data_array, + signal_data->sender_unique_name)); + } + + /* remove the match rule from the bus unless NameLost or NameAcquired (see subscribe()) */ + if ((connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) && + !is_signal_data_for_name_lost_or_acquired (signal_data) && + !g_dbus_connection_is_closed (connection) && + !connection->finalizing) + { + /* The check for g_dbus_connection_is_closed() means that + * sending the RemoveMatch message can't fail with + * G_IO_ERROR_CLOSED, because we're holding the lock, + * so on_worker_closed() can't happen between the check we just + * did, and releasing the lock later. + */ + remove_match_rule (connection, signal_data->rule); + } + + signal_data_free (signal_data); +} + /* called in any thread */ /* must hold lock when calling this (except if connection->finalizing is TRUE) * returns the number of removed subscribers */ @@ -3669,7 +3715,6 @@ unsubscribe_id_internal (GDBusConnection *connection, guint subscription_id) { SignalData *signal_data; - GPtrArray *signal_data_array; guint n; guint n_removed = 0; @@ -3696,40 +3741,8 @@ unsubscribe_id_internal (GDBusConnection *connection, GUINT_TO_POINTER (subscription_id))); n_removed++; g_ptr_array_remove_index_fast (signal_data->subscribers, n); - - if (signal_data->subscribers->len == 0) - { - g_warn_if_fail (g_hash_table_remove (connection->map_rule_to_signal_data, signal_data->rule)); - - signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, - signal_data->sender_unique_name); - g_warn_if_fail (signal_data_array != NULL); - g_warn_if_fail (g_ptr_array_remove (signal_data_array, signal_data)); - - if (signal_data_array->len == 0) - { - g_warn_if_fail (g_hash_table_remove (connection->map_sender_unique_name_to_signal_data_array, - signal_data->sender_unique_name)); - } - - /* remove the match rule from the bus unless NameLost or NameAcquired (see subscribe()) */ - if ((connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION) && - !is_signal_data_for_name_lost_or_acquired (signal_data) && - !g_dbus_connection_is_closed (connection) && - !connection->finalizing) - { - /* The check for g_dbus_connection_is_closed() means that - * sending the RemoveMatch message can't fail with - * G_IO_ERROR_CLOSED, because we're holding the lock, - * so on_worker_closed() can't happen between the check we just - * did, and releasing the lock later. - */ - remove_match_rule (connection, signal_data->rule); - } - - signal_data_free (signal_data); - } - + /* May free signal_data */ + remove_signal_data_if_unused (connection, signal_data); goto out; }