[glib: 3/8] gdbusconnection: Tidy up destroy notification for signal subscriptions

[Philip Withnall <pwithnall src gnome org>]

commit bee27dd9f09c9b73d7682bec38c70aeea2588682
Author: Philip Withnall <withnall endlessm com>
Date:   Fri Jan 17 19:52:46 2020 +0000

    gdbusconnection: Tidy up destroy notification for signal subscriptions
    
    Tie the destruction of the `user_data` to the destruction of the
    `SignalSubscriber` struct. This is tidier, and ensures that the fields
    in `SignalSubscriber` are all immutable after being set, so the
    structure can safely be used across threads without locking.
    
    It doesn’t matter which thread we call `call_destroy_notify()` in, since
    it always defers calling `user_data_free_func` to the user-provided
    `GMainContext`.
    
    Signed-off-by: Philip Withnall <withnall endlessm com>
    
    Helps: #978

 gio/gdbusconnection.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)
---
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index f2cba14cd..5115c4767 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -3267,6 +3267,7 @@ signal_data_free (SignalData *signal_data)
 
 typedef struct
 {
+  /* All fields are immutable after construction. */
   gatomicrefcount ref_count;
   GDBusSignalCallback callback;
   gpointer user_data;
@@ -3287,6 +3288,14 @@ signal_subscriber_unref (SignalSubscriber *subscriber)
 {
   if (g_atomic_ref_count_dec (&subscriber->ref_count))
     {
+      /* Destroy the user data. It doesn’t matter which thread
+       * signal_subscriber_unref() is called in (or whether it’s called with a
+       * lock held), as call_destroy_notify() always defers to the next
+       * #GMainContext iteration. */
+      call_destroy_notify (subscriber->context,
+                           subscriber->user_data_free_func,
+                           subscriber->user_data);
+
       g_main_context_unref (subscriber->context);
       g_free (subscriber);
     }
@@ -3682,7 +3691,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
                                       guint            subscription_id)
 {
   GPtrArray *subscribers;
-  guint n;
 
   g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
   g_return_if_fail (check_initialized (connection));
@@ -3698,17 +3706,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
   /* invariant */
   g_assert (subscribers->len == 0 || subscribers->len == 1);
 
-  /* call GDestroyNotify without lock held */
-  for (n = 0; n < subscribers->len; n++)
-    {
-      SignalSubscriber *subscriber = subscribers->pdata[n];
-      call_destroy_notify (subscriber->context,
-                           subscriber->user_data_free_func,
-                           subscriber->user_data);
-      subscriber->user_data_free_func = NULL;
-      subscriber->user_data = NULL;
-    }
-
   g_ptr_array_unref (subscribers);
 }
 
@@ -3996,17 +3993,6 @@ purge_all_signal_subscriptions (GDBusConnection *connection)
     }
   g_array_free (ids, TRUE);
 
-  /* call GDestroyNotify without lock held */
-  for (n = 0; n < subscribers->len; n++)
-    {
-      SignalSubscriber *subscriber = subscribers->pdata[n];
-      call_destroy_notify (subscriber->context,
-                           subscriber->user_data_free_func,
-                           subscriber->user_data);
-      subscriber->user_data_free_func = NULL;
-      subscriber->user_data = NULL;
-    }
-
   g_ptr_array_unref (subscribers);
 }