[gnome-software] Add some thread locking in GsApp

[Richard Hughes <rhughes src gnome org>]

commit cfb0b2c0fc9c35295dae214879803d7994dc8b1a
Author: Richard Hughes <richard hughsie com>
Date:   Thu Dec 1 21:30:16 2016 +0000

    Add some thread locking in GsApp
    
    This should fix some hard-to-debug fairly common threading issues reported in
    the Fedora retrace server.

 src/gs-app.c       |   35 +++++++++++++++++++++++++++++++++++
 src/gs-self-test.c |   30 ++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 0 deletions(-)
---
diff --git a/src/gs-app.c b/src/gs-app.c
index b38f961..89b6b01 100644
--- a/src/gs-app.c
+++ b/src/gs-app.c
@@ -58,6 +58,7 @@ struct _GsApp
 {
        GObject                  parent_instance;
 
+       GMutex                   mutex;
        gchar                   *id;
        gchar                   *unique_id;
        gboolean                 unique_id_valid;
@@ -555,6 +556,7 @@ gs_app_get_id (GsApp *app)
 void
 gs_app_set_id (GsApp *app, const gchar *id)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->id);
        app->id = g_strdup (id);
@@ -972,6 +974,7 @@ gs_app_set_kind (GsApp *app, AsAppKind kind)
 const gchar *
 gs_app_get_unique_id (GsApp *app)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_val_if_fail (GS_IS_APP (app), NULL);
 
        /* invalid */
@@ -1005,6 +1008,7 @@ gs_app_get_unique_id (GsApp *app)
 void
 gs_app_set_unique_id (GsApp *app, const gchar *unique_id)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* check for sanity */
@@ -1046,6 +1050,7 @@ gs_app_get_name (GsApp *app)
 void
 gs_app_set_name (GsApp *app, GsAppQuality quality, const gchar *name)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* only save this if the data is sufficiently high quality */
@@ -1086,6 +1091,7 @@ gs_app_get_branch (GsApp *app)
 void
 gs_app_set_branch (GsApp *app, const gchar *branch)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->branch);
        app->branch = g_strdup (branch);
@@ -1171,6 +1177,7 @@ gs_app_get_sources (GsApp *app)
 void
 gs_app_set_sources (GsApp *app, GPtrArray *sources)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (app->sources != NULL)
                g_ptr_array_unref (app->sources);
@@ -1224,6 +1231,7 @@ gs_app_get_source_ids (GsApp *app)
 void
 gs_app_clear_source_ids (GsApp *app)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_ptr_array_set_size (app->source_ids, 0);
 }
@@ -1241,6 +1249,7 @@ gs_app_clear_source_ids (GsApp *app)
 void
 gs_app_set_source_ids (GsApp *app, GPtrArray *source_ids)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (app->source_ids != NULL)
                g_ptr_array_unref (app->source_ids);
@@ -1304,6 +1313,7 @@ gs_app_get_project_group (GsApp *app)
 void
 gs_app_set_project_group (GsApp *app, const gchar *project_group)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->project_group);
        app->project_group = g_strdup (project_group);
@@ -1356,6 +1366,7 @@ gs_app_get_icons (GsApp *app)
 void
 gs_app_add_icon (GsApp *app, AsIcon *icon)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (icon == NULL) {
                g_ptr_array_set_size (app->icons, 0);
@@ -1395,6 +1406,7 @@ gs_app_get_local_file (GsApp *app)
 void
 gs_app_set_local_file (GsApp *app, GFile *local_file)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_set_object (&app->local_file, local_file);
 }
@@ -1428,6 +1440,7 @@ gs_app_get_content_rating (GsApp *app)
 void
 gs_app_set_content_rating (GsApp *app, AsContentRating *content_rating)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_set_object (&app->content_rating, content_rating);
 }
