[gnome-photos] base-item: Consolidate mutex_download; separate mutex for save_metadata

[Debarshi Ray <debarshir src gnome org>]

commit 352b256b33f448df7e81bae4857576ac82c2940b
Author: Debarshi Ray <debarshir gnome org>
Date:   Fri Jan 13 16:07:16 2017 +0100

    base-item: Consolidate mutex_download; separate mutex for save_metadata
    
    Having the locking scattered across various photos_base_item_download
    callers leaves us vulnerable to bugs. For example, it isn't obvious
    that photos_base_item_load_buffer should lock mutex_download, or that
    some implementations of metadata_add_shared might involve downloading.
    So, let's consolidate the locking to avoid this.
    
    We need a separate mutex for save_metadata to guard the section that
    updates the file.

 src/photos-base-item.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)
---
diff --git a/src/photos-base-item.c b/src/photos-base-item.c
index b79a320..70cc5ec 100644
--- a/src/photos-base-item.c
+++ b/src/photos-base-item.c
@@ -71,6 +71,7 @@ struct _PhotosBaseItemPrivate
   GeglNode *load;
   GeglProcessor *processor;
   GMutex mutex_download;
+  GMutex mutex_save_metadata;
   GQuark equipment;
   GQuark flash;
   GQuark orientation;
@@ -612,14 +613,9 @@ photos_base_item_download_in_thread_func (GTask *task,
                                           GCancellable *cancellable)
 {
   PhotosBaseItem *self = PHOTOS_BASE_ITEM (source_object);
-  PhotosBaseItemPrivate *priv;
   GError *error;
   gchar *path = NULL;
 
-  priv = photos_base_item_get_instance_private (self);
-
-  g_mutex_lock (&priv->mutex_download);
-
   error = NULL;
   path = photos_base_item_download (self, cancellable, &error);
   if (error != NULL)
@@ -632,7 +628,6 @@ photos_base_item_download_in_thread_func (GTask *task,
 
  out:
   g_free (path);
-  g_mutex_unlock (&priv->mutex_download);
 }
 
 
@@ -1268,14 +1263,9 @@ photos_base_item_load_buffer_in_thread_func (GTask *task,
                                              GCancellable *cancellable)
 {
   PhotosBaseItem *self = PHOTOS_BASE_ITEM (source_object);
-  PhotosBaseItemPrivate *priv;
   GeglBuffer *buffer = NULL;
   GError *error = NULL;
 
-  priv = photos_base_item_get_instance_private (self);
-
-  g_mutex_lock (&priv->mutex_download);
-
   buffer = photos_base_item_load_buffer (self, cancellable, &error);
   if (error != NULL)
     {
@@ -1287,7 +1277,6 @@ photos_base_item_load_buffer_in_thread_func (GTask *task,
 
  out:
   g_clear_object (&buffer);
-  g_mutex_unlock (&priv->mutex_download);
 }
 
 
@@ -1504,7 +1493,7 @@ photos_base_item_save_metadata_in_thread_func (GTask *task,
 
   priv = photos_base_item_get_instance_private (self);
 
-  g_mutex_lock (&priv->mutex_download);
+  g_mutex_lock (&priv->mutex_save_metadata);
 
   error = NULL;
   source_path = photos_base_item_download (self, cancellable, &error);
@@ -1535,7 +1524,7 @@ photos_base_item_save_metadata_in_thread_func (GTask *task,
   g_task_return_boolean (task, TRUE);
 
  out:
-  g_mutex_unlock (&priv->mutex_download);
+  g_mutex_unlock (&priv->mutex_save_metadata);
   g_clear_object (&metadata);
   g_free (export_path);
   g_free (source_path);
@@ -2369,6 +2358,7 @@ photos_base_item_finalize (GObject *object)
   g_free (priv->uri);
 
   g_mutex_clear (&priv->mutex_download);
+  g_mutex_clear (&priv->mutex_save_metadata);
 
   G_OBJECT_CLASS (photos_base_item_parent_class)->finalize (object);
 
@@ -2458,6 +2448,7 @@ photos_base_item_init (PhotosBaseItem *self)
   priv->cancellable = g_cancellable_new ();
 
   g_mutex_init (&priv->mutex_download);
+  g_mutex_init (&priv->mutex_save_metadata);
 
   priv->sel_cntrlr = photos_selection_controller_dup_singleton ();
 }
@@ -2691,8 +2682,17 @@ photos_base_item_destroy (PhotosBaseItem *self)
 gchar *
 photos_base_item_download (PhotosBaseItem *self, GCancellable *cancellable, GError **error)
 {
+  PhotosBaseItemPrivate *priv;
+  gchar *ret_val;
+
   g_return_val_if_fail (PHOTOS_IS_BASE_ITEM (self), NULL);
-  return PHOTOS_BASE_ITEM_GET_CLASS (self)->download (self, cancellable, error);
+  priv = photos_base_item_get_instance_private (self);
+
+  g_mutex_lock (&priv->mutex_download);
+  ret_val = PHOTOS_BASE_ITEM_GET_CLASS (self)->download (self, cancellable, error);
+  g_mutex_unlock (&priv->mutex_download);
+
+  return ret_val;
 }