From e5f73b24c4950ec8e51f6970ad658d604baf6d24 Mon Sep 17 00:00:00 2001

From: Kalev Lember <klember@redhat.com>

Date: Fri, 14 Dec 2018 11:40:46 +0100

Subject: [PATCH 1/3] store: Add thread safe dup() functions for multithreaded

 clients

gnome-software spawns a new worker thread for each of its plugin

operations and occasionally this leads to issues where one thread is

reading from AsStore, and another one is changing it. There's also file

monitor callbacks in AsStore that run in a yet another thread and can

update internal data structures as well.

This leads to a situation where functions returning pointers to internal

data structures can't guarantee the data structure lifetime because

another thread might be changing it in the background.

To fix this, this commit adds new as_store_dup_apps() and

as_store_dup_apps_by_id_merge() functions that return a deep copy of the

internal structure, and updates existing "transfer container"

as_store_get_apps_by_id() to return a deep copy, instead just returning

a reffed container.

---

 libappstream-glib/as-self-test.c |  4 +-

 libappstream-glib/as-store.c     | 65 +++++++++++++++++++++++++++++++-

 libappstream-glib/as-store.h     |  3 ++

 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c

index d90af48..7fd7d75 100644

--- a/libappstream-glib/as-self-test.c

+++ b/libappstream-glib/as-self-test.c

@@ -3477,12 +3477,12 @@ as_test_store_flatpak_func (void)

 	AsApp *app;

 	AsFormat *format;

 	GError *error = NULL;

-	GPtrArray *apps;

 	gboolean ret;

 	g_autofree gchar *filename = NULL;

 	g_autofree gchar *filename_root = NULL;

 	g_autoptr(AsStore) store = NULL;

 	g_autoptr(GFile) file = NULL;

+	g_autoptr(GPtrArray) apps = NULL;

 	/* make throws us under a bus, yet again */

 	g_setenv ("AS_SELF_TEST_PREFIX_DELIM", "_", TRUE);

@@ -3503,7 +3503,7 @@ as_test_store_flatpak_func (void)

 	/* test extraction of symlink data */

 	g_assert_cmpstr (as_store_get_origin (store), ==, "flatpak");

 	g_assert_cmpint (as_store_get_size (store), ==, 1);

-	apps = as_store_get_apps (store);

+	apps = as_store_dup_apps (store);

 	g_assert_cmpint (apps->len, ==, 1);

 	app = g_ptr_array_index (apps, 0);

 	g_assert_cmpstr (as_app_get_id (app), ==, "flatpak:test.desktop");

diff --git a/libappstream-glib/as-store.c b/libappstream-glib/as-store.c

index d2075eb..59a7f1c 100644

--- a/libappstream-glib/as-store.c

+++ b/libappstream-glib/as-store.c

@@ -1,6 +1,7 @@

 /* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*-

  *

  * Copyright (C) 2013-2016 Richard Hughes <richard@hughsie.com>

+ * Copyright (C) 2015-2018 Kalev Lember <klember@redhat.com>

  *

  * Licensed under the GNU Lesser General Public License Version 2.1

  *

@@ -264,6 +265,22 @@ as_store_changed_uninhibit_cb (void *v)

 #define _cleanup_uninhibit_ __attribute__ ((cleanup(as_store_changed_uninhibit_cb)))

+static GPtrArray *

+_dup_app_array (GPtrArray *array)

+{

+	GPtrArray *array_dup;

+

+	g_return_val_if_fail (array != NULL, NULL);

+

+	array_dup = g_ptr_array_new_full (array->len, (GDestroyNotify) g_object_unref);

+	for (guint i = 0; i < array->len; i++) {

+		AsApp *app = g_ptr_array_index (array, i);

+		g_ptr_array_add (array_dup, g_object_ref (app));

+	}

+

+	return array_dup;

+}

+

 /**

  * as_store_add_filter:

  * @store: a #AsStore instance.

@@ -344,6 +361,27 @@ as_store_get_apps (AsStore *store)

 	return priv->array;

 }

+/**

+ * as_store_dup_apps:

+ * @store: a #AsStore instance.

+ *

+ * Gets an array of all the valid applications in the store.

+ *

+ * Returns: (element-type AsApp) (transfer container): an array

+ *

+ * Since: 0.7.15

+ **/

