[gnome-software/wip/kalev/appstream-silo-threadsafety: 4/6] appstream: Add locking for XbSilo

[Kalev Lember <klember src gnome org>]

commit a18fd72abf73df70a7c1611fe3177dd26a7435a8
Author: Kalev Lember <klember redhat com>
Date:   Fri May 10 12:25:45 2019 +0200

    appstream: Add locking for XbSilo
    
    We clear priv->silo and rebuild it in the gnome-software code, which
    means the locking has to be done here and we can't fix it inside XbSilo.
    
    This uses GRWLock so that we only take the writer lock when rebuilding
    silo, and use the reader lock when querying it.

 plugins/core/gs-plugin-appstream.c | 55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
---
diff --git a/plugins/core/gs-plugin-appstream.c b/plugins/core/gs-plugin-appstream.c
index dca85091..e19db57e 100644
--- a/plugins/core/gs-plugin-appstream.c
+++ b/plugins/core/gs-plugin-appstream.c
@@ -27,6 +27,7 @@
 
 struct GsPluginData {
        XbSilo                  *silo;
+       GRWLock                  silo_lock;
        GSettings               *settings;
 };
 
@@ -35,6 +36,10 @@ gs_plugin_initialize (GsPlugin *plugin)
 {
        GsPluginData *priv = gs_plugin_alloc_data (plugin, sizeof(GsPluginData));
 
+       /* XbSilo needs external locking as we destroy the silo and build a new
+        * one when something changes */
+       g_rw_lock_init (&priv->silo_lock);
+
        /* need package name */
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_RUN_AFTER, "dpkg");
 
@@ -48,6 +53,7 @@ gs_plugin_destroy (GsPlugin *plugin)
        GsPluginData *priv = gs_plugin_get_data (plugin);
        g_object_unref (priv->silo);
        g_object_unref (priv->settings);
+       g_rw_lock_clear (&priv->silo_lock);
 }
 
 static gboolean
