[glib: 4/8] gdbusconnection: Fix race when emitting D-Bus signal callbacks

[Philip Withnall <pwithnall src gnome org>]

commit 130455bbb2e94e29615c461eace229b034a07f0e
Author: Philip Withnall <withnall endlessm com>
Date:   Fri Jan 17 19:38:55 2020 +0000

    gdbusconnection: Fix race when emitting D-Bus signal callbacks
    
    Instead of storing a copy of the `callback` and `user_data` from a
    `SignalSubscriber` in a `SignalInstance` struct (which is the closure
    for signal callback data as it’s sent from the D-Bus worker thread to
    the thread which originally subscribed to a signal), store a strong
    reference to the `SignalSubscriber` struct itself.
    
    This keeps the `SignalSubscriber` alive until the emission is
    complete, which ensures that the `user_data` is not freed prematurely.
    It also slightly reduces the allocation size of `SignalInstance` (not
    that it matters).
    
    This is threadsafe because the fields in `SignalSubscriber` are all
    immutable after construction.
    
    Signed-off-by: Philip Withnall <withnall endlessm com>
    
    Helps: #978

 gio/gdbusconnection.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)
---
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 5115c4767..11f67cd43 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -3713,9 +3713,7 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
 
 typedef struct
 {
-  guint                subscription_id;
-  GDBusSignalCallback  callback;
-  gpointer             user_data;
+  SignalSubscriber    *subscriber;  /* (owned) */
   GDBusMessage        *message;
   GDBusConnection     *connection;
   const gchar         *sender;
@@ -3747,7 +3745,7 @@ emit_signal_instance_in_idle_cb (gpointer data)
 
 #if 0
   g_print ("in emit_signal_instance_in_idle_cb (id=%d sender=%s path=%s interface=%s member=%s params=%s)\n",
-           signal_instance->subscription_id,
+           signal_instance->subscriber->id,
            signal_instance->sender,
            signal_instance->path,
            signal_instance->interface,
@@ -3759,18 +3757,18 @@ emit_signal_instance_in_idle_cb (gpointer data)
   CONNECTION_LOCK (signal_instance->connection);
   has_subscription = FALSE;
   if (g_hash_table_lookup (signal_instance->connection->map_id_to_signal_data,
-                           GUINT_TO_POINTER (signal_instance->subscription_id)) != NULL)
+                           GUINT_TO_POINTER (signal_instance->subscriber->id)) != NULL)
     has_subscription = TRUE;
   CONNECTION_UNLOCK (signal_instance->connection);
 
   if (has_subscription)
-    signal_instance->callback (signal_instance->connection,
-                               signal_instance->sender,
-                               signal_instance->path,
-                               signal_instance->interface,
-                               signal_instance->member,
-                               parameters,
-                               signal_instance->user_data);
+    signal_instance->subscriber->callback (signal_instance->connection,
+                                           signal_instance->sender,
+                                           signal_instance->path,
+                                           signal_instance->interface,
+                                           signal_instance->member,
+                                           parameters,
+                                           signal_instance->subscriber->user_data);
 
   g_variant_unref (parameters);
 
@@ -3782,6 +3780,7 @@ signal_instance_free (SignalInstance *signal_instance)
 {
   g_object_unref (signal_instance->message);
   g_object_unref (signal_instance->connection);
+  signal_subscriber_unref (signal_instance->subscriber);
   g_free (signal_instance);
 }
 
@@ -3901,9 +3900,7 @@ schedule_callbacks (GDBusConnection *connection,
           SignalInstance *signal_instance;
 
           signal_instance = g_new0 (SignalInstance, 1);
-          signal_instance->subscription_id = subscriber->id;
-          signal_instance->callback = subscriber->callback;
-          signal_instance->user_data = subscriber->user_data;
+          signal_instance->subscriber = signal_subscriber_ref (subscriber);
           signal_instance->message = g_object_ref (message);
           signal_instance->connection = g_object_ref (connection);
           signal_instance->sender = sender;