[gnome-builder] unsaved-files: protect access to unsaved files array

[Christian Hergert <chergert src gnome org>]

commit 011afa82c2d9440b0c46adac62427b33db353915
Author: Christian Hergert <chergert redhat com>
Date:   Thu Jan 4 20:38:07 2018 -0800

    unsaved-files: protect access to unsaved files array
    
    There is a miss-behaving plugin that is not accessing these correctly
    *only* from the main thread. So lets protect against that as the first
    step to mitigate the miss-use.

 src/libide/buffers/ide-unsaved-files.c |  125 +++++++++++++++++++++++---------
 1 files changed, 91 insertions(+), 34 deletions(-)
---
diff --git a/src/libide/buffers/ide-unsaved-files.c b/src/libide/buffers/ide-unsaved-files.c
index 005eb16..d830d49 100644
--- a/src/libide/buffers/ide-unsaved-files.c
+++ b/src/libide/buffers/ide-unsaved-files.c
@@ -47,6 +47,7 @@ typedef struct
 struct _IdeUnsavedFiles
 {
   IdeObject  parent_instance;
+  GMutex     mutex;
   GPtrArray *unsaved_files;
   gint64     sequence;
 };
@@ -59,6 +60,10 @@ typedef struct
 
 G_DEFINE_TYPE (IdeUnsavedFiles, ide_unsaved_files, IDE_TYPE_OBJECT)
 