+GPtrArray *

+as_store_dup_apps (AsStore *store)

+{

+

+	AsStorePrivate *priv = GET_PRIVATE (store);

+

+	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	return _dup_app_array (priv->array);

+}

+

 /**

  * as_store_remove_all:

  * @store: a #AsStore instance.

@@ -471,7 +509,7 @@ as_store_get_apps_by_id (AsStore *store, const gchar *id)

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

 	apps = g_hash_table_lookup (priv->hash_id, id);

 	if (apps != NULL)

-		return g_ptr_array_ref (apps);

+		return _dup_app_array (apps);

 	return g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

 }

@@ -494,6 +532,31 @@ as_store_get_apps_by_id_merge (AsStore *store, const gchar *id)

 	return g_hash_table_lookup (priv->hash_merge_id, id);

 }

+/**

+ * as_store_dup_apps_by_id_merge:

+ * @store: a #AsStore instance.

+ * @id: the application full ID.

+ *

+ * Gets an array of all the merge applications that match a specific ID.

+ *

+ * Returns: (element-type AsApp) (transfer container): an array

+ *

+ * Since: 0.7.15

+ **/

+GPtrArray *

+as_store_dup_apps_by_id_merge (AsStore *store, const gchar *id)

+{

+	AsStorePrivate *priv = GET_PRIVATE (store);

+	GPtrArray *apps;

+

+	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	apps = g_hash_table_lookup (priv->hash_merge_id, id);

+	if (apps != NULL)

+		return _dup_app_array (apps);

+	return g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

+}

