[gnome-software/1792-crash-under-gs_flatpak_refine_app_unlocked: 18/18] flatpak: Make sure the XbSilo is non-NULL when passing it to other functions

[Milan Crha <mcrha src gnome org>]

commit 10ee30331103bdd9c3bf3100b44c0296926e36a4
Author: Milan Crha <mcrha redhat com>
Date:   Tue Jun 7 12:06:59 2022 +0200

    flatpak: Make sure the XbSilo is non-NULL when passing it to other functions
    
    It can happen the GsFlatpak::XbSilo is NULL even after refreshing
    the GsFlatpak, thus make sure the functions properly end with an error,
    rather than expecting the XbSilo is always non-NULL.
    
    Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1792

 plugins/flatpak/gs-flatpak.c | 73 +++++++++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 25 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 7880071e9..c7a09813d 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -1131,6 +1131,41 @@ gs_flatpak_rescan_app_data (GsFlatpak *self,
        return TRUE;
 }
 
+/* Returns with a read lock held on @self->silo_lock on success.
+   The *locker should be NULL when being called. */
+static gboolean
+ensure_flatpak_silo_with_locker (GsFlatpak            *self,
+                                GRWLockReaderLocker **locker,
+                                gboolean              interactive,
+                                GCancellable         *cancellable,
+                                GError              **error)
+{
+       /* should not hold the lock when called */
+       g_return_val_if_fail (*locker == NULL, FALSE);
+
+       /* ensure valid */
+       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+               return FALSE;
+
+       *locker = g_rw_lock_reader_locker_new (&self->silo_lock);
+
+       while (self->silo == NULL) {
+               g_clear_pointer (locker, g_rw_lock_reader_locker_free);
+
+               if (!gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error)) {
+                       gs_flatpak_internal_data_changed (self);
+                       return FALSE;
+               }
+
+               /* At this point either rescan_appstream_store() returned an error or it successfully
+                * initialised self->silo. There is the possibility that another thread will invalidate
+                * the silo before we regain the lock. If so, we’ll have to rescan again. */
+               *locker = g_rw_lock_reader_locker_new (&self->silo_lock);
+       }
+
+       return TRUE;
+}
+
 gboolean
 gs_flatpak_setup (GsFlatpak *self, GCancellable *cancellable, GError **error)
 {
@@ -3197,7 +3232,8 @@ gs_flatpak_refine_app_unlocked (GsFlatpak *self,
        if (gs_app_get_bundle_kind (app) != AS_BUNDLE_KIND_FLATPAK)
                return TRUE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
+               return FALSE;
 
        /* always do AppStream properties */
        if (!gs_flatpak_refine_appstream (self, app, self->silo, flags, interactive, cancellable, error))
@@ -3392,12 +3428,9 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app,
        if (id == NULL)
                return TRUE;
 
-       /* ensure valid */
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
-
        /* find all apps when matching any prefixes */
        xpath = g_strdup_printf ("components/component/id[text()='%s']/..", id);
        components = xb_silo_query (self->silo, xpath, 0, &error_local);
@@ -3946,10 +3979,9 @@ gs_flatpak_search (GsFlatpak *self,
        GHashTableIter iter;
        gpointer key, value;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_search (self->plugin, self->silo, values, list_tmp,
                                  cancellable, error))
                return FALSE;
@@ -4020,10 +4052,9 @@ gs_flatpak_search_developer_apps (GsFlatpak *self,
        GHashTableIter iter;
        gpointer key, value;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_search_developer_apps (self->plugin, self->silo, values, list_tmp,
                                                 cancellable, error))
                return FALSE;
@@ -4089,10 +4120,9 @@ gs_flatpak_add_category_apps (GsFlatpak *self,
 {
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        return gs_appstream_add_category_apps (self->plugin, self->silo,
                                               category, list,
                                               cancellable, error);
@@ -4107,10 +4137,9 @@ gs_flatpak_add_categories (GsFlatpak *self,
 {
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        return gs_appstream_add_categories (self->silo,
                                            list, cancellable, error);
 }
@@ -4125,10 +4154,9 @@ gs_flatpak_add_popular (GsFlatpak *self,
        g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_add_popular (self->silo, list_tmp,
                                       cancellable, error))
                return FALSE;
@@ -4148,10 +4176,9 @@ gs_flatpak_add_featured (GsFlatpak *self,
        g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_add_featured (self->silo, list_tmp,
                                        cancellable, error))
                return FALSE;
@@ -4171,10 +4198,9 @@ gs_flatpak_add_deployment_featured (GsFlatpak *self,
 {
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        return gs_appstream_add_deployment_featured (self->silo, deployments, list, cancellable, error);
 }
 
@@ -4189,10 +4215,9 @@ gs_flatpak_add_alternates (GsFlatpak *self,
        g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_add_alternates (self->silo, app, list_tmp,
                                          cancellable, error))
                return FALSE;
@@ -4213,10 +4238,9 @@ gs_flatpak_add_recent (GsFlatpak *self,
        g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_add_recent (self->plugin, self->silo, list_tmp, age,
                                      cancellable, error))
                return FALSE;
@@ -4238,10 +4262,9 @@ gs_flatpak_url_to_app (GsFlatpak *self,
        g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
        g_autoptr(GRWLockReaderLocker) locker = NULL;
 
-       if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error))
+       if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error))
                return FALSE;
 
-       locker = g_rw_lock_reader_locker_new (&self->silo_lock);
        if (!gs_appstream_url_to_app (self->plugin, self->silo, list_tmp, url, cancellable, error))
                return FALSE;