@@ -1461,6 +1474,7 @@ gs_app_get_runtime (GsApp *app)
 void
 gs_app_set_runtime (GsApp *app, GsApp *runtime)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_set_object (&app->runtime, runtime);
 }
@@ -1477,6 +1491,7 @@ gs_app_set_runtime (GsApp *app, GsApp *runtime)
 void
 gs_app_set_pixbuf (GsApp *app, GdkPixbuf *pixbuf)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_set_object (&app->pixbuf, pixbuf);
 }
@@ -1636,6 +1651,7 @@ gs_app_get_version_ui (GsApp *app)
 void
 gs_app_set_version (GsApp *app, const gchar *version)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->version);
        app->version = g_strdup (version);
@@ -1673,6 +1689,7 @@ gs_app_get_summary (GsApp *app)
 void
 gs_app_set_summary (GsApp *app, GsAppQuality quality, const gchar *summary)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* only save this if the data is sufficiently high quality */
@@ -1714,6 +1731,7 @@ gs_app_get_description (GsApp *app)
 void
 gs_app_set_description (GsApp *app, GsAppQuality quality, const gchar *description)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* only save this if the data is sufficiently high quality */
@@ -1756,6 +1774,7 @@ gs_app_get_url (GsApp *app, AsUrlKind kind)
 void
 gs_app_set_url (GsApp *app, AsUrlKind kind, const gchar *url)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_hash_table_insert (app->urls,
                             g_strdup (as_url_kind_to_string (kind)),
@@ -1826,6 +1845,7 @@ gs_app_get_license_token_is_nonfree (const gchar *token)
 void
 gs_app_set_license (GsApp *app, GsAppQuality quality, const gchar *license)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        guint i;
        g_auto(GStrv) tokens = NULL;
 
@@ -1886,6 +1906,7 @@ gs_app_get_summary_missing (GsApp *app)
 void
 gs_app_set_summary_missing (GsApp *app, const gchar *summary_missing)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->summary_missing);
        app->summary_missing = g_strdup (summary_missing);
@@ -1923,6 +1944,7 @@ gs_app_get_menu_path (GsApp *app)
 void
 gs_app_set_menu_path (GsApp *app, gchar **menu_path)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_strfreev (app->menu_path);
        app->menu_path = g_strdupv (menu_path);
@@ -1957,6 +1979,7 @@ gs_app_get_origin (GsApp *app)
 void
 gs_app_set_origin (GsApp *app, const gchar *origin)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (origin == app->origin)
                return;
@@ -2007,6 +2030,7 @@ gs_app_get_origin_ui (GsApp *app)
 void
 gs_app_set_origin_ui (GsApp *app, const gchar *origin_ui)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (origin_ui == app->origin_ui)
                return;
@@ -2049,6 +2073,7 @@ gs_app_get_origin_hostname (GsApp *app)
 void
 gs_app_set_origin_hostname (GsApp *app, const gchar *origin_hostname)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_autoptr(SoupURI) uri = NULL;
        guint i;
        const gchar *prefixes[] = { "download.", "mirrors.", NULL };
@@ -2169,6 +2194,7 @@ gs_app_set_update_version_internal (GsApp *app, const gchar *update_version)
 void
 gs_app_set_update_version (GsApp *app, const gchar *update_version)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        gs_app_set_update_version_internal (app, update_version);
        gs_app_queue_notify (app, "version");
@@ -2203,6 +2229,7 @@ gs_app_get_update_details (GsApp *app)
 void
 gs_app_set_update_details (GsApp *app, const gchar *update_details)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_free (app->update_details);
        app->update_details = g_strdup (update_details);
@@ -2280,6 +2307,7 @@ gs_app_get_management_plugin (GsApp *app)
 void
 gs_app_set_management_plugin (GsApp *app, const gchar *management_plugin)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* plugins cannot adopt wildcard packages */
@@ -2372,6 +2400,7 @@ gs_app_get_review_ratings (GsApp *app)
 void
 gs_app_set_review_ratings (GsApp *app, GArray *review_ratings)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (app->review_ratings != NULL)
                g_array_unref (app->review_ratings);
