[gnome-software/wip/kalev/appstream-silo-threadsafety: 2/3] flatpak: Add locking for XbSilo
- From: Kalev Lember <klember src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/wip/kalev/appstream-silo-threadsafety: 2/3] flatpak: Add locking for XbSilo
- Date: Fri, 10 May 2019 13:11:49 +0000 (UTC)
commit 35e4aeee1b9844c198ec26a133eebc0e03e81e95
Author: Kalev Lember <klember redhat com>
Date: Fri May 10 14:15:58 2019 +0200
flatpak: Add locking for XbSilo
This is the same as the previous commit, just for flatpak. To avoid
deadlocks, this commit also adds _unlocked() versions of two public
functions for internal use.
plugins/flatpak/gs-flatpak.c | 130 +++++++++++++++++++++++++++++++++++--------
1 file changed, 106 insertions(+), 24 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 9d1ef74b..e0668300 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -33,6 +33,7 @@ struct _GsFlatpak {
AsAppScope scope;
GsPlugin *plugin;
XbSilo *silo;
+ GRWLock silo_lock;
gchar *id;
guint changed_id;
};
@@ -649,13 +650,18 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self,
g_autofree gchar *blobfn = NULL;
g_autoptr(GFile) file = NULL;
g_autoptr(GPtrArray) xremotes = NULL;
+ g_autoptr(GRWLockReaderLocker) reader_locker = NULL;
+ g_autoptr(GRWLockWriterLocker) writer_locker = NULL;
g_autoptr(XbBuilder) builder = xb_builder_new ();
+ reader_locker = g_rw_lock_reader_locker_new (&self->silo_lock);
/* everything is okay */
if (self->silo != NULL && xb_silo_is_valid (self->silo))
return TRUE;
+ g_clear_pointer (&reader_locker, g_rw_lock_reader_locker_free);
/* drat! silo needs regenerating */
+ writer_locker = g_rw_lock_writer_locker_new (&self->silo_lock);
g_clear_object (&self->silo);
/* verbose profiling */
@@ -1445,8 +1451,10 @@ gs_flatpak_refresh (GsFlatpak *self,
}
/* manually do this in case we created the first appstream file */
+ g_rw_lock_reader_lock (&self->silo_lock);
if (self->silo != NULL)
xb_silo_invalidate (self->silo);
+ g_rw_lock_reader_unlock (&self->silo_lock);
/* update AppStream metadata */
if (!gs_flatpak_refresh_appstream (self, cache_age, cancellable, error))
@@ -1636,20 +1644,18 @@ gs_flatpak_create_fake_ref (GsApp *app, GError **error)
return xref;
}
-gboolean
-gs_flatpak_refine_app_state (GsFlatpak *self,
- GsApp *app,
- GCancellable *cancellable,
- GError **error)
+/* the _unlocked() version doesn't call gs_flatpak_rescan_appstream_store,
+ * in order to avoid taking the writer lock on self->silo_lock */
+static gboolean
+gs_flatpak_refine_app_state_unlocked (GsFlatpak *self,
+ GsApp *app,
+ GCancellable *cancellable,
+ GError **error)
{
g_autoptr(FlatpakInstalledRef) ref = NULL;
g_autoptr(GError) error_local = NULL;
g_autoptr(GPtrArray) refs = NULL;
- /* ensure valid */
- if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
- return FALSE;
-
/* already found */
if (gs_app_get_state (app) != AS_APP_STATE_UNKNOWN)
return TRUE;
@@ -1724,6 +1730,19 @@ gs_flatpak_refine_app_state (GsFlatpak *self,
return TRUE;
}
+gboolean
+gs_flatpak_refine_app_state (GsFlatpak *self,
+ GsApp *app,
+ GCancellable *cancellable,
+ GError **error)
+{
+ /* ensure valid */
+ if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
+ return FALSE;
+
+ return gs_flatpak_refine_app_state_unlocked (self, app, cancellable, error);
+}
+
static GsApp *
gs_flatpak_create_runtime (GsFlatpak *self, GsApp *parent, const gchar *runtime)
{
@@ -1989,10 +2008,10 @@ gs_plugin_refine_item_size (GsFlatpak *self,
/* is the app_runtime already installed? */
app_runtime = gs_app_get_runtime (app);
- if (!gs_flatpak_refine_app_state (self,
- app_runtime,
- cancellable,
- error))
+ if (!gs_flatpak_refine_app_state_unlocked (self,
+ app_runtime,
+ cancellable,
+ error))
return FALSE;
if (gs_app_get_state (app_runtime) == AS_APP_STATE_INSTALLED) {
g_debug ("runtime %s is already installed, so not adding size",
@@ -2107,22 +2126,23 @@ gs_flatpak_refine_appstream (GsFlatpak *self,
return TRUE;
}
-gboolean
-gs_flatpak_refine_app (GsFlatpak *self,
- GsApp *app,
- GsPluginRefineFlags flags,
- GCancellable *cancellable,
- GError **error)
+/* the _unlocked() version doesn't call gs_flatpak_rescan_appstream_store,
+ * in order to avoid taking the writer lock on self->silo_lock */
+static gboolean
+gs_flatpak_refine_app_unlocked (GsFlatpak *self,
+ GsApp *app,
+ GsPluginRefineFlags flags,
+ GCancellable *cancellable,
+ GError **error)
{
AsAppState old_state = gs_app_get_state (app);
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
/* not us */
if (gs_app_get_bundle_kind (app) != AS_BUNDLE_KIND_FLATPAK)
return TRUE;
- /* ensure valid */
- if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
- return FALSE;
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
/* always do AppStream properties */
if (!gs_flatpak_refine_appstream (self, app, self->silo, flags, error))
@@ -2135,7 +2155,7 @@ gs_flatpak_refine_app (GsFlatpak *self,
}
/* check the installed state */
- if (!gs_flatpak_refine_app_state (self, app, cancellable, error)) {
+ if (!gs_flatpak_refine_app_state_unlocked (self, app, cancellable, error)) {
g_prefix_error (error, "failed to get state: ");
return FALSE;
}
@@ -2191,6 +2211,20 @@ gs_flatpak_refine_app (GsFlatpak *self,
return TRUE;
}
+gboolean
+gs_flatpak_refine_app (GsFlatpak *self,
+ GsApp *app,
+ GsPluginRefineFlags flags,
+ GCancellable *cancellable,
+ GError **error)
+{
+ /* ensure valid */
+ if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
+ return FALSE;
+
+ return gs_flatpak_refine_app_unlocked (self, app, flags, cancellable, error);
+}
+
gboolean
gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app,
GsAppList *list, GsPluginRefineFlags refine_flags,
@@ -2200,6 +2234,7 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app,
g_autofree gchar *xpath = NULL;
g_autoptr(GError) error_local = NULL;
g_autoptr(GPtrArray) components = NULL;
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
/* not enough info to find */
id = gs_app_get_id (app);
@@ -2210,6 +2245,8 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app,
if (!gs_flatpak_rescan_appstream_store (self, 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);
@@ -2229,7 +2266,7 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app,
if (new == NULL)
return FALSE;
gs_flatpak_claim_app (self, new);
- if (!gs_flatpak_refine_app (self, new, refine_flags, cancellable, error))
+ if (!gs_flatpak_refine_app_unlocked (self, new, refine_flags, cancellable, error))
return FALSE;
gs_app_subsume_metadata (new, app);
gs_app_list_add (list, new);
@@ -2666,13 +2703,19 @@ gs_flatpak_search (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, 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;
+
gs_flatpak_claim_app_list (self, list_tmp);
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2684,14 +2727,20 @@ gs_flatpak_add_category_apps (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
return FALSE;
+
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
if (!gs_appstream_add_category_apps (self->plugin, self->silo,
category, list_tmp,
cancellable, error))
return FALSE;
+
gs_flatpak_claim_app_list (self, list_tmp);
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2701,8 +2750,12 @@ gs_flatpak_add_categories (GsFlatpak *self,
GCancellable *cancellable,
GError **error)
{
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
return FALSE;
+
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
return gs_appstream_add_categories (self->plugin, self->silo,
list, cancellable, error);
}
@@ -2714,12 +2767,18 @@ gs_flatpak_add_popular (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
return FALSE;
+
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
if (!gs_appstream_add_popular (self->plugin, self->silo, list_tmp,
cancellable, error))
return FALSE;
+
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2730,12 +2789,18 @@ gs_flatpak_add_featured (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
return FALSE;
+
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
if (!gs_appstream_add_featured (self->plugin, self->silo, list_tmp,
cancellable, error))
return FALSE;
+
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2747,12 +2812,18 @@ gs_flatpak_add_alternates (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, cancellable, error))
return FALSE;
+
+ locker = g_rw_lock_reader_locker_new (&self->silo_lock);
if (!gs_appstream_add_alternates (self->plugin, self->silo, app, list_tmp,
cancellable, error))
return FALSE;
+
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2764,13 +2835,19 @@ gs_flatpak_add_recent (GsFlatpak *self,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GRWLockReaderLocker) locker = NULL;
+
if (!gs_flatpak_rescan_appstream_store (self, 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;
+
gs_flatpak_claim_app_list (self, list_tmp);
gs_app_list_add_list (list, list_tmp);
+
return TRUE;
}
@@ -2823,6 +2900,7 @@ gs_flatpak_finalize (GObject *object)
g_object_unref (self->plugin);
g_hash_table_unref (self->broken_remotes);
g_mutex_clear (&self->broken_remotes_mutex);
+ g_rw_lock_clear (&self->silo_lock);
G_OBJECT_CLASS (gs_flatpak_parent_class)->finalize (object);
}
@@ -2837,6 +2915,10 @@ gs_flatpak_class_init (GsFlatpakClass *klass)
static void
gs_flatpak_init (GsFlatpak *self)
{
+ /* XbSilo needs external locking as we destroy the silo and build a new
+ * one when something changes */
+ g_rw_lock_init (&self->silo_lock);
+
g_mutex_init (&self->broken_remotes_mutex);
self->broken_remotes = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, NULL);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]