[gnome-software/wip/kalev/gnome-3-22: 72/96] trivial: Add some more locking to GsApp

[Kalev Lember <klember src gnome org>]

commit cb3f8aa1255d3fa9c0ebb40317d641283d8ac147
Author: Richard Hughes <richard hughsie com>
Date:   Wed Feb 22 20:52:55 2017 +0000

    trivial: Add some more locking to GsApp
    
    To make this work reliably, add a function to get the unique_id that can be used
    when the mutex is taken, as the unique_id getter can actually generate the
    string at runtime.

 src/gs-app.c |   67 ++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 40 insertions(+), 27 deletions(-)
---
diff --git a/src/gs-app.c b/src/gs-app.c
index 956b083..05008fd 100644
--- a/src/gs-app.c
+++ b/src/gs-app.c
@@ -244,6 +244,29 @@ _as_app_quirk_flag_to_string (AsAppQuirk quirk)
        return NULL;
 }
 
+/* mutex must be held */
+static const gchar *
+gs_app_get_unique_id_unlocked (GsApp *app)
+{
+       /* invalid */
+       if (app->id == NULL)
+               return NULL;
+
+       /* hmm, do what we can */
+       if (app->unique_id == NULL || !app->unique_id_valid) {
+               g_debug ("autogenerating unique-id for %s", app->id);
+               g_free (app->unique_id);
+               app->unique_id = as_utils_unique_id_build (app->scope,
+                                                          app->bundle_kind,
+                                                          app->origin,
+                                                          app->kind,
+                                                          app->id,
+                                                          app->branch);
+               app->unique_id_valid = TRUE;
+       }
+       return app->unique_id;
+}
+
 /**
  * _as_app_quirk_to_string:
  * @quirk: a #AsAppQuirk
@@ -750,6 +773,7 @@ gs_app_set_state_recover (GsApp *app)
        gs_app_queue_notify (app, "state");
 }
 
+/* mutex must be held */
 static gboolean
 gs_app_set_state_internal (GsApp *app, AsAppState state)
 {
@@ -843,7 +867,7 @@ gs_app_set_state_internal (GsApp *app, AsAppState state)
        /* this state change was unexpected */
        if (!state_change_ok) {
                g_warning ("State change on %s from %s to %s is not OK",
-                          gs_app_get_unique_id (app),
+                          gs_app_get_unique_id_unlocked (app),
                           as_app_state_to_string (app->state),
                           as_app_state_to_string (state));
                return FALSE;
@@ -865,7 +889,7 @@ gs_app_set_state_internal (GsApp *app, AsAppState state)
        default:
                if (app->state_recover != state) {
                        g_debug ("%s non-transient state now %s",
-                                gs_app_get_unique_id (app),
+                                gs_app_get_unique_id_unlocked (app),
                                 as_app_state_to_string (state));
                        app->state_recover = state;
                }
@@ -892,13 +916,14 @@ gs_app_set_state_internal (GsApp *app, AsAppState state)
 void
 gs_app_set_progress (GsApp *app, guint percentage)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (app->progress == percentage)
                return;
        if (percentage > 100) {
                g_warning ("Cannot set '%u' as the progress for app '%s'. "
                           "Setting the maximum allowed value instead: 100.",
-                          percentage, gs_app_get_unique_id (app));
+                          percentage, gs_app_get_unique_id_unlocked (app));
                percentage = 100;
        }
        app->progress = percentage;
@@ -932,6 +957,7 @@ gs_app_set_progress (GsApp *app, guint percentage)
 void
 gs_app_set_state (GsApp *app, AsAppState state)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        if (gs_app_set_state_internal (app, state))
@@ -976,6 +1002,7 @@ void
 gs_app_set_kind (GsApp *app, AsAppKind kind)
 {
        gboolean state_change_ok = FALSE;
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
 
        g_return_if_fail (GS_IS_APP (app));
 
@@ -988,7 +1015,7 @@ gs_app_set_kind (GsApp *app, AsAppKind kind)
            kind == AS_APP_KIND_UNKNOWN) {
                g_warning ("automatically prevented from changing "
                           "kind on %s from %s to %s!",
-                          gs_app_get_unique_id (app),
+                          gs_app_get_unique_id_unlocked (app),
                           as_app_kind_to_string (app->kind),
                           as_app_kind_to_string (kind));
                return;
@@ -1043,24 +1070,7 @@ 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 */
-       if (app->id == NULL)
-               return NULL;
-
-       /* hmm, do what we can */
-       if (app->unique_id == NULL || !app->unique_id_valid) {
-               g_debug ("autogenerating unique-id for %s", app->id);
-               g_free (app->unique_id);
-               app->unique_id = as_utils_unique_id_build (app->scope,
-                                                          app->bundle_kind,
-                                                          app->origin,
-                                                          app->kind,
-                                                          app->id,
-                                                          app->branch);
-               app->unique_id_valid = TRUE;
-       }
-       return app->unique_id;
+       return gs_app_get_unique_id_unlocked (app);
 }
 
 /**
@@ -1195,6 +1205,7 @@ gs_app_add_source (GsApp *app, const gchar *source)
 {
        const gchar *tmp;
        guint i;
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
 
        g_return_if_fail (GS_IS_APP (app));
        g_return_if_fail (source != NULL);
@@ -2036,7 +2047,7 @@ gs_app_get_origin (GsApp *app)
 void
 gs_app_set_origin (GsApp *app, const gchar *origin)
 {
-       g_autoptr(GMutexLocker) locker = NULL;
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        /* same */
@@ -2047,12 +2058,11 @@ gs_app_set_origin (GsApp *app, const gchar *origin)
        if (app->origin != NULL && origin != NULL) {
                g_warning ("automatically prevented from changing "
                           "origin on %s from %s to %s!",
-                          gs_app_get_unique_id (app),
+                          gs_app_get_unique_id_unlocked (app),
                           app->origin, origin);
                return;
        }
 
-       locker = g_mutex_locker_new (&app->mutex);
        g_free (app->origin);
        app->origin = g_strdup (origin);
 
@@ -2372,7 +2382,7 @@ gs_app_set_management_plugin (GsApp *app, const gchar *management_plugin)
        if (gs_app_has_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX)) {
                g_warning ("plugins should not set the management plugin on "
                           "%s to %s -- create a new GsApp in refine()!",
-                          gs_app_get_unique_id (app),
+                          gs_app_get_unique_id_unlocked (app),
                           management_plugin);
                return;
        }
@@ -2385,7 +2395,7 @@ gs_app_set_management_plugin (GsApp *app, const gchar *management_plugin)
        if (app->management_plugin != NULL && management_plugin != NULL) {
                g_warning ("automatically prevented from changing "
                           "management plugin on %s from %s to %s!",
-                          gs_app_get_unique_id (app),
+                          gs_app_get_unique_id_unlocked (app),
                           app->management_plugin,
                           management_plugin);
                return;
@@ -2424,6 +2434,7 @@ gs_app_get_rating (GsApp *app)
 void
 gs_app_set_rating (GsApp *app, gint rating)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
        if (rating == app->rating)
                return;
@@ -3305,6 +3316,7 @@ gs_app_has_quirk (GsApp *app, AsAppQuirk quirk)
 void
 gs_app_add_quirk (GsApp *app, AsAppQuirk quirk)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        app->quirk |= quirk;
@@ -3323,6 +3335,7 @@ gs_app_add_quirk (GsApp *app, AsAppQuirk quirk)
 void
 gs_app_remove_quirk (GsApp *app, AsAppQuirk quirk)
 {
+       g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&app->mutex);
        g_return_if_fail (GS_IS_APP (app));
 
        app->quirk &= ~quirk;