From f1140fd98666fbdb35dfe13a6b3f58fedcb1ad0b Mon Sep 17 00:00:00 2001 Date: Mon, 29 Apr 2024 17:37:57 +0200 Subject: [PATCH 1/3] gs-plugin-appstream: Simplify XbSilo locking Instead of holding RW lock on the internal silo variable, simply return a reference to it and work with it like with an immutable object. That simplifies locking and makes the code cleaner. It can also make the code quicker, in some cases. --- plugins/core/gs-plugin-appstream.c | 194 ++++++++++++++++------------- 1 file changed, 107 insertions(+), 87 deletions(-) diff --git a/plugins/core/gs-plugin-appstream.c b/plugins/core/gs-plugin-appstream.c index 7ccbac12a5..5749a175f3 100644 --- a/plugins/core/gs-plugin-appstream.c +++ b/plugins/core/gs-plugin-appstream.c @@ -36,7 +36,7 @@ struct _GsPluginAppstream GsWorkerThread *worker; /* (owned) */ XbSilo *silo; - GRWLock silo_lock; + GMutex silo_lock; gchar *silo_filename; GHashTable *silo_installed_by_desktopid; GHashTable *silo_installed_by_id; @@ -65,7 +65,7 @@ gs_plugin_appstream_dispose (GObject *object) g_clear_pointer (&self->silo_installed_by_desktopid, g_hash_table_unref); g_clear_pointer (&self->silo_installed_by_id, g_hash_table_unref); g_clear_object (&self->settings); - g_rw_lock_clear (&self->silo_lock); + g_mutex_clear (&self->silo_lock); g_clear_object (&self->worker); g_clear_pointer (&self->file_monitors, g_ptr_array_unref); @@ -77,9 +77,7 @@ gs_plugin_appstream_init (GsPluginAppstream *self) { GApplication *application = g_application_get_default (); - /* 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->silo_lock); /* require settings */ self->settings = g_settings_new ("org.gnome.software"); @@ -530,10 +528,13 @@ gs_add_appstream_metainfo_location (GPtrArray *locations, const gchar *root) g_build_filename (root, "appdata", NULL)); } -static gboolean -gs_plugin_appstream_check_silo (GsPluginAppstream *self, - GCancellable *cancellable, - GError **error) +static XbSilo * +gs_plugin_appstream_ref_silo (GsPluginAppstream *self, + gchar **out_silo_filename, + GHashTable **out_silo_installed_by_desktopid, + GHashTable **out_silo_installed_by_id, + GCancellable *cancellable, + GError **error) { const gchar *test_xml; g_autofree gchar *blobfn = NULL; @@ -541,21 +542,25 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, g_autoptr(XbNode) n = NULL; g_autoptr(GFile) file = NULL; g_autoptr(GPtrArray) installed = NULL; - g_autoptr(GRWLockReaderLocker) reader_locker = NULL; - g_autoptr(GRWLockWriterLocker) writer_locker = NULL; + g_autoptr(GMutexLocker) locker = NULL; g_autoptr(GPtrArray) parent_appdata = g_ptr_array_new_with_free_func (g_free); g_autoptr(GPtrArray) parent_appstream = NULL; g_autoptr(GMainContext) old_thread_default = NULL; - reader_locker = g_rw_lock_reader_locker_new (&self->silo_lock); + locker = g_mutex_locker_new (&self->silo_lock); /* everything is okay */ if (self->silo != NULL && xb_silo_is_valid (self->silo) && - g_atomic_int_get (&self->file_monitor_stamp_current) == g_atomic_int_get (&self->file_monitor_stamp)) - return TRUE; - g_clear_pointer (&reader_locker, g_rw_lock_reader_locker_free); + g_atomic_int_get (&self->file_monitor_stamp_current) == g_atomic_int_get (&self->file_monitor_stamp)) { + if (out_silo_filename != NULL) + *out_silo_filename = g_strdup (self->silo_filename); + if (out_silo_installed_by_desktopid != NULL) + *out_silo_installed_by_desktopid = self->silo_installed_by_desktopid ? g_hash_table_ref (self->silo_installed_by_desktopid) : NULL; + if (out_silo_installed_by_id != NULL) + *out_silo_installed_by_id = self->silo_installed_by_id ? g_hash_table_ref (self->silo_installed_by_id) : NULL; + return g_object_ref (self->silo); + } /* drat! silo needs regenerating */ - writer_locker = g_rw_lock_writer_locker_new (&self->silo_lock); reload: g_clear_object (&self->silo); g_clear_pointer (&self->silo_filename, g_free); @@ -594,7 +599,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, if (!xb_builder_source_load_xml (source, test_xml, XB_BUILDER_SOURCE_FLAG_NONE, error)) - return FALSE; + return NULL; fixup1 = xb_builder_fixup_new ("AddOriginKeywords", gs_plugin_appstream_add_origin_keyword_cb, self, NULL); @@ -640,7 +645,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, if (!gs_plugin_appstream_load_appstream (self, builder, fn, cancellable, error)) { if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); - return FALSE; + return NULL; } } for (guint i = 0; i < parent_appdata->len; i++) { @@ -648,7 +653,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, if (!gs_plugin_appstream_load_appdata (self, builder, fn, cancellable, error)) { if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); - return FALSE; + return NULL; } } for (guint i = 0; i < parent_desktop->len; i++) { @@ -657,7 +662,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, if (!gs_appstream_load_desktop_files (builder, dir, NULL, &file_monitor, cancellable, error)) { if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); - return FALSE; + return NULL; } gs_plugin_appstream_maybe_store_file_monitor (self, file_monitor); } @@ -678,7 +683,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, GS_UTILS_CACHE_FLAG_CREATE_DIRECTORY, error); if (blobfn == NULL) - return FALSE; + return NULL; file = g_file_new_for_path (blobfn); g_debug ("ensuring %s", blobfn); @@ -696,7 +701,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, if (self->silo == NULL) { if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); - return FALSE; + return NULL; } if (old_thread_default != NULL) @@ -717,7 +722,7 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_NOT_SUPPORTED, "No AppStream data found"); - return FALSE; + return NULL; } g_clear_object (&n); @@ -768,7 +773,13 @@ gs_plugin_appstream_check_silo (GsPluginAppstream *self, } /* success */ - return TRUE; + if (out_silo_filename != NULL) + *out_silo_filename = g_strdup (self->silo_filename); + if (out_silo_installed_by_desktopid != NULL) + *out_silo_installed_by_desktopid = self->silo_installed_by_desktopid ? g_hash_table_ref (self->silo_installed_by_desktopid) : NULL; + if (out_silo_installed_by_id != NULL) + *out_silo_installed_by_id = self->silo_installed_by_id ? g_hash_table_ref (self->silo_installed_by_id) : NULL; + return g_object_ref (self->silo); } static void @@ -776,7 +787,6 @@ gs_plugin_appstream_reload (GsPlugin *plugin) { GsPluginAppstream *self; g_autoptr(GsAppList) list = NULL; - g_autoptr(GRWLockWriterLocker) writer_locker = NULL; guint sz; g_return_if_fail (GS_IS_PLUGIN_APPSTREAM (plugin)); @@ -790,9 +800,8 @@ gs_plugin_appstream_reload (GsPlugin *plugin) } self = GS_PLUGIN_APPSTREAM (plugin); - writer_locker = g_rw_lock_writer_locker_new (&self->silo_lock); - if (self->silo != NULL) - xb_silo_invalidate (self->silo); + /* simpler than getting the silo and setting it invalid */ + g_atomic_int_inc (&self->file_monitor_stamp); } static gint @@ -834,11 +843,13 @@ setup_thread_cb (GTask *task, GCancellable *cancellable) { GsPluginAppstream *self = GS_PLUGIN_APPSTREAM (source_object); + g_autoptr(XbSilo) silo = NULL; g_autoptr(GError) local_error = NULL; assert_in_worker (self); - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) + silo = gs_plugin_appstream_ref_silo (self, NULL, NULL, NULL, cancellable, &local_error); + if (silo == NULL) g_task_return_error (task, g_steal_pointer (&local_error)); else g_task_return_boolean (task, TRUE); @@ -910,21 +921,21 @@ url_to_app_thread_cb (GTask *task, GsPluginAppstream *self = GS_PLUGIN_APPSTREAM (source_object); GsPluginUrlToAppData *data = task_data; g_autoptr(GsAppList) list = NULL; - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; g_autoptr(GError) local_error = NULL; assert_in_worker (self); /* check silo is valid */ - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) { + silo = gs_plugin_appstream_ref_silo (self, NULL, NULL, NULL, cancellable, &local_error); + if (silo == NULL) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } - locker = g_rw_lock_reader_locker_new (&self->silo_lock); list = gs_app_list_new (); - if (gs_appstream_url_to_app (GS_PLUGIN (self), self->silo, list, data->url, cancellable, &local_error)) + if (gs_appstream_url_to_app (GS_PLUGIN (self), silo, list, data->url, cancellable, &local_error)) g_task_return_pointer (task, g_steal_pointer (&list), g_object_unref); else g_task_return_error (task, g_steal_pointer (&local_error)); @@ -1006,17 +1017,14 @@ gs_plugin_appstream_set_compulsory_quirk (GsApp *app, XbNode *component) static gboolean gs_plugin_appstream_refine_state (GsPluginAppstream *self, GsApp *app, + GHashTable *silo_installed_by_id, GError **error) { - g_autoptr(GRWLockReaderLocker) locker = NULL; - /* Ignore apps with no ID */ - if (gs_app_get_id (app) == NULL) + if (gs_app_get_id (app) == NULL || silo_installed_by_id == NULL) return TRUE; - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - - if (g_hash_table_contains (self->silo_installed_by_id, gs_app_get_id (app))) + if (g_hash_table_contains (silo_installed_by_id, gs_app_get_id (app))) gs_app_set_state (app, GS_APP_STATE_INSTALLED); return TRUE; } @@ -1027,20 +1035,21 @@ gs_plugin_refine_from_id (GsPluginAppstream *self, GsPluginRefineFlags flags, GHashTable *apps_by_id, GHashTable *apps_by_origin_and_id, + XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, + GHashTable *silo_installed_by_id, gboolean *found, GError **error) { const gchar *id, *origin; GPtrArray *components; - g_autoptr(GRWLockReaderLocker) locker = NULL; /* not enough info to find */ id = gs_app_get_id (app); if (id == NULL) return TRUE; - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - origin = gs_app_get_origin_appstream (app); /* look in AppStream then fall back to AppData */ @@ -1056,15 +1065,15 @@ gs_plugin_refine_from_id (GsPluginAppstream *self, for (guint i = 0; i < components->len; i++) { XbNode *component = g_ptr_array_index (components, i); - if (!gs_appstream_refine_app (GS_PLUGIN (self), app, self->silo, component, flags, self->silo_installed_by_desktopid, - self->silo_filename ? self->silo_filename : "", self->default_scope, error)) + if (!gs_appstream_refine_app (GS_PLUGIN (self), app, silo, component, flags, silo_installed_by_desktopid, + silo_filename ? silo_filename : "", self->default_scope, error)) return FALSE; gs_plugin_appstream_set_compulsory_quirk (app, component); } /* if an installed desktop or appdata file exists set to installed */ if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN) { - if (!gs_plugin_appstream_refine_state (self, app, error)) + if (!gs_plugin_appstream_refine_state (self, app, silo_installed_by_id, error)) return FALSE; } @@ -1077,6 +1086,10 @@ static gboolean gs_plugin_refine_from_pkgname (GsPluginAppstream *self, GsApp *app, GsPluginRefineFlags flags, + XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, + GHashTable *silo_installed_by_id, GError **error) { GPtrArray *sources = gs_app_get_sources (app); @@ -1089,33 +1102,30 @@ gs_plugin_refine_from_pkgname (GsPluginAppstream *self, /* find all apps when matching any prefixes */ for (guint j = 0; j < sources->len; j++) { const gchar *pkgname = g_ptr_array_index (sources, j); - g_autoptr(GRWLockReaderLocker) locker = NULL; g_autoptr(GString) xpath = g_string_new (NULL); g_autoptr(XbNode) component = NULL; - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - /* prefer actual apps and then fallback to anything else */ xb_string_append_union (xpath, "components/component[@type='desktop-application']/pkgname[text()='%s']/..", pkgname); xb_string_append_union (xpath, "components/component[@type='console-application']/pkgname[text()='%s']/..", pkgname); xb_string_append_union (xpath, "components/component[@type='web-application']/pkgname[text()='%s']/..", pkgname); xb_string_append_union (xpath, "components/component/pkgname[text()='%s']/..", pkgname); - component = xb_silo_query_first (self->silo, xpath->str, &error_local); + component = xb_silo_query_first (silo, xpath->str, &error_local); if (component == NULL) { if (g_error_matches (error_local, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) continue; g_propagate_error (error, g_steal_pointer (&error_local)); return FALSE; } - if (!gs_appstream_refine_app (GS_PLUGIN (self), app, self->silo, component, flags, self->silo_installed_by_desktopid, - self->silo_filename ? self->silo_filename : "", self->default_scope, error)) + if (!gs_appstream_refine_app (GS_PLUGIN (self), app, silo, component, flags, silo_installed_by_desktopid, + silo_filename ? silo_filename : "", self->default_scope, error)) return FALSE; gs_plugin_appstream_set_compulsory_quirk (app, component); } /* if an installed desktop or appdata file exists set to installed */ if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN) { - if (!gs_plugin_appstream_refine_state (self, app, error)) + if (!gs_plugin_appstream_refine_state (self, app, silo_installed_by_id, error)) return FALSE; } @@ -1153,6 +1163,10 @@ static gboolean refine_wildcard (GsPluginAppstream *self, GsAppList *list, GsPluginRefineFlags refine_flags, GHashTable *apps_by_id, + XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, + GHashTable *silo_installed_by_id, GCancellable *cancellable, GError **error); @@ -1171,13 +1185,17 @@ refine_thread_cb (GTask *task, g_autoptr(GHashTable) apps_by_id = NULL; g_autoptr(GHashTable) apps_by_origin_and_id = NULL; g_autoptr(GPtrArray) components = NULL; - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; + g_autofree gchar *silo_filename = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; + g_autoptr(GHashTable) silo_installed_by_id = NULL; g_autoptr(GError) local_error = NULL; assert_in_worker (self); /* check silo is valid */ - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) { + silo = gs_plugin_appstream_ref_silo (self, &silo_filename, &silo_installed_by_desktopid, &silo_installed_by_id, cancellable, &local_error); + if (silo == NULL) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } @@ -1185,9 +1203,7 @@ refine_thread_cb (GTask *task, apps_by_id = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_ptr_array_unref); apps_by_origin_and_id = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_ptr_array_unref); - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - - components = xb_silo_query (self->silo, "components/component/id", 0, NULL); + components = xb_silo_query (silo, "components/component/id", 0, NULL); for (guint i = 0; components != NULL && i < components->len; i++) { XbNode *node = g_ptr_array_index (components, i); g_autoptr(XbNode) component_node = xb_node_get_parent (node); @@ -1234,7 +1250,7 @@ refine_thread_cb (GTask *task, } g_clear_pointer (&components, g_ptr_array_unref); - components = xb_silo_query (self->silo, "component/id", 0, NULL); + components = xb_silo_query (silo, "component/id", 0, NULL); for (guint i = 0; components != NULL && i < components->len; i++) { XbNode *node = g_ptr_array_index (components, i); g_autoptr(XbNode) component_node = xb_node_get_parent (node); @@ -1262,12 +1278,14 @@ refine_thread_cb (GTask *task, continue; /* find by ID then fall back to package name */ - if (!gs_plugin_refine_from_id (self, app, flags, apps_by_id, apps_by_origin_and_id, &found, &local_error)) { + if (!gs_plugin_refine_from_id (self, app, flags, apps_by_id, apps_by_origin_and_id, silo, silo_filename, + silo_installed_by_desktopid, silo_installed_by_id, &found, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (!found) { - if (!gs_plugin_refine_from_pkgname (self, app, flags, &local_error)) { + if (!gs_plugin_refine_from_pkgname (self, app, flags, silo, silo_filename, + silo_installed_by_desktopid, silo_installed_by_id, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } @@ -1286,7 +1304,8 @@ refine_thread_cb (GTask *task, GsApp *app = gs_app_list_index (app_list, j); if (gs_app_has_quirk (app, GS_APP_QUIRK_IS_WILDCARD) && - !refine_wildcard (self, app, list, flags, apps_by_id, cancellable, &local_error)) { + !refine_wildcard (self, app, list, flags, apps_by_id, silo, silo_filename, + silo_installed_by_desktopid, silo_installed_by_id,cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } @@ -1311,20 +1330,21 @@ refine_wildcard (GsPluginAppstream *self, GsAppList *list, GsPluginRefineFlags refine_flags, GHashTable *apps_by_id, + XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, + GHashTable *silo_installed_by_id, GCancellable *cancellable, GError **error) { const gchar *id; GPtrArray *components; - g_autoptr(GRWLockReaderLocker) locker = NULL; /* not enough info to find */ id = gs_app_get_id (app); if (id == NULL) return TRUE; - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - components = g_hash_table_lookup (apps_by_id, id); if (components == NULL) return TRUE; @@ -1333,20 +1353,20 @@ refine_wildcard (GsPluginAppstream *self, g_autoptr(GsApp) new = NULL; /* new app */ - new = gs_appstream_create_app (GS_PLUGIN (self), self->silo, component, self->silo_filename ? self->silo_filename : "", + new = gs_appstream_create_app (GS_PLUGIN (self), silo, component, silo_filename ? silo_filename : "", self->default_scope, error); if (new == NULL) return FALSE; gs_app_set_scope (new, AS_COMPONENT_SCOPE_SYSTEM); gs_app_subsume_metadata (new, app); - if (!gs_appstream_refine_app (GS_PLUGIN (self), new, self->silo, component, refine_flags, self->silo_installed_by_desktopid, - self->silo_filename ? self->silo_filename : "", self->default_scope, error)) + if (!gs_appstream_refine_app (GS_PLUGIN (self), new, silo, component, refine_flags, silo_installed_by_desktopid, + silo_filename ? silo_filename : "", self->default_scope, error)) return FALSE; gs_plugin_appstream_set_compulsory_quirk (new, component); /* if an installed desktop or appdata file exists set to installed */ if (gs_app_get_state (new) == GS_APP_STATE_UNKNOWN) { - if (!gs_plugin_appstream_refine_state (self, new, error)) + if (!gs_plugin_appstream_refine_state (self, new, silo_installed_by_id, error)) return FALSE; } @@ -1398,21 +1418,20 @@ refine_categories_thread_cb (GTask *task, GCancellable *cancellable) { GsPluginAppstream *self = GS_PLUGIN_APPSTREAM (source_object); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; GsPluginRefineCategoriesData *data = task_data; g_autoptr(GError) local_error = NULL; assert_in_worker (self); /* check silo is valid */ - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) { + silo = gs_plugin_appstream_ref_silo (self, NULL, NULL, NULL, cancellable, &local_error); + if (silo == NULL) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - - if (!gs_appstream_refine_category_sizes (self->silo, data->list, cancellable, &local_error)) { + if (!gs_appstream_refine_category_sizes (silo, data->list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } @@ -1462,7 +1481,7 @@ list_apps_thread_cb (GTask *task, GCancellable *cancellable) { GsPluginAppstream *self = GS_PLUGIN_APPSTREAM (source_object); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; g_autoptr(GsAppList) list = gs_app_list_new (); GsPluginListAppsData *data = task_data; GDateTime *released_since = NULL; @@ -1516,65 +1535,64 @@ list_apps_thread_cb (GTask *task, } /* check silo is valid */ - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) { + silo = gs_plugin_appstream_ref_silo (self, NULL, NULL, NULL, cancellable, &local_error); + if (silo == NULL) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } - locker = g_rw_lock_reader_locker_new (&self->silo_lock); - if (released_since != NULL && - !gs_appstream_add_recent (GS_PLUGIN (self), self->silo, list, age_secs, + !gs_appstream_add_recent (GS_PLUGIN (self), silo, list, age_secs, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (is_curated != GS_APP_QUERY_TRISTATE_UNSET && - !gs_appstream_add_popular (self->silo, list, cancellable, &local_error)) { + !gs_appstream_add_popular (silo, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (is_featured != GS_APP_QUERY_TRISTATE_UNSET && - !gs_appstream_add_featured (self->silo, list, cancellable, &local_error)) { + !gs_appstream_add_featured (silo, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (category != NULL && - !gs_appstream_add_category_apps (GS_PLUGIN (self), self->silo, category, list, cancellable, &local_error)) { + !gs_appstream_add_category_apps (GS_PLUGIN (self), silo, category, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (is_installed == GS_APP_QUERY_TRISTATE_TRUE && - !gs_appstream_add_installed (GS_PLUGIN (self), self->silo, list, cancellable, &local_error)) { + !gs_appstream_add_installed (GS_PLUGIN (self), silo, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (deployment_featured != NULL && - !gs_appstream_add_deployment_featured (self->silo, deployment_featured, list, + !gs_appstream_add_deployment_featured (silo, deployment_featured, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (developers != NULL && - !gs_appstream_search_developer_apps (GS_PLUGIN (self), self->silo, developers, list, cancellable, &local_error)) { + !gs_appstream_search_developer_apps (GS_PLUGIN (self), silo, developers, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (keywords != NULL && - !gs_appstream_search (GS_PLUGIN (self), self->silo, keywords, list, cancellable, &local_error)) { + !gs_appstream_search (GS_PLUGIN (self), silo, keywords, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } if (alternate_of != NULL && - !gs_appstream_add_alternates (self->silo, alternate_of, list, cancellable, &local_error)) { + !gs_appstream_add_alternates (silo, alternate_of, list, cancellable, &local_error)) { g_task_return_error (task, g_steal_pointer (&local_error)); return; } @@ -1623,12 +1641,14 @@ refresh_metadata_thread_cb (GTask *task, GCancellable *cancellable) { GsPluginAppstream *self = GS_PLUGIN_APPSTREAM (source_object); + g_autoptr(XbSilo) silo = NULL; g_autoptr(GError) local_error = NULL; assert_in_worker (self); /* Checking the silo will refresh it if needed. */ - if (!gs_plugin_appstream_check_silo (self, cancellable, &local_error)) + silo = gs_plugin_appstream_ref_silo (self, NULL, NULL, NULL, cancellable, &local_error); + if (silo == NULL) g_task_return_error (task, g_steal_pointer (&local_error)); else g_task_return_boolean (task, TRUE); -- GitLab From cf04c32b5fb1fd267096fad8dd11a22429ad788a Mon Sep 17 00:00:00 2001 Date: Mon, 29 Apr 2024 17:45:08 +0200 Subject: [PATCH 2/3] gs-plugin-appstream: Rename file_monitor_stamp to silo_change_stamp Since it's used also in the gs_plugin_appstream_reload() and works like a change indicator, name it appropriately to avoid confusion. --- plugins/core/gs-plugin-appstream.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/core/gs-plugin-appstream.c b/plugins/core/gs-plugin-appstream.c index 5749a175f3..99fe5a1432 100644 --- a/plugins/core/gs-plugin-appstream.c +++ b/plugins/core/gs-plugin-appstream.c @@ -46,8 +46,8 @@ struct _GsPluginAppstream GPtrArray *file_monitors; /* (owned) (element-type GFileMonitor) */ /* The stamps help to avoid locking the silo lock in the main thread and also to detect changes while loading other appstream data. */ - gint file_monitor_stamp; /* the file monitor stamp, increased on every file monitor change */ - gint file_monitor_stamp_current; /* the currently known file monitor stamp, checked for changes */ + gint silo_change_stamp; /* the silo change stamp, increased on every silo change */ + gint silo_change_stamp_current; /* the currently known silo change stamp, checked for changes */ }; G_DEFINE_TYPE (GsPluginAppstream, gs_plugin_appstream, GS_TYPE_PLUGIN) @@ -224,7 +224,7 @@ gs_plugin_appstream_file_monitor_changed_cb (GFileMonitor *monitor, gpointer user_data) { GsPluginAppstream *self = user_data; - g_atomic_int_inc (&self->file_monitor_stamp); + g_atomic_int_inc (&self->silo_change_stamp); } static void @@ -550,7 +550,7 @@ gs_plugin_appstream_ref_silo (GsPluginAppstream *self, locker = g_mutex_locker_new (&self->silo_lock); /* everything is okay */ if (self->silo != NULL && xb_silo_is_valid (self->silo) && - g_atomic_int_get (&self->file_monitor_stamp_current) == g_atomic_int_get (&self->file_monitor_stamp)) { + g_atomic_int_get (&self->silo_change_stamp_current) == g_atomic_int_get (&self->silo_change_stamp)) { if (out_silo_filename != NULL) *out_silo_filename = g_strdup (self->silo_filename); if (out_silo_installed_by_desktopid != NULL) @@ -568,7 +568,7 @@ gs_plugin_appstream_ref_silo (GsPluginAppstream *self, g_clear_pointer (&blobfn, g_free); self->default_scope = AS_COMPONENT_SCOPE_UNKNOWN; g_ptr_array_set_size (self->file_monitors, 0); - g_atomic_int_set (&self->file_monitor_stamp_current, g_atomic_int_get (&self->file_monitor_stamp)); + g_atomic_int_set (&self->silo_change_stamp_current, g_atomic_int_get (&self->silo_change_stamp)); /* FIXME: https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1422 */ old_thread_default = g_main_context_ref_thread_default (); @@ -707,7 +707,7 @@ gs_plugin_appstream_ref_silo (GsPluginAppstream *self, if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); - if (g_atomic_int_get (&self->file_monitor_stamp_current) != g_atomic_int_get (&self->file_monitor_stamp)) { + if (g_atomic_int_get (&self->silo_change_stamp_current) != g_atomic_int_get (&self->silo_change_stamp)) { g_ptr_array_set_size (parent_appdata, 0); g_ptr_array_set_size (parent_appstream, 0); g_debug ("appstream: File monitors reported change while loading appstream data, reloading..."); @@ -800,8 +800,8 @@ gs_plugin_appstream_reload (GsPlugin *plugin) } self = GS_PLUGIN_APPSTREAM (plugin); - /* simpler than getting the silo and setting it invalid */ - g_atomic_int_inc (&self->file_monitor_stamp); + /* Invalidate the reference to the current silo */ + g_atomic_int_inc (&self->silo_change_stamp); } static gint -- GitLab From 59b301b8a878f3e81359909a8d4107715eb60a35 Mon Sep 17 00:00:00 2001 Date: Tue, 30 Apr 2024 13:13:36 +0200 Subject: [PATCH 3/3] flatpak: Simplify XbSilo locking Instead of holding RW lock on the internal silo variable, simply return a reference to it and work with it like with an immutable object. That simplifies locking and makes the code cleaner. It can also make the code quicker, in some cases. --- plugins/flatpak/gs-flatpak.c | 307 +++++++++++++++++++---------------- 1 file changed, 163 insertions(+), 144 deletions(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index 2e175fd08e..9893cf88b7 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -54,9 +54,11 @@ struct _GsFlatpak { AsComponentScope scope; GsPlugin *plugin; XbSilo *silo; - GRWLock silo_lock; + GMutex silo_lock; gchar *silo_filename; GHashTable *silo_installed_by_desktopid; + gint silo_change_stamp; + gint silo_change_stamp_current; gchar *id; guint changed_id; GHashTable *app_silos; @@ -654,10 +656,7 @@ gs_flatpak_create_source (GsFlatpak *self, FlatpakRemote *xremote) static void gs_flatpak_invalidate_silo (GsFlatpak *self) { - g_rw_lock_writer_lock (&self->silo_lock); - if (self->silo != NULL) - xb_silo_invalidate (self->silo); - g_rw_lock_writer_unlock (&self->silo_lock); + g_atomic_int_inc (&self->silo_change_stamp); } static void @@ -1085,32 +1084,39 @@ gs_flatpak_rescan_installed (GsFlatpak *self, g_debug ("Failed to read flatpak .desktop files in %s: %s", path, error_local->message); } -static gboolean -gs_flatpak_rescan_appstream_store (GsFlatpak *self, - gboolean interactive, - GCancellable *cancellable, - GError **error) +static XbSilo * +gs_flatpak_ref_silo (GsFlatpak *self, + gboolean interactive, + gchar **out_silo_filename, + GHashTable **out_silo_installed_by_desktopid, + GCancellable *cancellable, + GError **error) { g_autofree gchar *blobfn = NULL; g_autoptr(GFile) file = NULL; g_autoptr(GPtrArray) xremotes = NULL; g_autoptr(GPtrArray) desktop_paths = NULL; - g_autoptr(GRWLockReaderLocker) reader_locker = NULL; - g_autoptr(GRWLockWriterLocker) writer_locker = NULL; + g_autoptr(GMutexLocker) locker = NULL; g_autoptr(XbBuilder) builder = NULL; g_autoptr(GMainContext) old_thread_default = NULL; - reader_locker = g_rw_lock_reader_locker_new (&self->silo_lock); + locker = g_mutex_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); + if (self->silo != NULL && xb_silo_is_valid (self->silo) && + g_atomic_int_get (&self->silo_change_stamp_current) == g_atomic_int_get (&self->silo_change_stamp)) { + if (out_silo_filename != NULL) + *out_silo_filename = g_strdup (self->silo_filename); + if (out_silo_installed_by_desktopid != NULL && self->silo_installed_by_desktopid) + *out_silo_installed_by_desktopid = g_hash_table_ref (self->silo_installed_by_desktopid); + return g_object_ref (self->silo); + } /* drat! silo needs regenerating */ - writer_locker = g_rw_lock_writer_locker_new (&self->silo_lock); + reload: g_clear_object (&self->silo); g_clear_pointer (&self->silo_filename, g_free); g_clear_pointer (&self->silo_installed_by_desktopid, g_hash_table_unref); + g_atomic_int_set (&self->silo_change_stamp_current, g_atomic_int_get (&self->silo_change_stamp)); /* FIXME: https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1422 */ old_thread_default = g_main_context_ref_thread_default (); @@ -1138,7 +1144,7 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, error); if (xremotes == NULL) { gs_flatpak_error_convert (error); - return FALSE; + return NULL; } for (guint i = 0; i < xremotes->len; i++) { g_autoptr(GError) error_local = NULL; @@ -1152,7 +1158,7 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, flatpak_remote_get_name (xremote), error_local->message); if (g_cancellable_set_error_if_cancelled (cancellable, error)) { gs_flatpak_error_convert (error); - return FALSE; + return NULL; } } } @@ -1176,7 +1182,7 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, GS_UTILS_CACHE_FLAG_CREATE_DIRECTORY, error); if (blobfn == NULL) - return FALSE; + return NULL; file = g_file_new_for_path (blobfn); g_debug ("ensuring %s", blobfn); @@ -1195,6 +1201,17 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); + if (g_atomic_int_get (&self->silo_change_stamp_current) != g_atomic_int_get (&self->silo_change_stamp)) { + g_clear_pointer (&blobfn, g_free); + g_clear_pointer (&xremotes, g_ptr_array_unref); + g_clear_pointer (&desktop_paths, g_ptr_array_unref); + g_clear_pointer (&old_thread_default, g_main_context_unref); + g_clear_object (&file); + g_clear_object (&builder); + g_debug ("flatpak: Reported change while loading appstream data, reloading..."); + goto reload; + } + if (self->silo != NULL) { g_autoptr(GPtrArray) installed = NULL; g_autoptr(XbNode) info_filename = NULL; @@ -1218,66 +1235,46 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, info_filename = xb_silo_query_first (self->silo, "/info/filename", NULL); if (info_filename != NULL) self->silo_filename = g_strdup (xb_node_get_text (info_filename)); + + if (out_silo_filename != NULL) + *out_silo_filename = g_strdup (self->silo_filename); + if (out_silo_installed_by_desktopid != NULL && self->silo_installed_by_desktopid) + *out_silo_installed_by_desktopid = g_hash_table_ref (self->silo_installed_by_desktopid); + return g_object_ref (self->silo); } - /* success */ - return self->silo != NULL; + return NULL; } static gboolean gs_flatpak_rescan_app_data (GsFlatpak *self, gboolean interactive, + XbSilo **out_silo, + gchar **out_silo_filename, + GHashTable **out_silo_installed_by_desktopid, GCancellable *cancellable, GError **error) { + g_autoptr(XbSilo) silo = NULL; + if (self->requires_full_rescan) { gboolean res = gs_flatpak_refresh (self, 60, interactive, cancellable, error); - if (res) + if (res) { self->requires_full_rescan = FALSE; - else + } else { gs_flatpak_internal_data_changed (self); - return res; + return res; + } } - if (!gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error)) { + silo = gs_flatpak_ref_silo (self, interactive, out_silo_filename, out_silo_installed_by_desktopid, cancellable, error); + if (silo == NULL) { gs_flatpak_internal_data_changed (self); return FALSE; } - 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); - } + if (out_silo != NULL) + *out_silo = g_steal_pointer (&silo); return TRUE; } @@ -1412,6 +1409,7 @@ gs_flatpak_refresh_appstream (GsFlatpak *self, { gboolean ret; g_autoptr(GPtrArray) xremotes = NULL; + g_autoptr(XbSilo) silo = NULL; /* get remotes */ xremotes = flatpak_installation_list_remotes (gs_flatpak_get_installation (self, interactive), @@ -1498,7 +1496,8 @@ gs_flatpak_refresh_appstream (GsFlatpak *self, } /* ensure the AppStream silo is up to date */ - if (!gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error)) { + silo = gs_flatpak_ref_silo (self, interactive, NULL, NULL, cancellable, error); + if (silo == NULL) { gs_flatpak_internal_data_changed (self); return FALSE; } @@ -1646,7 +1645,7 @@ gs_flatpak_add_sources (GsFlatpak *self, FlatpakInstallation *installation = gs_flatpak_get_installation (self, interactive); /* refresh */ - if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, NULL, NULL, NULL, cancellable, error)) return FALSE; /* get installed apps and runtimes */ @@ -2080,7 +2079,7 @@ gs_flatpak_add_updates (GsFlatpak *self, FlatpakInstallation *installation = gs_flatpak_get_installation (self, interactive); /* ensure valid */ - if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, NULL, NULL, NULL, cancellable, error)) return FALSE; /* get all the updatable apps and runtimes */ @@ -2390,10 +2389,8 @@ gs_flatpak_create_fake_ref (GsApp *app, GError **error) return xref; } -/* the _unlocked() version doesn't call gs_flatpak_rescan_app_data, - * in order to avoid taking the writer lock on self->silo_lock */ static gboolean -gs_flatpak_refine_app_state_unlocked (GsFlatpak *self, +gs_flatpak_refine_app_state_internal (GsFlatpak *self, GsApp *app, gboolean interactive, gboolean force_state_update, @@ -2505,10 +2502,10 @@ gs_flatpak_refine_app_state (GsFlatpak *self, GError **error) { /* ensure valid */ - if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, NULL, NULL, NULL, cancellable, error)) return FALSE; - return gs_flatpak_refine_app_state_unlocked (self, app, interactive, force_state_update, cancellable, error); + return gs_flatpak_refine_app_state_internal (self, app, interactive, force_state_update, cancellable, error); } static GsApp * @@ -2594,7 +2591,7 @@ gs_flatpak_create_runtime (GsFlatpak *self, gs_flatpak_app_set_ref_name (app, split[0]); gs_flatpak_app_set_ref_arch (app, split[1]); - if (!gs_flatpak_refine_app_state_unlocked (self, app, interactive, FALSE, NULL, &local_error)) + if (!gs_flatpak_refine_app_state_internal (self, app, interactive, FALSE, NULL, &local_error)) g_debug ("Failed to refine state for runtime '%s': %s", gs_app_get_unique_id (app), local_error->message); /* save in the cache */ @@ -2984,7 +2981,7 @@ 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_unlocked (self, + if (!gs_flatpak_refine_app_state_internal (self, app_runtime, interactive, FALSE, @@ -3096,6 +3093,8 @@ gs_flatpak_refine_appstream_from_bytes (GsFlatpak *self, GBytes *appstream_gz, GsPluginRefineFlags flags, gboolean interactive, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, GCancellable *cancellable, GError **error) { @@ -3229,8 +3228,8 @@ gs_flatpak_refine_appstream_from_bytes (GsFlatpak *self, } /* copy details from AppStream to app */ - if (!gs_appstream_refine_app (self->plugin, app, silo, component_node, flags, self->silo_installed_by_desktopid, - self->silo_filename ? self->silo_filename : "", self->scope, error)) + if (!gs_appstream_refine_app (self->plugin, app, silo, component_node, flags, silo_installed_by_desktopid, + silo_filename ? silo_filename : "", self->scope, error)) return FALSE; if (gs_app_get_origin (app)) @@ -3324,6 +3323,8 @@ static gboolean gs_flatpak_refine_appstream (GsFlatpak *self, GsApp *app, XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, GsPluginRefineFlags flags, GHashTable *components_by_bundle, gboolean interactive, @@ -3410,11 +3411,12 @@ gs_flatpak_refine_appstream (GsFlatpak *self, appstream_gz, flags, interactive, + silo_filename, silo_installed_by_desktopid, cancellable, error); } - if (!gs_appstream_refine_app (self->plugin, app, silo, component, flags, self->silo_installed_by_desktopid, - self->silo_filename ? self->silo_filename : "", self->scope, error)) + if (!gs_appstream_refine_app (self->plugin, app, silo, component, flags, silo_installed_by_desktopid, + silo_filename ? silo_filename : "", self->scope, error)) return FALSE; /* use the default release as the version number */ @@ -3423,13 +3425,15 @@ gs_flatpak_refine_appstream (GsFlatpak *self, } static gboolean -gs_flatpak_refine_app_unlocked (GsFlatpak *self, +gs_flatpak_refine_app_internal (GsFlatpak *self, GsApp *app, GsPluginRefineFlags flags, gboolean interactive, gboolean force_state_update, GHashTable *components_by_bundle, - GRWLockReaderLocker **locker, + XbSilo *silo, + const gchar *silo_filename, + GHashTable *silo_installed_by_desktopid, GCancellable *cancellable, GError **error) { @@ -3440,13 +3444,9 @@ gs_flatpak_refine_app_unlocked (GsFlatpak *self, if (gs_app_get_bundle_kind (app) != AS_BUNDLE_KIND_FLATPAK) return TRUE; - g_clear_pointer (locker, g_rw_lock_reader_locker_free); - - 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, components_by_bundle, interactive, cancellable, error)) + if (!gs_flatpak_refine_appstream (self, app, silo, silo_filename, silo_installed_by_desktopid, + flags, components_by_bundle, interactive, cancellable, error)) return FALSE; /* AppStream sets the source to appname/arch/branch */ @@ -3456,7 +3456,7 @@ gs_flatpak_refine_app_unlocked (GsFlatpak *self, } /* check the installed state */ - if (!gs_flatpak_refine_app_state_unlocked (self, app, interactive, force_state_update, cancellable, error)) { + if (!gs_flatpak_refine_app_state_internal (self, app, interactive, force_state_update, cancellable, error)) { g_prefix_error (error, "failed to get state: "); return FALSE; } @@ -3473,7 +3473,8 @@ gs_flatpak_refine_app_unlocked (GsFlatpak *self, /* if the state was changed, perhaps set the version from the release */ if (old_state != gs_app_get_state (app)) { - if (!gs_flatpak_refine_appstream (self, app, self->silo, flags, components_by_bundle, interactive, cancellable, error)) + if (!gs_flatpak_refine_appstream (self, app, silo, silo_filename, silo_installed_by_desktopid, + flags, components_by_bundle, interactive, cancellable, error)) return FALSE; } @@ -3567,11 +3568,16 @@ gs_flatpak_refine_addons (GsFlatpak *self, gboolean interactive, GCancellable *cancellable) { - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; + g_autofree gchar *silo_filename = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; g_autoptr(GsAppList) addons = NULL; g_autoptr(GString) errors = NULL; guint ii, sz; + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, &silo_filename, &silo_installed_by_desktopid, cancellable, NULL)) + return; + addons = gs_app_dup_addons (parent_app); sz = addons ? gs_app_list_length (addons) : 0; @@ -3582,7 +3588,8 @@ gs_flatpak_refine_addons (GsFlatpak *self, if (state != gs_app_get_state (addon)) continue; - if (!gs_flatpak_refine_app_unlocked (self, addon, flags, interactive, TRUE, NULL, &locker, cancellable, &local_error)) { + if (!gs_flatpak_refine_app_internal (self, addon, flags, interactive, TRUE, NULL, silo, silo_filename, + silo_installed_by_desktopid, cancellable, &local_error)) { if (errors) g_string_append_c (errors, '\n'); else @@ -3614,13 +3621,16 @@ gs_flatpak_refine_app (GsFlatpak *self, GCancellable *cancellable, GError **error) { - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; + g_autofree gchar *silo_filename = NULL; /* ensure valid */ - if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, &silo_filename, &silo_installed_by_desktopid, cancellable, error)) return FALSE; - return gs_flatpak_refine_app_unlocked (self, app, flags, interactive, force_state_update, NULL, &locker, cancellable, error); + return gs_flatpak_refine_app_internal (self, app, flags, interactive, force_state_update, NULL, + silo, silo_filename, silo_installed_by_desktopid, cancellable, error); } gboolean @@ -3634,7 +3644,9 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, const gchar *id; GPtrArray* components = NULL; g_autoptr(GError) error_local = NULL; - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; + g_autofree gchar *silo_filename = NULL; GS_PROFILER_BEGIN_SCOPED (FlatpakRefineWildcard, "Flatpak (refine wildcard)", NULL); @@ -3643,7 +3655,8 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, if (id == NULL) return TRUE; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + silo = gs_flatpak_ref_silo (self, interactive, &silo_filename, &silo_installed_by_desktopid, cancellable, error); + if (silo == NULL) return FALSE; GS_PROFILER_BEGIN_SCOPED (FlatpakRefineWildcardQuerySilo, "Flatpak (query silo)", NULL); @@ -3653,7 +3666,7 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, } else { g_autoptr(GPtrArray) components_with_id = NULL; *inout_components_by_id = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_ptr_array_unref); - components_with_id = xb_silo_query (self->silo, "components/component/id", 0, &error_local); + components_with_id = xb_silo_query (silo, "components/component/id", 0, &error_local); if (components_with_id == NULL) { if (g_error_matches (error_local, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) return TRUE; @@ -3686,7 +3699,7 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, g_autoptr(GPtrArray) bundles = NULL; *inout_components_by_bundle = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); - bundles = xb_silo_query (self->silo, "/components/component/bundle[@type='flatpak']", 0, NULL); + bundles = xb_silo_query (silo, "/components/component/bundle[@type='flatpak']", 0, NULL); for (guint b = 0; bundles != NULL && b < bundles->len; b++) { XbNode *bundle_node = g_ptr_array_index (bundles, b); g_autoptr(XbNode) component_node = xb_node_get_parent (bundle_node); @@ -3709,7 +3722,7 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, g_autoptr(GsApp) new = NULL; GS_PROFILER_BEGIN_SCOPED (FlatpakRefineWildcardCreateAppstreamApp, "Flatpak (create Appstream app)", NULL); - new = gs_appstream_create_app (self->plugin, self->silo, component, self->silo_filename ? self->silo_filename : "", + new = gs_appstream_create_app (self->plugin, silo, component, silo_filename ? silo_filename : "", self->scope, error); GS_PROFILER_END_SCOPED (FlatpakRefineWildcardCreateAppstreamApp); @@ -3761,7 +3774,8 @@ gs_flatpak_refine_wildcard (GsFlatpak *self, GsApp *app, g_debug ("Failed to get ref info for '%s' from wildcard '%s', skipping it...", gs_app_get_id (new), id); } else { GS_PROFILER_BEGIN_SCOPED (FlatpakRefineWildcardRefineNewApp, "Flatpak (refine new app)", NULL); - if (!gs_flatpak_refine_app_unlocked (self, new, refine_flags, interactive, FALSE, *inout_components_by_bundle, &locker, cancellable, error)) + if (!gs_flatpak_refine_app_internal (self, new, refine_flags, interactive, FALSE, *inout_components_by_bundle, + silo, silo_filename, silo_installed_by_desktopid, cancellable, error)) return FALSE; GS_PROFILER_END_SCOPED (FlatpakRefineWildcardRefineNewApp); @@ -3894,10 +3908,18 @@ gs_flatpak_file_to_app_bundle (GsFlatpak *self, /* load AppStream */ appstream_gz = flatpak_bundle_ref_get_appstream (xref_bundle); if (appstream_gz != NULL) { + g_autofree gchar *silo_filename = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; + g_autoptr(XbSilo) tmp_silo = NULL; + + tmp_silo = gs_flatpak_ref_silo (self, interactive, &silo_filename, &silo_installed_by_desktopid, cancellable, error); + if (tmp_silo == NULL) + return NULL; if (!gs_flatpak_refine_appstream_from_bytes (self, app, NULL, NULL, appstream_gz, GS_PLUGIN_REFINE_FLAGS_REQUIRE_ID, interactive, + silo_filename, silo_installed_by_desktopid, cancellable, error)) return NULL; } else { @@ -3995,6 +4017,9 @@ gs_flatpak_file_to_app_ref (GsFlatpak *self, g_autoptr(GsApp) app = NULL; g_autoptr(XbBuilder) builder = xb_builder_new (); g_autoptr(XbSilo) silo = NULL; + g_autoptr(XbSilo) tmp_silo = NULL; + g_autoptr(GHashTable) silo_installed_by_desktopid = NULL; + g_autofree gchar *silo_filename = NULL; g_autofree gchar *origin_url = NULL; g_autofree gchar *ref_comment = NULL; g_autofree gchar *ref_description = NULL; @@ -4274,8 +4299,12 @@ gs_flatpak_file_to_app_ref (GsFlatpak *self, g_debug ("showing AppStream data: %s", xml); } + tmp_silo = gs_flatpak_ref_silo (self, interactive, &silo_filename, &silo_installed_by_desktopid, cancellable, error); + if (tmp_silo == NULL) + return NULL; + /* get extra AppStream data if available */ - if (!gs_flatpak_refine_appstream (self, app, silo, + if (!gs_flatpak_refine_appstream (self, app, silo, silo_filename, silo_installed_by_desktopid, GS_PLUGIN_REFINE_FLAGS_MASK, NULL, interactive, @@ -4296,17 +4325,16 @@ gs_flatpak_search (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; g_autoptr(GMutexLocker) app_silo_locker = NULL; g_autoptr(GPtrArray) silos_to_remove = g_ptr_array_new (); + g_autoptr(XbSilo) silo = NULL; GHashTableIter iter; gpointer key, value; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_search (self->plugin, self->silo, values, list_tmp, - cancellable, error)) + if (!gs_appstream_search (self->plugin, silo, values, list_tmp, cancellable, error)) return FALSE; gs_flatpak_ensure_remote_title (self, interactive, cancellable); @@ -4353,8 +4381,8 @@ gs_flatpak_search (GsFlatpak *self, } for (guint i = 0; i < silos_to_remove->len; i++) { - const char *silo = g_ptr_array_index (silos_to_remove, i); - g_hash_table_remove (self->app_silos, silo); + const char *app_ref = g_ptr_array_index (silos_to_remove, i); + g_hash_table_remove (self->app_silos, app_ref); } return TRUE; @@ -4369,17 +4397,16 @@ gs_flatpak_search_developer_apps (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; g_autoptr(GMutexLocker) app_silo_locker = NULL; g_autoptr(GPtrArray) silos_to_remove = g_ptr_array_new (); + g_autoptr(XbSilo) silo = NULL; GHashTableIter iter; gpointer key, value; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_search_developer_apps (self->plugin, self->silo, values, list_tmp, - cancellable, error)) + if (!gs_appstream_search_developer_apps (self->plugin, silo, values, list_tmp, cancellable, error)) return FALSE; gs_flatpak_ensure_remote_title (self, interactive, cancellable); @@ -4426,8 +4453,8 @@ gs_flatpak_search_developer_apps (GsFlatpak *self, } for (guint i = 0; i < silos_to_remove->len; i++) { - const char *silo = g_ptr_array_index (silos_to_remove, i); - g_hash_table_remove (self->app_silos, silo); + const char *app_ref = g_ptr_array_index (silos_to_remove, i); + g_hash_table_remove (self->app_silos, app_ref); } return TRUE; @@ -4441,14 +4468,12 @@ gs_flatpak_add_category_apps (GsFlatpak *self, GCancellable *cancellable, GError **error) { - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - return gs_appstream_add_category_apps (self->plugin, self->silo, - category, list, - cancellable, error); + return gs_appstream_add_category_apps (self->plugin, silo, category, list, cancellable, error); } gboolean @@ -4458,12 +4483,12 @@ gs_flatpak_refine_category_sizes (GsFlatpak *self, GCancellable *cancellable, GError **error) { - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - return gs_appstream_refine_category_sizes (self->silo, list, cancellable, error); + return gs_appstream_refine_category_sizes (silo, list, cancellable, error); } gboolean @@ -4474,13 +4499,12 @@ gs_flatpak_add_popular (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_add_popular (self->silo, list_tmp, - cancellable, error)) + if (!gs_appstream_add_popular (silo, list_tmp, cancellable, error)) return FALSE; gs_app_list_add_list (list, list_tmp); @@ -4496,13 +4520,12 @@ gs_flatpak_add_featured (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_add_featured (self->silo, list_tmp, - cancellable, error)) + if (!gs_appstream_add_featured (silo, list_tmp, cancellable, error)) return FALSE; gs_app_list_add_list (list, list_tmp); @@ -4518,12 +4541,12 @@ gs_flatpak_add_deployment_featured (GsFlatpak *self, GCancellable *cancellable, GError **error) { - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - return gs_appstream_add_deployment_featured (self->silo, deployments, list, cancellable, error); + return gs_appstream_add_deployment_featured (silo, deployments, list, cancellable, error); } gboolean @@ -4535,13 +4558,12 @@ gs_flatpak_add_alternates (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_add_alternates (self->silo, app, list_tmp, - cancellable, error)) + if (!gs_appstream_add_alternates (silo, app, list_tmp, cancellable, error)) return FALSE; gs_app_list_add_list (list, list_tmp); @@ -4558,13 +4580,12 @@ gs_flatpak_add_recent (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_add_recent (self->plugin, self->silo, list_tmp, age, - cancellable, error)) + if (!gs_appstream_add_recent (self->plugin, silo, list_tmp, age, cancellable, error)) return FALSE; gs_flatpak_claim_app_list (self, list_tmp, interactive); @@ -4582,12 +4603,12 @@ gs_flatpak_url_to_app (GsFlatpak *self, GError **error) { g_autoptr(GsAppList) list_tmp = gs_app_list_new (); - g_autoptr(GRWLockReaderLocker) locker = NULL; + g_autoptr(XbSilo) silo = NULL; - if (!ensure_flatpak_silo_with_locker (self, &locker, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, &silo, NULL, NULL, cancellable, error)) return FALSE; - if (!gs_appstream_url_to_app (self->plugin, self->silo, list_tmp, url, cancellable, error)) + if (!gs_appstream_url_to_app (self->plugin, silo, list_tmp, url, cancellable, error)) return FALSE; gs_flatpak_claim_app_list (self, list_tmp, interactive); @@ -4638,10 +4659,8 @@ gs_flatpak_finalize (GObject *object) g_signal_handler_disconnect (self->monitor, self->changed_id); self->changed_id = 0; } - if (self->silo != NULL) - g_object_unref (self->silo); - if (self->monitor != NULL) - g_object_unref (self->monitor); + g_clear_object (&self->silo); + g_clear_object (&self->monitor); g_clear_pointer (&self->silo_filename, g_free); g_clear_pointer (&self->silo_installed_by_desktopid, g_hash_table_unref); @@ -4654,7 +4673,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_mutex_clear (&self->silo_lock); g_hash_table_unref (self->app_silos); g_mutex_clear (&self->app_silos_mutex); g_clear_pointer (&self->remote_title, g_hash_table_unref); @@ -4675,7 +4694,7 @@ 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->silo_lock); g_mutex_init (&self->installed_refs_mutex); self->installed_refs = NULL; -- GitLab