+

 /**

  * as_store_add_metadata_index:

  * @store: a #AsStore instance.

diff --git a/libappstream-glib/as-store.h b/libappstream-glib/as-store.h

index 3d8c749..b15aca4 100644

--- a/libappstream-glib/as-store.h

+++ b/libappstream-glib/as-store.h

@@ -209,10 +209,13 @@ void		 as_store_set_search_match	(AsStore	*store,

 guint16		 as_store_get_search_match	(AsStore	*store);

 void		 as_store_remove_all		(AsStore	*store);

 GPtrArray	*as_store_get_apps		(AsStore	*store);

+GPtrArray	*as_store_dup_apps		(AsStore	*store);

 GPtrArray	*as_store_get_apps_by_id	(AsStore	*store,

 						 const gchar	*id);

 GPtrArray	*as_store_get_apps_by_id_merge	(AsStore	*store,

 						 const gchar	*id);

+GPtrArray	*as_store_dup_apps_by_id_merge	(AsStore	*store,

+						 const gchar	*id);

 GPtrArray	*as_store_get_apps_by_metadata	(AsStore	*store,

 						 const gchar	*key,

 						 const gchar	*value);

-- 

2.19.1

From 0f79c943948abb62a54d0e54b1135ff89ebf5ab4 Mon Sep 17 00:00:00 2001

From: Kalev Lember <klember@redhat.com>

Date: Fri, 14 Dec 2018 11:52:19 +0100

Subject: [PATCH 2/3] store: Return deep copy in as_store_get_apps_by_metadata

This is strictly not necessary for making gnome-software's AsStore use

thread safe as gnome-software doesn't actually use this function, but

let's fix it anyway while I'm at it.

This also updates tests that test the performance of the function as the

deep copying makes it noticably slower (but that's fine because nothing

actually uses it and it's still reasonably fast).

---

 libappstream-glib/as-self-test.c | 2 +-

 libappstream-glib/as-store.c     | 2 +-

 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c

index 7fd7d75..981b790 100644

--- a/libappstream-glib/as-self-test.c

+++ b/libappstream-glib/as-self-test.c

@@ -4730,7 +4730,7 @@ static void

 as_test_store_metadata_index_func (void)

 {

 	GPtrArray *apps;

-	const guint repeats = 10000;

+	const guint repeats = 500;

 	guint i;

 	g_autoptr(AsStore) store = NULL;

 	g_autoptr(GTimer) timer = NULL;

diff --git a/libappstream-glib/as-store.c b/libappstream-glib/as-store.c

index 59a7f1c..018bdb5 100644

--- a/libappstream-glib/as-store.c

+++ b/libappstream-glib/as-store.c

@@ -473,7 +473,7 @@ as_store_get_apps_by_metadata (AsStore *store,

 		}

 		apps = g_hash_table_lookup (index, value);

 		if (apps != NULL)

-			return g_ptr_array_ref (apps);

+			return _dup_app_array (apps);

 		return g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

 	}

-- 

2.19.1

From 24913fb9ac74f671fdba7547049bcaa0899704ee Mon Sep 17 00:00:00 2001

From: Kalev Lember <klember@redhat.com>

Date: Fri, 14 Dec 2018 12:03:37 +0100

Subject: [PATCH 3/3] store: Add internal locking

This adds fine-grained locking around priv->array and various

priv-stored hash table use. There are more thread safe issues in

AsStore, but this should take care of those that cause frequent

gnome-software crashes in F29.

This may regress the perfomance a bit because it changes a few places to

keep a deep copy to simplify locking, but I haven't observed any visible

performance regressions in gnome-software. Also, gnome-software is in

the process of switching to libxmlb anyway so it shouldn't matter a

whole lot in the long run.

This patch takes care to make sure that locking is fine grained enough

so that we can be sure it doesn't lead into deadlocks, and also makes

sure that we never invoke any callbacks (signals) while locked to

prevent deadlocks when a client app calls back into AsStore code.

---

 libappstream-glib/as-store.c | 191 ++++++++++++++++++++++++++++-------

 1 file changed, 155 insertions(+), 36 deletions(-)

diff --git a/libappstream-glib/as-store.c b/libappstream-glib/as-store.c

index 018bdb5..0308585 100644

--- a/libappstream-glib/as-store.c

+++ b/libappstream-glib/as-store.c

@@ -68,6 +68,7 @@ typedef struct

 	GHashTable		*hash_merge_id;	/* of GPtrArray of AsApp{id} */

 	GHashTable		*hash_unique_id;	/* of AsApp{unique_id} */

 	GHashTable		*hash_pkgname;	/* of AsApp{pkgname} */

+	GMutex			 mutex;

 	AsMonitor		*monitor;

 	GHashTable		*metadata_indexes;	/* GHashTable{key} */

 	GHashTable		*appinfo_dirs;	/* GHashTable{path:AsStorePathData} */

@@ -140,6 +141,7 @@ as_store_finalize (GObject *object)

 	g_hash_table_unref (priv->metadata_indexes);

 	g_hash_table_unref (priv->appinfo_dirs);

 	g_hash_table_unref (priv->search_blacklist);

+	g_mutex_clear (&priv->mutex);

 	G_OBJECT_CLASS (as_store_parent_class)->finalize (object);

 }

@@ -339,7 +341,11 @@ guint

 as_store_get_size (AsStore *store)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_val_if_fail (AS_IS_STORE (store), 0);

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	return priv->array->len;

 }

@@ -357,7 +363,11 @@ GPtrArray *

 as_store_get_apps (AsStore *store)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	return priv->array;

 }

@@ -376,9 +386,11 @@ as_store_dup_apps (AsStore *store)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

 	return _dup_app_array (priv->array);

 }

@@ -394,7 +406,11 @@ void

 as_store_remove_all (AsStore *store)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_if_fail (AS_IS_STORE (store));

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	g_ptr_array_set_size (priv->array, 0);

 	g_hash_table_remove_all (priv->hash_id);

 	g_hash_table_remove_all (priv->hash_merge_id);