@@ -2595,6 +2624,7 @@ gs_app_get_metadata_item (GsApp *app, const gchar *key)
 void
 gs_app_set_metadata (GsApp *app, const gchar *key, const gchar *value)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        const gchar *found;
        GString *str;
 
@@ -2881,6 +2911,7 @@ gs_app_has_category (GsApp *app, const gchar *category)
 void
 gs_app_set_categories (GsApp *app, GPtrArray *categories)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_return_if_fail (categories != NULL);
        if (app->categories != NULL)
@@ -2936,6 +2967,7 @@ gs_app_get_key_colors (GsApp *app)
 void
 gs_app_set_key_colors (GsApp *app, GPtrArray *key_colors)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_return_if_fail (key_colors != NULL);
        if (app->key_colors != NULL)
@@ -2989,6 +3021,7 @@ gs_app_get_keywords (GsApp *app)
 void
 gs_app_set_keywords (GsApp *app, GPtrArray *keywords)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        g_return_if_fail (keywords != NULL);
        if (app->keywords != NULL)
@@ -3392,6 +3425,7 @@ gs_app_finalize (GObject *object)
 {
        GsApp *app = GS_APP (object);
 
+       g_mutex_clear (&app->mutex);
        g_free (app->id);
        g_free (app->unique_id);
        g_free (app->branch);
@@ -3564,6 +3598,7 @@ gs_app_init (GsApp *app)
                                            g_str_equal,
                                            g_free,
                                            g_free);
+       g_mutex_init (&app->mutex);
 }
 
 /**
diff --git a/src/gs-self-test.c b/src/gs-self-test.c
index 3481c28..4cac23d 100644
--- a/src/gs-self-test.c
+++ b/src/gs-self-test.c
@@ -304,6 +304,35 @@ gs_plugin_func (void)
        g_object_unref (list);
 }
 
+static gpointer
+gs_app_thread_cb (gpointer data)
+{
+       GsApp *app = GS_APP (data);
+       for (guint i = 0; i < 10000; i++) {
+               g_assert_cmpstr (gs_app_get_unique_id (app), !=, NULL);
+               gs_app_set_branch (app, "master");
+               g_assert_cmpstr (gs_app_get_unique_id (app), !=, NULL);
+               gs_app_set_branch (app, "stable");
+       }
+       return NULL;
+}
+
+static void
+gs_app_thread_func (void)
+{
+       g_autoptr(GsApp) app = gs_app_new ("gimp.desktop");
+       g_autoptr(GThread) thread1 = NULL;
+       g_autoptr(GThread) thread2 = NULL;
+
+       /* try really hard to cause a threading problem */
+       g_setenv ("G_MESSAGES_DEBUG", "", TRUE);
+       thread1 = g_thread_new ("thread1", gs_app_thread_cb, app);
+       thread2 = g_thread_new ("thread2", gs_app_thread_cb, app);
+       g_thread_join (thread1);
+       g_thread_join (thread2);
+       g_setenv ("G_MESSAGES_DEBUG", "all", TRUE);
+}
+
 static void
 gs_app_unique_id_func (void)
 {
@@ -1467,6 +1496,7 @@ main (int argc, char **argv)
        g_test_add_func ("/gnome-software/os-release", gs_os_release_func);
        g_test_add_func ("/gnome-software/app", gs_app_func);
        g_test_add_func ("/gnome-software/app{unique-id}", gs_app_unique_id_func);
+       g_test_add_func ("/gnome-software/app{thread}", gs_app_thread_func);
        g_test_add_func ("/gnome-software/plugin", gs_plugin_func);
        g_test_add_func ("/gnome-software/plugin{global-cache}", gs_plugin_global_cache_func);
        g_test_add_func ("/gnome-software/auth{secret}", gs_auth_secret_func);