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
This commit is contained in:
Philip Withnall 2020-01-17 19:52:46 +00:00
parent 9b1c8d7dd5
commit bee27dd9f0

View File

@ -3267,6 +3267,7 @@ signal_data_free (SignalData *signal_data)
typedef struct typedef struct
{ {
/* All fields are immutable after construction. */
gatomicrefcount ref_count; gatomicrefcount ref_count;
GDBusSignalCallback callback; GDBusSignalCallback callback;
gpointer user_data; gpointer user_data;
@ -3287,6 +3288,14 @@ signal_subscriber_unref (SignalSubscriber *subscriber)
{ {
if (g_atomic_ref_count_dec (&subscriber->ref_count)) if (g_atomic_ref_count_dec (&subscriber->ref_count))
{ {
/* Destroy the user data. It doesnt matter which thread
* signal_subscriber_unref() is called in (or whether its 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_main_context_unref (subscriber->context);
g_free (subscriber); g_free (subscriber);
} }
@ -3682,7 +3691,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
guint subscription_id) guint subscription_id)
{ {
GPtrArray *subscribers; GPtrArray *subscribers;
guint n;
g_return_if_fail (G_IS_DBUS_CONNECTION (connection)); g_return_if_fail (G_IS_DBUS_CONNECTION (connection));
g_return_if_fail (check_initialized (connection)); g_return_if_fail (check_initialized (connection));
@ -3698,17 +3706,6 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
/* invariant */ /* invariant */
g_assert (subscribers->len == 0 || subscribers->len == 1); 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); g_ptr_array_unref (subscribers);
} }
@ -3996,17 +3993,6 @@ purge_all_signal_subscriptions (GDBusConnection *connection)
} }
g_array_free (ids, TRUE); 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); g_ptr_array_unref (subscribers);
} }