@@ -422,9 +428,12 @@ gs_plugin_appstream_check_silo (GsPlugin *plugin,
        g_autoptr(XbBuilder) builder = xb_builder_new ();
        g_autoptr(XbNode) n = NULL;
        g_autoptr(GFile) file = NULL;
+       g_autoptr(GRWLockWriterLocker) locker = NULL;
        g_autoptr(GPtrArray) parent_appdata = g_ptr_array_new_with_free_func (g_free);
        g_autoptr(GPtrArray) parent_appstream = g_ptr_array_new_with_free_func (g_free);
 
+       locker = g_rw_lock_writer_locker_new (&priv->silo_lock);
+
        /* everything is okay */
        if (priv->silo != NULL && xb_silo_is_valid (priv->silo))
                return TRUE;
@@ -571,6 +580,7 @@ gs_plugin_url_to_app (GsPlugin *plugin,
        g_autofree gchar *path = NULL;
        g_autofree gchar *scheme = NULL;
        g_autofree gchar *xpath = NULL;
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
        g_autoptr(GsApp) app = NULL;
        g_autoptr(XbNode) component = NULL;
 
@@ -583,6 +593,8 @@ gs_plugin_url_to_app (GsPlugin *plugin,
        if (g_strcmp0 (scheme, "appstream") != 0)
                return TRUE;
 
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
        /* create app */
        path = gs_utils_get_url_path (url);
        xpath = g_strdup_printf ("components/component/id[text()='%s']", path);
@@ -648,8 +660,11 @@ gs_plugin_appstream_refine_state (GsPlugin *plugin, GsApp *app, GError **error)
        GsPluginData *priv = gs_plugin_get_data (plugin);
        g_autofree gchar *xpath = NULL;
        g_autoptr(GError) error_local = NULL;
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
        g_autoptr(XbNode) component = NULL;
 
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
        xpath = g_strdup_printf ("component/id[text()='%s']", gs_app_get_id (app));
        component = xb_silo_query_first (priv->silo, xpath, &error_local);
        if (component == NULL) {
@@ -674,6 +689,7 @@ gs_plugin_refine_from_id (GsPlugin *plugin,
        GsPluginData *priv = gs_plugin_get_data (plugin);
        const gchar *id;
        g_autoptr(GError) error_local = NULL;
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
        g_autoptr(GString) xpath = g_string_new (NULL);
        g_autoptr(GPtrArray) components = NULL;
 
@@ -682,6 +698,8 @@ gs_plugin_refine_from_id (GsPlugin *plugin,
        if (id == NULL)
                return TRUE;
 
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
        /* look in AppStream then fall back to AppData */
        xb_string_append_union (xpath, "components/component/id[text()='%s']/../pkgname/..", id);
        xb_string_append_union (xpath, "component/id[text()='%s']/../pkgname/..", id);
@@ -731,8 +749,11 @@ gs_plugin_refine_from_pkgname (GsPlugin *plugin,
        for (guint j = 0; j < sources->len; j++) {
                const gchar *pkgname = g_ptr_array_index (sources, j);
                g_autofree gchar *xpath = NULL;
+               g_autoptr(GRWLockReaderLocker) locker = NULL;
                g_autoptr(GPtrArray) components = NULL;
 
+               locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
                xpath = g_strdup_printf ("components/component/pkgname[text()='%s']/..",
                                         pkgname);
                components = xb_silo_query (priv->silo, xpath, 0, &error_local);
@@ -799,6 +820,7 @@ gs_plugin_refine_wildcard (GsPlugin *plugin,
        const gchar *id;
        g_autofree gchar *xpath = NULL;
        g_autoptr(GError) error_local = NULL;
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
        g_autoptr(GPtrArray) components = NULL;
 
        /* check silo is valid */
@@ -810,6 +832,8 @@ gs_plugin_refine_wildcard (GsPlugin *plugin,
        if (id == NULL)
                return TRUE;
 
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
        /* find all app with package names when matching any prefixes */
        xpath = g_strdup_printf ("components/component/id[text()='%s']/../pkgname/..", id);
        components = xb_silo_query (priv->silo, xpath, 0, &error_local);
@@ -849,8 +873,12 @@ gs_plugin_add_category_apps (GsPlugin *plugin,
                             GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_category_apps (plugin,
                                               priv->silo,
                                               category,
@@ -867,8 +895,12 @@ gs_plugin_add_search (GsPlugin *plugin,
                      GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_search (plugin,
                                    priv->silo,
                                    values,
@@ -884,12 +916,15 @@ gs_plugin_add_installed (GsPlugin *plugin,
                         GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
        g_autoptr(GPtrArray) components = NULL;
 
        /* check silo is valid */
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
 
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
+
        /* get all installed appdata files (notice no 'components/' prefix...) */
        components = xb_silo_query (priv->silo, "component", 0, NULL);
        if (components == NULL)
@@ -913,8 +948,12 @@ gs_plugin_add_categories (GsPlugin *plugin,
                          GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_categories (plugin, priv->silo, list,
                                            cancellable, error);
 }
@@ -926,8 +965,12 @@ gs_plugin_add_popular (GsPlugin *plugin,
                       GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_popular (plugin, priv->silo, list, cancellable, error);
 }
 
@@ -938,8 +981,12 @@ gs_plugin_add_featured (GsPlugin *plugin,
                        GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_featured (plugin, priv->silo, list, cancellable, error);
 }
 
@@ -951,8 +998,12 @@ gs_plugin_add_recent (GsPlugin *plugin,
                      GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_recent (plugin, priv->silo, list, age,
                                        cancellable, error);
 }
@@ -965,8 +1016,12 @@ gs_plugin_add_alternates (GsPlugin *plugin,
                          GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autoptr(GRWLockReaderLocker) locker = NULL;
+
        if (!gs_plugin_appstream_check_silo (plugin, cancellable, error))
                return FALSE;
+
+       locker = g_rw_lock_reader_locker_new (&priv->silo_lock);
        return gs_appstream_add_alternates (plugin, priv->silo, app, list,
                                            cancellable, error);
 }