+static void ide_unsaved_files_update_locked (IdeUnsavedFiles *self,
+                                             GFile           *file,
+                                             GBytes          *content);
+
 gchar *
 get_drafts_directory (IdeContext *context)
 {
@@ -85,7 +90,7 @@ async_state_free (gpointer data)
 
   g_assert (IDE_IS_MAIN_THREAD ());
 
-  if (state)
+  if (state != NULL)
     {
       g_clear_pointer (&state->drafts_directory, g_free);
       g_clear_pointer (&state->unsaved_files, g_ptr_array_unref);
@@ -186,7 +191,6 @@ hash_uri (const gchar *uri)
 static gchar *
 get_buffers_dir (IdeContext *context)
 {
-  g_assert (IDE_IS_MAIN_THREAD ());
   g_assert (IDE_IS_CONTEXT (context));
 
   return ide_context_cache_filename (context, "buffers", NULL);
@@ -292,16 +296,18 @@ ide_unsaved_files_save_async (IdeUnsavedFiles     *self,
   g_assert (state->unsaved_files != NULL);
   g_assert (state->drafts_directory != NULL);
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < self->unsaved_files->len; i++)
     {
-      UnsavedFile *uf;
-      UnsavedFile *uf_copy;
+      UnsavedFile *uf = g_ptr_array_index (self->unsaved_files, i);
+      UnsavedFile *uf_copy = unsaved_file_copy (uf);
 
-      uf = g_ptr_array_index (self->unsaved_files, i);
-      uf_copy = unsaved_file_copy (uf);
       g_ptr_array_add (state->unsaved_files, uf_copy);
     }
 
+  g_mutex_unlock (&self->mutex);
+
   task = g_task_new (self, cancellable, callback, user_data);
   g_task_set_source_tag (task, ide_unsaved_files_save_async);
   g_task_set_priority (task, G_PRIORITY_LOW);
@@ -440,33 +446,36 @@ ide_unsaved_files_restore_async (IdeUnsavedFiles     *files,
 }
 
 gboolean
-ide_unsaved_files_restore_finish (IdeUnsavedFiles  *files,
+ide_unsaved_files_restore_finish (IdeUnsavedFiles  *self,
                                   GAsyncResult     *result,
                                   GError          **error)
 {
   AsyncState *state;
 
   g_return_val_if_fail (IDE_IS_MAIN_THREAD (), FALSE);
-  g_return_val_if_fail (IDE_IS_UNSAVED_FILES (files), FALSE);
+  g_return_val_if_fail (IDE_IS_UNSAVED_FILES (self), FALSE);
   g_return_val_if_fail (G_IS_TASK (result), FALSE);
 
   state = g_task_get_task_data (G_TASK (result));
   g_assert (state != NULL);
   g_assert (state->unsaved_files != NULL);
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < state->unsaved_files->len; i++)
     {
       const UnsavedFile *uf = g_ptr_array_index (state->unsaved_files, i);
-
-      ide_unsaved_files_update (files, uf->file, uf->content);
+      ide_unsaved_files_update_locked (self, uf->file, uf->content);
     }
 
+  g_mutex_unlock (&self->mutex);
+
   return g_task_propagate_boolean (G_TASK (result), error);
 }
 
 static void
-ide_unsaved_files_move_to_front (IdeUnsavedFiles *self,
-                                 guint            index)
+ide_unsaved_files_move_to_front_locked (IdeUnsavedFiles *self,
+                                        guint            index)
 {
   UnsavedFile *new_front;
   UnsavedFile *old_front;
@@ -490,8 +499,8 @@ ide_unsaved_files_move_to_front (IdeUnsavedFiles *self,
 }
 
 static void
-ide_unsaved_files_remove_draft (IdeUnsavedFiles *self,
-                                GFile           *file)
+ide_unsaved_files_remove_draft_locked (IdeUnsavedFiles *self,
+                                       GFile           *file)
 {
   IdeContext *context;
   g_autofree gchar *drafts_directory = NULL;
@@ -528,18 +537,22 @@ ide_unsaved_files_remove (IdeUnsavedFiles *self,
   g_return_if_fail (IDE_IS_UNSAVED_FILES (self));
   g_return_if_fail (G_IS_FILE (file));
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < self->unsaved_files->len; i++)
     {
       const UnsavedFile *unsaved = g_ptr_array_index (self->unsaved_files, i);
 
       if (g_file_equal (file, unsaved->file))
         {
-          ide_unsaved_files_remove_draft (self, file);
+          ide_unsaved_files_remove_draft_locked (self, file);
           g_ptr_array_remove_index_fast (self->unsaved_files, i);
-          IDE_EXIT;
+          break;
         }
     }
 
+  g_mutex_unlock (&self->mutex);
+
   IDE_EXIT;
 }
 
@@ -592,10 +605,10 @@ setup_tempfile (IdeContext  *context,
     *temp_path_out = g_steal_pointer (&tmpl_path);
 }
 
-void
-ide_unsaved_files_update (IdeUnsavedFiles *self,
-                          GFile           *file,
-                          GBytes          *content)
+static void
+ide_unsaved_files_update_locked (IdeUnsavedFiles *self,
+                                 GFile           *file,
+                                 GBytes          *content)
 {
   UnsavedFile *unsaved;
   IdeContext *context;
@@ -604,14 +617,14 @@ ide_unsaved_files_update (IdeUnsavedFiles *self,
   g_return_if_fail (IDE_IS_UNSAVED_FILES (self));
   g_return_if_fail (G_IS_FILE (file));
 
-  self->sequence++;
-
   if (content == NULL)
     {
       ide_unsaved_files_remove (self, file);
       return;
     }
 
+  self->sequence++;
+
   context = ide_object_get_context (IDE_OBJECT (self));
 
   for (guint i = 0; i < self->unsaved_files->len; i++)
@@ -633,8 +646,8 @@ ide_unsaved_files_update (IdeUnsavedFiles *self,
            * the beginning of the array to increase its chances of being the
            * first entry we check.
            */
-          if (i != 0)
-            ide_unsaved_files_move_to_front (self, i);
+          if (i > 0)
+            ide_unsaved_files_move_to_front_locked (self, i);
 
           return;
         }
@@ -649,6 +662,19 @@ ide_unsaved_files_update (IdeUnsavedFiles *self,
   g_ptr_array_add (self->unsaved_files, unsaved);
 }
 
+void
+ide_unsaved_files_update (IdeUnsavedFiles *self,
+                          GFile           *file,
+                          GBytes          *content)
+{
+  g_assert (IDE_IS_UNSAVED_FILES (self));
+  g_assert (G_IS_FILE (file));
+
+  g_mutex_lock (&self->mutex);
+  ide_unsaved_files_update_locked (self, file, content);
+  g_mutex_unlock (&self->mutex);
+}
+
 /**
  * ide_unsaved_files_to_array:
  *
@@ -674,17 +700,22 @@ ide_unsaved_files_to_array (IdeUnsavedFiles *self)
 
   ar = g_ptr_array_new_with_free_func ((GDestroyNotify)ide_unsaved_file_unref);
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < self->unsaved_files->len; i++)
     {
-      const UnsavedFile *uf;
-      IdeUnsavedFile *item;
-
-      uf = g_ptr_array_index (self->unsaved_files, i);
-      item = _ide_unsaved_file_new (uf->file, uf->content, uf->temp_path, uf->sequence);
+      const UnsavedFile *uf = g_ptr_array_index (self->unsaved_files, i);
+      g_autoptr(IdeUnsavedFile) item = NULL;
 
+      item = _ide_unsaved_file_new (uf->file,
+                                    uf->content,
+                                    uf->temp_path,
+                                    uf->sequence);
       g_ptr_array_add (ar, g_steal_pointer (&item));
     }
 
+  g_mutex_unlock (&self->mutex);
+
   return g_steal_pointer (&ar);
 }
 
@@ -692,19 +723,28 @@ gboolean
 ide_unsaved_files_contains (IdeUnsavedFiles *self,
                             GFile           *file)
 {
+  gboolean ret = FALSE;
+
   g_return_val_if_fail (IDE_IS_MAIN_THREAD (), FALSE);
   g_return_val_if_fail (IDE_IS_UNSAVED_FILES (self), FALSE);
   g_return_val_if_fail (G_IS_FILE (file), FALSE);
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < self->unsaved_files->len; i++)
     {
       UnsavedFile *uf = g_ptr_array_index (self->unsaved_files, i);
 
       if (g_file_equal (uf->file, file))
-        return TRUE;
+        {
+          ret = TRUE;
+          break;
+        }
     }
 
-  return FALSE;
+  g_mutex_unlock (&self->mutex);
+
+  return ret;
 }
 
 /**
@@ -714,6 +754,9 @@ ide_unsaved_files_contains (IdeUnsavedFiles *self,
  * file content is registered, %NULL is returned.
  *
  * Returns: (nullable) (transfer full): An #IdeUnsavedFile or %NULL.
+ *
+ * Thread safety: you may call this from any thread, as long as you
+ *   hold a reference to @self.
  */
 IdeUnsavedFile *
 ide_unsaved_files_get_unsaved_file (IdeUnsavedFiles *self,
@@ -723,7 +766,6 @@ ide_unsaved_files_get_unsaved_file (IdeUnsavedFiles *self,
 
   IDE_ENTRY;
 
-  g_return_val_if_fail (IDE_IS_MAIN_THREAD (), NULL);
   g_return_val_if_fail (IDE_IS_UNSAVED_FILES (self), NULL);
 
 #ifdef IDE_ENABLE_TRACE
@@ -733,6 +775,8 @@ ide_unsaved_files_get_unsaved_file (IdeUnsavedFiles *self,
   }
 #endif
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < self->unsaved_files->len; i++)
     {
       const UnsavedFile *uf = g_ptr_array_index (self->unsaved_files, i);
@@ -744,16 +788,23 @@ ide_unsaved_files_get_unsaved_file (IdeUnsavedFiles *self,
         }
     }
 
+  g_mutex_unlock (&self->mutex);
+
   IDE_RETURN (ret);
 }
 
 gint64
 ide_unsaved_files_get_sequence (IdeUnsavedFiles *self)
 {
-  g_return_val_if_fail (IDE_IS_MAIN_THREAD (), -1);
+  gint64 ret;
+
   g_return_val_if_fail (IDE_IS_UNSAVED_FILES (self), -1);
 
-  return self->sequence;
+  g_mutex_lock (&self->mutex);
+  ret = self->sequence;
+  g_mutex_unlock (&self->mutex);
+
+  return ret;
 }
 
 static void
@@ -795,6 +846,7 @@ ide_unsaved_files_finalize (GObject *object)
 
   g_assert (IDE_IS_MAIN_THREAD ());
 
+  g_mutex_clear (&self->mutex);
   g_clear_pointer (&self->unsaved_files, g_ptr_array_unref);
 
   G_OBJECT_CLASS (ide_unsaved_files_parent_class)->finalize (object);
@@ -814,6 +866,7 @@ ide_unsaved_files_class_init (IdeUnsavedFilesClass *klass)
 static void
 ide_unsaved_files_init (IdeUnsavedFiles *self)
 {
+  g_mutex_init (&self->mutex);
   self->unsaved_files = g_ptr_array_new_with_free_func (unsaved_file_free);
 }
 
@@ -827,6 +880,8 @@ ide_unsaved_files_clear (IdeUnsavedFiles *self)
 
   ar = ide_unsaved_files_to_array (self);
 
+  g_mutex_lock (&self->mutex);
+
   for (guint i = 0; i < ar->len; i++)
     {
       IdeUnsavedFile *uf = g_ptr_array_index (ar, i);
@@ -834,4 +889,6 @@ ide_unsaved_files_clear (IdeUnsavedFiles *self)
 
       ide_unsaved_files_remove (self, file);
     }
+
+  g_mutex_unlock (&self->mutex);
 }