[gnome-software: 12/14] gs-plugin: Hold weak references to the plugin in idle callbacks

[Philip Withnall <pwithnall src gnome org>]

commit c86f67a1da3feb01aa7b1b3f4ba0230deb0f52df
Author: Philip Withnall <pwithnall endlessos org>
Date:   Tue Mar 1 15:14:21 2022 +0000

    gs-plugin: Hold weak references to the plugin in idle callbacks
    
    Various signal emissions from `GsPlugin` are (correctly, I think)
    deferred to the global default main context via idle callbacks.
    
    If the plugin was finalised between one of these callbacks being
    attached to the main context, and it eventually being invoked, a
    use-after-free error would occur.
    
    Avoid that by holding a weak reference to the `GsPlugin` for each idle
    callback, and exiting early from the callback if that reference can’t be
    converted to a strong one.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Helps: #1661

 lib/gs-plugin.c | 75 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 18 deletions(-)
---
diff --git a/lib/gs-plugin.c b/lib/gs-plugin.c
index 0ff2316d3..e1e3c7288 100644
--- a/lib/gs-plugin.c
+++ b/lib/gs-plugin.c
@@ -702,7 +702,7 @@ gs_plugin_check_distro_id (GsPlugin *plugin, const gchar *distro_id)
 }
 
 typedef struct {
-       GsPlugin        *plugin;  /* (unowned) */
+       GWeakRef         plugin_weak;  /* (element-type GsPlugin) */
        GsApp           *app;  /* (owned) */
        GsPluginStatus   status;
        guint            percentage;
@@ -711,6 +711,7 @@ typedef struct {
 static void
 gs_plugin_status_helper_free (GsPluginStatusHelper *helper)
 {
+       g_weak_ref_clear (&helper->plugin_weak);
        g_clear_object (&helper->app);
        g_slice_free (GsPluginStatusHelper, helper);
 }
@@ -721,10 +722,16 @@ static gboolean
 gs_plugin_status_update_cb (gpointer user_data)
 {
        GsPluginStatusHelper *helper = (GsPluginStatusHelper *) user_data;
-       g_signal_emit (helper->plugin,
-                      signals[SIGNAL_STATUS_CHANGED], 0,
-                      helper->app,
-                      helper->status);
+       g_autoptr(GsPlugin) plugin = NULL;
+
+       /* Does the plugin still exist? */
+       plugin = g_weak_ref_get (&helper->plugin_weak);
+
+       if (plugin != NULL)
+               g_signal_emit (plugin,
+                              signals[SIGNAL_STATUS_CHANGED], 0,
+                              helper->app,
+                              helper->status);
 
        return G_SOURCE_REMOVE;
 }
@@ -746,7 +753,7 @@ gs_plugin_status_update (GsPlugin *plugin, GsApp *app, GsPluginStatus status)
        g_autoptr(GSource) idle_source = NULL;
 
        helper = g_slice_new0 (GsPluginStatusHelper);
-       helper->plugin = plugin;
+       g_weak_ref_init (&helper->plugin_weak, plugin);
        helper->status = status;
        if (app != NULL)
                helper->app = g_object_ref (app);
@@ -875,11 +882,33 @@ gs_plugin_app_launch (GsPlugin *plugin, GsApp *app, GError **error)
        return TRUE;
 }
 
+static void
+weak_ref_free (GWeakRef *weak)
+{
+       g_weak_ref_clear (weak);
+       g_free (weak);
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (GWeakRef, weak_ref_free)
+
+/* @obj is a gpointer rather than a GObject* to avoid the need for casts */
+static GWeakRef *
+weak_ref_new (gpointer obj)
+{
+       g_autoptr(GWeakRef) weak = g_new0 (GWeakRef, 1);
+       g_weak_ref_init (weak, obj);
+       return g_steal_pointer (&weak);
+}
+
 static gboolean
 gs_plugin_updates_changed_cb (gpointer user_data)
 {
-       GsPlugin *plugin = GS_PLUGIN (user_data);
-       g_signal_emit (plugin, signals[SIGNAL_UPDATES_CHANGED], 0);
+       GWeakRef *plugin_weak = user_data;
+       g_autoptr(GsPlugin) plugin = NULL;
+
+       plugin = g_weak_ref_get (plugin_weak);
+       if (plugin != NULL)
+               g_signal_emit (plugin, signals[SIGNAL_UPDATES_CHANGED], 0);
 
        return G_SOURCE_REMOVE;
 }
@@ -896,14 +925,19 @@ gs_plugin_updates_changed_cb (gpointer user_data)
 void
 gs_plugin_updates_changed (GsPlugin *plugin)
 {
-       g_idle_add (gs_plugin_updates_changed_cb, plugin);
+       g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, gs_plugin_updates_changed_cb,
+                        weak_ref_new (plugin), (GDestroyNotify) weak_ref_free);
 }
 
 static gboolean
 gs_plugin_reload_cb (gpointer user_data)
 {
-       GsPlugin *plugin = GS_PLUGIN (user_data);
-       g_signal_emit (plugin, signals[SIGNAL_RELOAD], 0);
+       GWeakRef *plugin_weak = user_data;
+       g_autoptr(GsPlugin) plugin = NULL;
+
+       plugin = g_weak_ref_get (plugin_weak);
+       if (plugin != NULL)
+               g_signal_emit (plugin, signals[SIGNAL_RELOAD], 0);
 
        return G_SOURCE_REMOVE;
 }
@@ -924,7 +958,8 @@ void
 gs_plugin_reload (GsPlugin *plugin)
 {
        g_debug ("emitting ::reload in idle");
-       g_idle_add (gs_plugin_reload_cb, plugin);
+       g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, gs_plugin_reload_cb,
+                        weak_ref_new (plugin), (GDestroyNotify) weak_ref_free);
 }
 
 typedef struct {
@@ -1823,7 +1858,7 @@ gs_plugin_new (void)
 }
 
 typedef struct {
-       GsPlugin *plugin;  /* (owned) */
+       GWeakRef  plugin_weak;  /* (owned) (element-type GsPlugin) */
        GsApp    *repository;  /* (owned) */
 } GsPluginRepositoryChangedHelper;
 
