[gnome-software: 1/3] gs-app: Return a reference from gs_app_dup_addons()
- From: Milan Crha <mcrha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software: 1/3] gs-app: Return a reference from gs_app_dup_addons()
- Date: Fri, 29 Apr 2022 09:27:05 +0000 (UTC)
commit 94bd8862f4be6de797bb77f128ef4c44f6afc51a
Author: Philip Withnall <pwithnall endlessos org>
Date: Wed Apr 13 15:53:12 2022 +0100
gs-app: Return a reference from gs_app_dup_addons()
Rename `gs_app_get_addons()` to `gs_app_dup_addons()`, access the
`GsApp`’s private data under lock, and atomically return a strong
reference to the list of addons.
This is one step to avoiding a race where the list of addons is modified
by one thread while being read by another. The second step (which will
happen in a following commit) is to change `gs_app_add_addon()` to
guarantee that the `GsAppList` returned by `gs_app_dup_addons()` is
immutable.
Signed-off-by: Philip Withnall <pwithnall endlessos org>
Helps: #1721
lib/gs-app-collation.h | 2 +-
lib/gs-app-list.c | 5 +++--
lib/gs-app.c | 14 ++++++++------
lib/gs-plugin-job-refine.c | 9 +++++----
lib/gs-plugin-loader.c | 22 +++++++++++-----------
plugins/dummy/gs-self-test.c | 5 +++--
plugins/flatpak/gs-flatpak.c | 10 +++++-----
plugins/flatpak/gs-plugin-flatpak.c | 4 ++--
plugins/packagekit/gs-plugin-packagekit.c | 16 +++++++++-------
src/gs-details-page.c | 12 ++++++------
10 files changed, 53 insertions(+), 46 deletions(-)
---
diff --git a/lib/gs-app-collation.h b/lib/gs-app-collation.h
index 892c75518..876ae5443 100644
--- a/lib/gs-app-collation.h
+++ b/lib/gs-app-collation.h
@@ -15,7 +15,7 @@
G_BEGIN_DECLS
GsAppList *gs_app_get_related (GsApp *app);
-GsAppList *gs_app_get_addons (GsApp *app);
+GsAppList *gs_app_dup_addons (GsApp *app);
GsAppList *gs_app_get_history (GsApp *app);
G_END_DECLS
diff --git a/lib/gs-app-list.c b/lib/gs-app-list.c
index 3122aa41d..1de573e1c 100644
--- a/lib/gs-app-list.c
+++ b/lib/gs-app-list.c
@@ -137,8 +137,9 @@ gs_app_list_add_watched_for_app (GsAppList *list, GPtrArray *apps, GsApp *app)
if (list->flags & GS_APP_LIST_FLAG_WATCH_APPS)
g_ptr_array_add (apps, app);
if (list->flags & GS_APP_LIST_FLAG_WATCH_APPS_ADDONS) {
- GsAppList *list2 = gs_app_get_addons (app);
- for (guint i = 0; i < gs_app_list_length (list2); i++) {
+ g_autoptr(GsAppList) list2 = gs_app_dup_addons (app);
+
+ for (guint i = 0; list2 != NULL && i < gs_app_list_length (list2); i++) {
GsApp *app2 = gs_app_list_index (list2, i);
g_ptr_array_add (apps, app2);
}
diff --git a/lib/gs-app.c b/lib/gs-app.c
index 6a30d0aef..52153c3e9 100644
--- a/lib/gs-app.c
+++ b/lib/gs-app.c
@@ -3866,21 +3866,23 @@ gs_app_set_metadata_variant (GsApp *app, const gchar *key, GVariant *value)
}
/**
- * gs_app_get_addons:
+ * gs_app_dup_addons:
* @app: a #GsApp
*
* Gets the list of addons for the application.
*
- * Returns: (transfer none): a list of addons
+ * Returns: (transfer full) (nullable): a list of addons, or %NULL if there are none
*
- * Since: 3.22
- **/
+ * Since: 43
+ */
GsAppList *
-gs_app_get_addons (GsApp *app)
+gs_app_dup_addons (GsApp *app)
{
GsAppPrivate *priv = gs_app_get_instance_private (app);
+ g_autoptr(GMutexLocker) locker = NULL;
g_return_val_if_fail (GS_IS_APP (app), NULL);
- return priv->addons;
+ locker = g_mutex_locker_new (&priv->mutex);
+ return (priv->addons != NULL) ? g_object_ref (priv->addons) : NULL;
}
/**
diff --git a/lib/gs-plugin-job-refine.c b/lib/gs-plugin-job-refine.c
index dabc8de42..f9618cd47 100644
--- a/lib/gs-plugin-job-refine.c
+++ b/lib/gs-plugin-job-refine.c
@@ -472,8 +472,9 @@ finish_refine_internal_op (GTask *task,
for (guint i = 0; i < gs_app_list_length (list); i++) {
GsApp *app = gs_app_list_index (list, i);
- GsAppList *addons = gs_app_get_addons (app);
- for (guint j = 0; j < gs_app_list_length (addons); j++) {
+ g_autoptr(GsAppList) addons = gs_app_dup_addons (app);
+
+ for (guint j = 0; addons != NULL && j < gs_app_list_length (addons); j++) {
GsApp *addon = gs_app_list_index (addons, j);
g_debug ("refining app %s addon %s",
gs_app_get_id (app),
@@ -665,13 +666,13 @@ run_cb (GObject *source_object,
for (guint i = 0; i < gs_app_list_length (result_list); i++) {
g_autoptr(GPtrArray) to_remove = g_ptr_array_new ();
GsApp *app = gs_app_list_index (result_list, i);
- GsAppList *addons = gs_app_get_addons (app);
+ g_autoptr(GsAppList) addons = gs_app_dup_addons (app);
/* find any apps with the same source */
const gchar *pkgname_parent = gs_app_get_source_default (app);
if (pkgname_parent == NULL)
continue;
- for (guint j = 0; j < gs_app_list_length (addons); j++) {
+ for (guint j = 0; addons != NULL && j < gs_app_list_length (addons); j++) {
GsApp *addon = gs_app_list_index (addons, j);
if (g_strcmp0 (gs_app_get_source_default (addon),
pkgname_parent) == 0) {
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index ab912992a..bb10a63c5 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -206,12 +206,12 @@ gs_plugin_loader_helper_new (GsPluginLoader *plugin_loader, GsPluginJob *plugin_
static void
reset_app_progress (GsApp *app)
{
- GsAppList *addons = gs_app_get_addons (app);
+ g_autoptr(GsAppList) addons = gs_app_dup_addons (app);
GsAppList *related = gs_app_get_related (app);
gs_app_set_progress (app, GS_APP_PROGRESS_UNKNOWN);
- for (guint i = 0; i < gs_app_list_length (addons); i++) {
+ for (guint i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *app_addons = gs_app_list_index (addons, i);
gs_app_set_progress (app_addons, GS_APP_PROGRESS_UNKNOWN);
}
@@ -1516,7 +1516,7 @@ save_install_queue (GsPluginLoader *plugin_loader)
static void
add_app_to_install_queue (GsPluginLoader *plugin_loader, GsApp *app)
{
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
guint i;
guint id;
@@ -1531,8 +1531,8 @@ add_app_to_install_queue (GsPluginLoader *plugin_loader, GsApp *app)
save_install_queue (plugin_loader);
/* recursively queue any addons */
- addons = gs_app_get_addons (app);
- for (i = 0; i < gs_app_list_length (addons); i++) {
+ addons = gs_app_dup_addons (app);
+ for (i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon = gs_app_list_index (addons, i);
if (gs_app_get_to_be_installed (addon))
add_app_to_install_queue (plugin_loader, addon);
@@ -1542,7 +1542,7 @@ add_app_to_install_queue (GsPluginLoader *plugin_loader, GsApp *app)
static gboolean
remove_app_from_install_queue (GsPluginLoader *plugin_loader, GsApp *app)
{
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
gboolean ret;
guint i;
guint id;
@@ -1558,8 +1558,8 @@ remove_app_from_install_queue (GsPluginLoader *plugin_loader, GsApp *app)
save_install_queue (plugin_loader);
/* recursively remove any queued addons */
- addons = gs_app_get_addons (app);
- for (i = 0; i < gs_app_list_length (addons); i++) {
+ addons = gs_app_dup_addons (app);
+ for (i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon = gs_app_list_index (addons, i);
remove_app_from_install_queue (plugin_loader, addon);
}
@@ -3280,9 +3280,9 @@ gs_plugin_loader_process_thread_cb (GTask *task,
/* unstage addons */
if (add_to_pending_array) {
- GsAppList *addons;
- addons = gs_app_get_addons (gs_plugin_job_get_app (helper->plugin_job));
- for (guint i = 0; i < gs_app_list_length (addons); i++) {
+ g_autoptr(GsAppList) addons = gs_app_dup_addons (gs_plugin_job_get_app (helper->plugin_job));
+
+ for (guint i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon = gs_app_list_index (addons, i);
if (gs_app_get_to_be_installed (addon))
gs_app_set_to_be_installed (addon, FALSE);
diff --git a/plugins/dummy/gs-self-test.c b/plugins/dummy/gs-self-test.c
index 9964f166c..121fac35f 100644
--- a/plugins/dummy/gs-self-test.c
+++ b/plugins/dummy/gs-self-test.c
@@ -352,7 +352,7 @@ gs_plugins_dummy_installed_func (GsPluginLoader *plugin_loader)
{
GsApp *app;
GsApp *addon;
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
g_autofree gchar *menu_path = NULL;
g_autoptr(GError) error = NULL;
g_autoptr(GsAppList) list = NULL;
@@ -405,7 +405,8 @@ gs_plugins_dummy_installed_func (GsPluginLoader *plugin_loader)
g_assert_cmpstr (menu_path, ==, "Create->Music Players");
/* check addon */
- addons = gs_app_get_addons (app);
+ addons = gs_app_dup_addons (app);
+ g_assert_nonnull (addons);
g_assert_cmpint (gs_app_list_length (addons), ==, 1);
addon = gs_app_list_index (addons, 0);
g_assert_cmpstr (gs_app_get_id (addon), ==, "zeus-spell.addon");
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 71d628035..4a113a005 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -2562,15 +2562,15 @@ gs_flatpak_prune_addons_list (GsFlatpak *self,
GCancellable *cancellable,
GError **error)
{
- GsAppList *addons_list;
+ g_autoptr(GsAppList) addons_list = NULL;
g_autoptr(GPtrArray) installed_related_refs = NULL;
g_autoptr(GPtrArray) remote_related_refs = NULL;
g_autofree gchar *ref = NULL;
FlatpakInstallation *installation = gs_flatpak_get_installation (self, interactive);
g_autoptr(GError) error_local = NULL;
- addons_list = gs_app_get_addons (app);
- if (gs_app_list_length (addons_list) == 0)
+ addons_list = gs_app_dup_addons (app);
+ if (addons_list == NULL || gs_app_list_length (addons_list) == 0)
return TRUE;
if (gs_app_get_origin (app) == NULL)
@@ -3282,11 +3282,11 @@ gs_flatpak_refine_addons (GsFlatpak *self,
gboolean interactive,
GCancellable *cancellable)
{
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
g_autoptr(GString) errors = NULL;
guint ii, sz;
- addons = gs_app_get_addons (parent_app);
+ addons = gs_app_dup_addons (parent_app);
sz = addons ? gs_app_list_length (addons) : 0;
for (ii = 0; ii < sz; ii++) {
diff --git a/plugins/flatpak/gs-plugin-flatpak.c b/plugins/flatpak/gs-plugin-flatpak.c
index 78b08b9c6..05849482a 100644
--- a/plugins/flatpak/gs-plugin-flatpak.c
+++ b/plugins/flatpak/gs-plugin-flatpak.c
@@ -1065,14 +1065,14 @@ gs_flatpak_cover_addons_in_transaction (GsPlugin *plugin,
GsApp *parent_app,
GsAppState state)
{
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
g_autoptr(GString) errors = NULL;
guint ii, sz;
g_return_if_fail (transaction != NULL);
g_return_if_fail (GS_IS_APP (parent_app));
- addons = gs_app_get_addons (parent_app);
+ addons = gs_app_dup_addons (parent_app);
sz = addons ? gs_app_list_length (addons) : 0;
for (ii = 0; ii < sz; ii++) {
diff --git a/plugins/packagekit/gs-plugin-packagekit.c b/plugins/packagekit/gs-plugin-packagekit.c
index 6ad23e988..0a80bfc9e 100644
--- a/plugins/packagekit/gs-plugin-packagekit.c
+++ b/plugins/packagekit/gs-plugin-packagekit.c
@@ -289,7 +289,9 @@ typedef gboolean (*GsAppFilterFunc) (GsApp *app);
/* The elements in the returned #GPtrArray reference memory from within the
* @apps list, so the array is only valid as long as @apps is not modified or
- * freed. The array is not NULL-terminated. */
+ * freed. The array is not NULL-terminated.
+ *
+ * If @apps is %NULL, that’s considered equivalent to an empty list. */
static GPtrArray *
app_list_get_package_ids (GsAppList *apps,
GsAppFilterFunc app_filter,
@@ -297,7 +299,7 @@ app_list_get_package_ids (GsAppList *apps,
{
g_autoptr(GPtrArray) list_package_ids = g_ptr_array_new_with_free_func (NULL);
- for (guint i = 0; i < gs_app_list_length (apps); i++) {
+ for (guint i = 0; apps != NULL && i < gs_app_list_length (apps); i++) {
GsApp *app = gs_app_list_index (apps, i);
GPtrArray *app_source_ids;
@@ -511,7 +513,7 @@ gs_plugin_app_install (GsPlugin *plugin,
GError **error)
{
GsPluginPackagekit *self = GS_PLUGIN_PACKAGEKIT (plugin);
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
GPtrArray *source_ids;
g_autoptr(GsPackagekitHelper) helper = gs_packagekit_helper_new (plugin);
const gchar *package_id;
@@ -598,7 +600,7 @@ gs_plugin_app_install (GsPlugin *plugin,
return FALSE;
}
- addons = gs_app_get_addons (app);
+ addons = gs_app_dup_addons (app);
array_package_ids = app_list_get_package_ids (addons,
gs_app_get_to_be_installed,
TRUE);
@@ -715,7 +717,7 @@ gs_plugin_app_remove (GsPlugin *plugin,
GsPluginPackagekit *self = GS_PLUGIN_PACKAGEKIT (plugin);
const gchar *package_id;
GPtrArray *source_ids;
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
g_autoptr(GsPackagekitHelper) helper = gs_packagekit_helper_new (plugin);
guint i;
guint cnt = 0;
@@ -771,8 +773,8 @@ gs_plugin_app_remove (GsPlugin *plugin,
}
/* Make sure addons' state is updated as well */
- addons = gs_app_get_addons (app);
- for (i = 0; i < gs_app_list_length (addons); i++) {
+ addons = gs_app_dup_addons (app);
+ for (i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon = gs_app_list_index (addons, i);
if (gs_app_get_state (addon) == GS_APP_STATE_INSTALLED) {
gs_app_set_state (addon, GS_APP_STATE_UNKNOWN);
diff --git a/src/gs-details-page.c b/src/gs-details-page.c
index dd1591087..413756348 100644
--- a/src/gs-details-page.c
+++ b/src/gs-details-page.c
@@ -1209,13 +1209,13 @@ static void gs_details_page_addon_remove_cb (GsAppAddonRow *row, gpointer user_d
static void
gs_details_page_refresh_addons (GsDetailsPage *self)
{
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
guint i, rows = 0;
gs_widget_remove_all (self->list_box_addons, (GsRemoveFunc) gtk_list_box_remove);
- addons = gs_app_get_addons (self->app);
- for (i = 0; i < gs_app_list_length (addons); i++) {
+ addons = gs_app_dup_addons (self->app);
+ for (i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon;
GtkWidget *row;
@@ -1978,12 +1978,12 @@ static void
gs_details_page_app_installed (GsPage *page, GsApp *app)
{
GsDetailsPage *self = GS_DETAILS_PAGE (page);
- GsAppList *addons;
+ g_autoptr(GsAppList) addons = NULL;
guint i;
/* if the app is just an addon, no need for a full refresh */
- addons = gs_app_get_addons (self->app);
- for (i = 0; i < gs_app_list_length (addons); i++) {
+ addons = gs_app_dup_addons (self->app);
+ for (i = 0; addons != NULL && i < gs_app_list_length (addons); i++) {
GsApp *addon;
addon = gs_app_list_index (addons, i);
if (addon == app)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]