From 72afc793463a2168ec85fcd717eba942e536f4f5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 20 Feb 2020 11:09:45 +0000 Subject: [PATCH] gdbusproxy: Replace home-grown weak ref implementation with GWeakRef The fix for bgo#651133 (commit 7e0f890e38) introduced a kind of weak ref, which had to be thread-safe due to the fact that `GDBusProxy` operates in one thread but can emit signals in another. Since that commit, `GWeakRef` was added, which does the same thing. Drop the custom code in favour of it; this should be functionally equivalent, but using an RW lock rather than a basic mutex, which should reduce contention. Signed-off-by: Philip Withnall --- gio/gdbusproxy.c | 104 +++++++++++------------------------------------ 1 file changed, 23 insertions(+), 81 deletions(-) diff --git a/gio/gdbusproxy.c b/gio/gdbusproxy.c index 39eed1688..f3b02ccce 100644 --- a/gio/gdbusproxy.c +++ b/gio/gdbusproxy.c @@ -95,28 +95,19 @@ G_LOCK_DEFINE_STATIC (properties_lock); /* ---------------------------------------------------------------------------------------------------- */ -G_LOCK_DEFINE_STATIC (signal_subscription_lock); - -typedef struct +static GWeakRef * +weak_ref_new (GObject *object) { - volatile gint ref_count; - GDBusProxy *proxy; -} SignalSubscriptionData; - -static SignalSubscriptionData * -signal_subscription_ref (SignalSubscriptionData *data) -{ - g_atomic_int_inc (&data->ref_count); - return data; + GWeakRef *weak_ref = g_new0 (GWeakRef, 1); + g_weak_ref_init (weak_ref, object); + return g_steal_pointer (&weak_ref); } static void -signal_subscription_unref (SignalSubscriptionData *data) +weak_ref_free (GWeakRef *weak_ref) { - if (g_atomic_int_dec_and_test (&data->ref_count)) - { - g_slice_free (SignalSubscriptionData, data); - } + g_weak_ref_clear (weak_ref); + g_free (weak_ref); } /* ---------------------------------------------------------------------------------------------------- */ @@ -152,8 +143,6 @@ struct _GDBusProxyPrivate /* mutable, protected by properties_lock */ GDBusObject *object; - - SignalSubscriptionData *signal_subscription_data; }; enum @@ -189,22 +178,6 @@ G_DEFINE_TYPE_WITH_CODE (GDBusProxy, g_dbus_proxy, G_TYPE_OBJECT, G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, initable_iface_init) G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, async_initable_iface_init)) -static void -g_dbus_proxy_dispose (GObject *object) -{ - GDBusProxy *proxy = G_DBUS_PROXY (object); - G_LOCK (signal_subscription_lock); - if (proxy->priv->signal_subscription_data != NULL) - { - proxy->priv->signal_subscription_data->proxy = NULL; - signal_subscription_unref (proxy->priv->signal_subscription_data); - proxy->priv->signal_subscription_data = NULL; - } - G_UNLOCK (signal_subscription_lock); - - G_OBJECT_CLASS (g_dbus_proxy_parent_class)->dispose (object); -} - static void g_dbus_proxy_finalize (GObject *object) { @@ -346,7 +319,6 @@ g_dbus_proxy_class_init (GDBusProxyClass *klass) { GObjectClass *gobject_class = G_OBJECT_CLASS (klass); - gobject_class->dispose = g_dbus_proxy_dispose; gobject_class->finalize = g_dbus_proxy_finalize; gobject_class->set_property = g_dbus_proxy_set_property; gobject_class->get_property = g_dbus_proxy_get_property; @@ -638,9 +610,6 @@ static void g_dbus_proxy_init (GDBusProxy *proxy) { proxy->priv = g_dbus_proxy_get_instance_private (proxy); - proxy->priv->signal_subscription_data = g_slice_new0 (SignalSubscriptionData); - proxy->priv->signal_subscription_data->ref_count = 1; - proxy->priv->signal_subscription_data->proxy = proxy; proxy->priv->properties = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, @@ -868,21 +837,12 @@ on_signal_received (GDBusConnection *connection, GVariant *parameters, gpointer user_data) { - SignalSubscriptionData *data = user_data; + GWeakRef *proxy_weak = user_data; GDBusProxy *proxy; - G_LOCK (signal_subscription_lock); - proxy = data->proxy; + proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak)); if (proxy == NULL) - { - G_UNLOCK (signal_subscription_lock); - return; - } - else - { - g_object_ref (proxy); - G_UNLOCK (signal_subscription_lock); - } + return; if (!proxy->priv->initialized) goto out; @@ -1038,7 +998,7 @@ on_properties_changed (GDBusConnection *connection, GVariant *parameters, gpointer user_data) { - SignalSubscriptionData *data = user_data; + GWeakRef *proxy_weak = user_data; gboolean emit_g_signal = FALSE; GDBusProxy *proxy; const gchar *interface_name_for_signal; @@ -1052,18 +1012,9 @@ on_properties_changed (GDBusConnection *connection, changed_properties = NULL; invalidated_properties = NULL; - G_LOCK (signal_subscription_lock); - proxy = data->proxy; + proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak)); if (proxy == NULL) - { - G_UNLOCK (signal_subscription_lock); - goto out; - } - else - { - g_object_ref (proxy); - G_UNLOCK (signal_subscription_lock); - } + return; if (!proxy->priv->initialized) goto out; @@ -1289,23 +1240,14 @@ on_name_owner_changed (GDBusConnection *connection, GVariant *parameters, gpointer user_data) { - SignalSubscriptionData *data = user_data; + GWeakRef *proxy_weak = user_data; GDBusProxy *proxy; const gchar *old_owner; const gchar *new_owner; - G_LOCK (signal_subscription_lock); - proxy = data->proxy; + proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak)); if (proxy == NULL) - { - G_UNLOCK (signal_subscription_lock); - goto out; - } - else - { - g_object_ref (proxy); - G_UNLOCK (signal_subscription_lock); - } + return; /* if we are already trying to load properties, cancel that */ if (proxy->priv->get_all_cancellable != NULL) @@ -1762,8 +1704,8 @@ async_initable_init_first (GAsyncInitable *initable) proxy->priv->interface_name, G_DBUS_SIGNAL_FLAGS_NONE, on_properties_changed, - signal_subscription_ref (proxy->priv->signal_subscription_data), - (GDestroyNotify) signal_subscription_unref); + weak_ref_new (G_OBJECT (proxy)), + (GDestroyNotify) weak_ref_free); } if (!(proxy->priv->flags & G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS)) @@ -1778,8 +1720,8 @@ async_initable_init_first (GAsyncInitable *initable) NULL, /* arg0 */ G_DBUS_SIGNAL_FLAGS_NONE, on_signal_received, - signal_subscription_ref (proxy->priv->signal_subscription_data), - (GDestroyNotify) signal_subscription_unref); + weak_ref_new (G_OBJECT (proxy)), + (GDestroyNotify) weak_ref_free); } if (proxy->priv->name != NULL && @@ -1794,8 +1736,8 @@ async_initable_init_first (GAsyncInitable *initable) proxy->priv->name, /* arg0 */ G_DBUS_SIGNAL_FLAGS_NONE, on_name_owner_changed, - signal_subscription_ref (proxy->priv->signal_subscription_data), - (GDestroyNotify) signal_subscription_unref); + weak_ref_new (G_OBJECT (proxy)), + (GDestroyNotify) weak_ref_free); } }