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
This commit is contained in:
Philip Withnall 2020-01-17 19:38:55 +00:00
parent bee27dd9f0
commit 130455bbb2

View File

@ -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;