@@ -461,9 +477,12 @@ as_store_get_apps_by_metadata (AsStore *store,

 	GHashTable *index;

 	GPtrArray *apps;

 	guint i;

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	/* do we have this indexed? */

 	index = g_hash_table_lookup (priv->metadata_indexes, key);

 	if (index != NULL) {

@@ -506,7 +525,12 @@ as_store_get_apps_by_id (AsStore *store, const gchar *id)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	GPtrArray *apps;

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	apps = g_hash_table_lookup (priv->hash_id, id);

 	if (apps != NULL)

 		return _dup_app_array (apps);

@@ -528,7 +552,11 @@ GPtrArray *

 as_store_get_apps_by_id_merge (AsStore *store, const gchar *id)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	return g_hash_table_lookup (priv->hash_merge_id, id);

 }

@@ -548,9 +576,12 @@ as_store_dup_apps_by_id_merge (AsStore *store, const gchar *id)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	GPtrArray *apps;

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	apps = g_hash_table_lookup (priv->hash_merge_id, id);

 	if (apps != NULL)

 		return _dup_app_array (apps);

@@ -572,6 +603,12 @@ as_store_dup_apps_by_id_merge (AsStore *store, const gchar *id)

 void

 as_store_add_metadata_index (AsStore *store, const gchar *key)

 {

+	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

+

+	g_return_if_fail (AS_IS_STORE (store));

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	as_store_regen_metadata_index_key (store, key);

 }

@@ -594,7 +631,11 @@ as_store_get_app_by_id (AsStore *store, const gchar *id)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	GPtrArray *apps;

+	g_autoptr(GMutexLocker) locker = NULL;

+

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+

+	locker = g_mutex_locker_new (&priv->mutex);

 	apps = g_hash_table_lookup (priv->hash_id, id);

 	if (apps == NULL)

 		return NULL;

@@ -641,6 +682,7 @@ as_store_get_app_by_app (AsStore *store, AsApp *app)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	guint i;

+	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->mutex);

 	for (i = 0; i < priv->array->len; i++) {

 		AsApp *app_tmp = g_ptr_array_index (priv->array, i);

@@ -675,8 +717,10 @@ as_store_get_app_by_unique_id (AsStore *store,

 	g_return_val_if_fail (unique_id != NULL, NULL);

 	/* no globs */

-	if ((search_flags & AS_STORE_SEARCH_FLAG_USE_WILDCARDS) == 0)

+	if ((search_flags & AS_STORE_SEARCH_FLAG_USE_WILDCARDS) == 0) {

+		g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->mutex);

 		return g_hash_table_lookup (priv->hash_unique_id, unique_id);

+	}

 	/* create virtual app using scope/system/origin/kind/id/branch */

 	app_tmp = _as_app_new_from_unique_id (unique_id);

@@ -706,11 +750,14 @@ as_store_get_app_by_provide (AsStore *store, AsProvideKind kind, const gchar *va

 	guint i;

 	guint j;

 	GPtrArray *provides;

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

 	g_return_val_if_fail (kind != AS_PROVIDE_KIND_UNKNOWN, NULL);

 	g_return_val_if_fail (value != NULL, NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	/* find an application that provides something */

 	for (i = 0; i < priv->array->len; i++) {

 		app = g_ptr_array_index (priv->array, i);

@@ -744,11 +791,14 @@ AsApp *

 as_store_get_app_by_launchable (AsStore *store, AsLaunchableKind kind, const gchar *value)

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

 	g_return_val_if_fail (kind != AS_LAUNCHABLE_KIND_UNKNOWN, NULL);

 	g_return_val_if_fail (value != NULL, NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	for (guint i = 0; i < priv->array->len; i++) {

 		AsApp *app = g_ptr_array_index (priv->array, i);

 		GPtrArray *launchables = as_app_get_launchables (app);

@@ -782,11 +832,14 @@ as_store_get_apps_by_provide (AsStore *store, AsProvideKind kind, const gchar *v

 {

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	GPtrArray *apps = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

 	g_return_val_if_fail (kind != AS_PROVIDE_KIND_UNKNOWN, NULL);

 	g_return_val_if_fail (value != NULL, NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	/* find an application that provides something */

 	for (guint i = 0; i < priv->array->len; i++) {

 		AsApp *app = g_ptr_array_index (priv->array, i);

@@ -820,10 +873,13 @@ as_store_get_app_by_id_ignore_prefix (AsStore *store, const gchar *id)

 	AsApp *app;

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	guint i;

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

 	g_return_val_if_fail (id != NULL, NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	/* find an application that provides something */

 	for (i = 0; i < priv->array->len; i++) {

 		app = g_ptr_array_index (priv->array, i);

@@ -1017,9 +1073,12 @@ as_store_get_app_by_pkgname (AsStore *store, const gchar *pkgname)

 	AsApp *app;

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	guint i;

+	g_autoptr(GMutexLocker) locker = NULL;

 	g_return_val_if_fail (AS_IS_STORE (store), NULL);

+	locker = g_mutex_locker_new (&priv->mutex);

+

 	/* in most cases, we can use the cache */

 	app = g_hash_table_lookup (priv->hash_pkgname, pkgname);

 	if (app != NULL)

@@ -1059,6 +1118,7 @@ as_store_get_app_by_pkgnames (AsStore *store, gchar **pkgnames)

 	g_return_val_if_fail (pkgnames != NULL, NULL);

 	for (i = 0; pkgnames[i] != NULL; i++) {

+		g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&priv->mutex);

 		app = g_hash_table_lookup (priv->hash_pkgname, pkgnames[i]);

 		if (app != NULL)

 			return app;

@@ -1087,6 +1147,7 @@ as_store_remove_app (AsStore *store, AsApp *app)

 	g_signal_emit (store, signals[SIGNAL_APP_REMOVED], 0, app);

 	/* only remove this specific unique app */

+	g_mutex_lock (&priv->mutex);

 	apps = g_hash_table_lookup (priv->hash_id, as_app_get_id (app));

 	if (apps != NULL) {

 		g_ptr_array_remove (apps, app);

@@ -1100,6 +1161,7 @@ as_store_remove_app (AsStore *store, AsApp *app)

 	g_hash_table_remove (priv->hash_unique_id, as_app_get_unique_id (app));

 	g_ptr_array_remove (priv->array, app);

 	g_hash_table_remove_all (priv->metadata_indexes);

+	g_mutex_unlock (&priv->mutex);

 	/* removed */

 	as_store_perhaps_emit_changed (store, "remove-app");

@@ -1117,27 +1179,37 @@ as_store_remove_app (AsStore *store, AsApp *app)

 void

 as_store_remove_app_by_id (AsStore *store, const gchar *id)

 {

-	AsApp *app;

 	AsStorePrivate *priv = GET_PRIVATE (store);

-	guint i;

+	g_autoptr(GPtrArray) apps = NULL;

 	g_return_if_fail (AS_IS_STORE (store));

-	if (!g_hash_table_remove (priv->hash_id, id))

+	g_mutex_lock (&priv->mutex);

+	if (!g_hash_table_remove (priv->hash_id, id)) {

+		g_mutex_unlock (&priv->mutex);

 		return;

-	for (i = 0; i < priv->array->len; i++) {

-		app = g_ptr_array_index (priv->array, i);

+	}

+	g_mutex_unlock (&priv->mutex);

+

+	apps = as_store_dup_apps (store);

+	for (guint i = 0; i < apps->len; i++) {

+		AsApp *app = g_ptr_array_index (apps, i);

+

 		if (g_strcmp0 (id, as_app_get_id (app)) != 0)

 			continue;

 		/* emit before removal */

 		g_signal_emit (store, signals[SIGNAL_APP_REMOVED], 0, app);

+		g_mutex_lock (&priv->mutex);

 		g_ptr_array_remove (priv->array, app);

 		g_hash_table_remove (priv->hash_unique_id,

 				     as_app_get_unique_id (app));

+		g_mutex_unlock (&priv->mutex);

 	}

+	g_mutex_lock (&priv->mutex);

 	g_hash_table_remove_all (priv->metadata_indexes);

+	g_mutex_unlock (&priv->mutex);

 	/* removed */

 	as_store_perhaps_emit_changed (store, "remove-app-by-id");

@@ -1242,7 +1314,9 @@ as_store_add_app (AsStore *store, AsApp *app)

 	if (as_app_has_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX)) {

 		guint64 flags = AS_APP_SUBSUME_FLAG_MERGE;

 		AsAppMergeKind merge_kind = as_app_get_merge_kind (app);

+		g_autoptr(GPtrArray) apps_changed = NULL;

+		g_mutex_lock (&priv->mutex);

 		apps = g_hash_table_lookup (priv->hash_merge_id, id);

 		if (apps == NULL) {

 			apps = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

@@ -1254,11 +1328,16 @@ as_store_add_app (AsStore *store, AsApp *app)

 			 as_app_merge_kind_to_string (merge_kind),

 			 as_app_get_unique_id (app));

 		g_ptr_array_add (apps, g_object_ref (app));

+		g_mutex_unlock (&priv->mutex);

 		/* apply to existing components */

 		flags |= AS_APP_SUBSUME_FLAG_NO_OVERWRITE;

 		if (merge_kind == AS_APP_MERGE_KIND_REPLACE)

 			flags |= AS_APP_SUBSUME_FLAG_REPLACE;

+

+		apps_changed = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

+

+		g_mutex_lock (&priv->mutex);

 		for (i = 0; i < priv->array->len; i++) {

 			AsApp *app_tmp = g_ptr_array_index (priv->array, i);

 			if (g_strcmp0 (as_app_get_id (app_tmp), id) != 0)

@@ -1267,7 +1346,11 @@ as_store_add_app (AsStore *store, AsApp *app)

 				 as_app_merge_kind_to_string (merge_kind),

 				 id, as_app_get_unique_id (app_tmp));

 			as_app_subsume_full (app_tmp, app, flags);

-

+			g_ptr_array_add (apps_changed, g_object_ref (app_tmp));

+		}

+		g_mutex_unlock (&priv->mutex);

+		for (i = 0; i < apps_changed->len; i++) {

+			AsApp *app_tmp = g_ptr_array_index (apps_changed, i);

 			/* emit after changes have been made */

 			g_signal_emit (store, signals[SIGNAL_APP_CHANGED],

 				       0, app_tmp);

@@ -1276,6 +1359,7 @@ as_store_add_app (AsStore *store, AsApp *app)

 	}

 	/* is there any merge components to add to this app */

+	g_mutex_lock (&priv->mutex);

 	apps = g_hash_table_lookup (priv->hash_merge_id, id);

 	if (apps != NULL) {

 		for (i = 0; i < apps->len; i++) {

@@ -1292,14 +1376,17 @@ as_store_add_app (AsStore *store, AsApp *app)

 			as_app_subsume_full (app, app_tmp, flags);

 		}

 	}

+	g_mutex_unlock (&priv->mutex);

 	/* find the item */

 	if (priv->add_flags & AS_STORE_ADD_FLAG_USE_UNIQUE_ID) {

 		item = as_store_get_app_by_app (store, app);

 	} else {

+		g_mutex_lock (&priv->mutex);

 		apps = g_hash_table_lookup (priv->hash_id, id);

 		if (apps != NULL && apps->len > 0)

 			item = g_ptr_array_index (apps, 0);

+		g_mutex_unlock (&priv->mutex);

 	}

 	if (item != NULL) {

 		AsFormat *app_format = as_app_get_format_default (app);

@@ -1427,6 +1514,7 @@ as_store_add_app (AsStore *store, AsApp *app)

 	}

 	/* create hash of id:[apps] if required */

+	g_mutex_lock (&priv->mutex);

 	apps = g_hash_table_lookup (priv->hash_id, id);

 	if (apps == NULL) {

 		apps = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);

@@ -1448,6 +1536,7 @@ as_store_add_app (AsStore *store, AsApp *app)

 				     g_strdup (pkgname),

 				     g_object_ref (app));

 	}

+	g_mutex_unlock (&priv->mutex);

 	/* add helper objects */

 	as_app_set_stemmer (app, priv->stemmer);

@@ -1495,15 +1584,16 @@ as_store_match_addons (AsStore *store)

 	AsStorePrivate *priv = GET_PRIVATE (store);

 	guint i;

 	g_autoptr(AsProfileTask) ptask = NULL;

+	g_autoptr(GPtrArray) apps = NULL;

 	/* profile */

 	ptask = as_profile_start_literal (priv->profile, "AsStore:match-addons");

 	g_assert (ptask != NULL);

-	for (i = 0; i < priv->array->len; i++) {

-		AsApp *app = g_ptr_array_index (priv->array, i);

-		if (as_app_get_kind (app) != AS_APP_KIND_ADDON)

-			continue;

-		as_store_match_addons_app (store, app);

+	apps = as_store_dup_apps (store);

+	for (i = 0; i < apps->len; i++) {

+		AsApp *app = g_ptr_array_index (apps, i);

+		if (as_app_get_kind (app) == AS_APP_KIND_ADDON)

+			as_store_match_addons_app (store, app);

 	}

 }

@@ -1858,15 +1948,15 @@ static void

 as_store_remove_by_source_file (AsStore *store, const gchar *filename)

 {

 	AsApp *app;

-	GPtrArray *apps;

 	guint i;

 	const gchar *tmp;

 	_cleanup_uninhibit_ guint32 *tok = NULL;

+	g_autoptr(GPtrArray) apps = NULL;

 	g_autoptr(GPtrArray) ids = NULL;

 	/* find any applications in the store with this source file */

 	ids = g_ptr_array_new_with_free_func (g_free);

-	apps = as_store_get_apps (store);

+	apps = as_store_dup_apps (store);

 	for (i = 0; i < apps->len; i++) {

 		AsFormat *format;

 		app = g_ptr_array_index (apps, i);

@@ -1909,16 +1999,21 @@ as_store_watch_source_added (AsStore *store, const gchar *filename)

 	if (!g_file_test (filename, G_FILE_TEST_IS_REGULAR))

 		return;

-	/* we helpfully saved this */

 	dirname = g_path_get_dirname (filename);

 	g_debug ("parsing new file %s from %s", filename, dirname);

+

+	/* we helpfully saved this */

+	g_mutex_lock (&priv->mutex);

 	path_data = g_hash_table_lookup (priv->appinfo_dirs, filename);

 	if (path_data == NULL)

 		path_data = g_hash_table_lookup (priv->appinfo_dirs, dirname);

 	if (path_data == NULL) {

 		g_warning ("no path data for %s", dirname);

+		g_mutex_unlock (&priv->mutex);

 		return;

 	}

+	g_mutex_unlock (&priv->mutex);

+

 	file = g_file_new_for_path (filename);

 	/* Do not watch the file for changes: we're already watching its

 	 * parent directory */

@@ -2010,7 +2105,9 @@ as_store_add_path_data (AsStore *store,

 	}

 	/* check not already exists */

+	g_mutex_lock (&priv->mutex);

 	path_data = g_hash_table_lookup (priv->appinfo_dirs, path);

+	g_mutex_unlock (&priv->mutex);

 	if (path_data != NULL) {

 		if (path_data->scope != scope ||

 		    g_strcmp0 (path_data->arch, arch) != 0) {

@@ -2033,7 +2130,9 @@ as_store_add_path_data (AsStore *store,

 	path_data = g_slice_new0 (AsStorePathData);

 	path_data->scope = scope;

 	path_data->arch = g_strdup (arch);

+	g_mutex_lock (&priv->mutex);

 	g_hash_table_insert (priv->appinfo_dirs, g_strdup (path), path_data);

+	g_mutex_unlock (&priv->mutex);

 }

 static gboolean