@@ -1831,7 +1866,7 @@ static void
 gs_plugin_repository_changed_helper_free (GsPluginRepositoryChangedHelper *helper)
 {
        g_clear_object (&helper->repository);
-       g_clear_object (&helper->plugin);
+       g_weak_ref_clear (&helper->plugin_weak);
        g_slice_free (GsPluginRepositoryChangedHelper, helper);
 }
 
@@ -1841,9 +1876,13 @@ static gboolean
 gs_plugin_repository_changed_cb (gpointer user_data)
 {
        GsPluginRepositoryChangedHelper *helper = user_data;
-       g_signal_emit (helper->plugin,
-                      signals[SIGNAL_REPOSITORY_CHANGED], 0,
-                      helper->repository);
+       g_autoptr(GsPlugin) plugin = NULL;
+
+       plugin = g_weak_ref_get (&helper->plugin_weak);
+       if (plugin != NULL)
+               g_signal_emit (plugin,
+                              signals[SIGNAL_REPOSITORY_CHANGED], 0,
+                              helper->repository);
 
        return G_SOURCE_REMOVE;
 }
@@ -1868,7 +1907,7 @@ gs_plugin_repository_changed (GsPlugin *plugin,
        g_return_if_fail (GS_IS_APP (repository));
 
        helper = g_slice_new0 (GsPluginRepositoryChangedHelper);
-       helper->plugin = g_object_ref (plugin);
+       g_weak_ref_init (&helper->plugin_weak, plugin);
        helper->repository = g_object_ref (repository);
 
        idle_source = g_idle_source_new ();