[gegl] buffer: add gegl_tile_read_{lock,unlock}()



commit 301b798b4b2d4a113a547df7ba29e6621a06357a
Author: Ell <ell_se yahoo com>
Date:   Sat Jun 30 17:02:34 2018 -0400

    buffer: add gegl_tile_read_{lock,unlock}()
    
    Currently, we don't manage read access to tile data -- code that
    reads from a tile simply calls gegl_tile_get_data(), and assumes
    the returned pointer is valid throghout.  This, however, might not
    be the case in general, in a multithreaded context:
    
    Suppose tiles A and B share the same data.  Thread 1 begins reading
    tile A through the pointer returned by gegl_tile_get_data().
    Thread 2 then locks tile A for writing (intending to write to a
    portion of the tile that doesn't overlap the portion thread 1 is
    reading).  Since the tile's data is shared, thread 2 unclones the
    data before writing, and so the data thread 1 is reading is now
    solely owned by tile B.  At this point, two things can go wrong:
    a concurrent write operation to tile B can overwrite the data
    thread 1 is reading (which supposedly belongs to tile A), or,
    tile B can be destroyed, freeing the data thread 1 is reading.
    
    To fix this, we add the gegl_tile_read_lock() and
    gegl_tile_read_unlock() functions, which "lock" the tile for read
    access.  Code that reads tile data should call these functions
    before/after reading from the tile, if the read may happen in a
    multithreaded context.  Note that in spite of the functions' names,
    they don't actually block execution in general.  Instead, they make
    sure that if gegl_tile_lock() is called on a cloned tile while it's
    being read, the function will block until reading is done before
    uncloning the tile.
    
    One exception to the above are clones of the global empty tile, for
    which we avoid the said blocking altogether, since their data may
    never end up being owned by a single, mutable tile.
    
    Using gegl_tile_lock/unlock() before/after writing, and
    gegl_tile_read_lock/unlock() before/after reading, guarantees that
    the following combinations of concurrent tile operations are safe:
    
      - Read/read.
    
      - Read/duplicate.
    
      - Duplicate/duplicate.
    
      - Non-overlapping read/write.
    
      - Non-overlapping write/write.

 gegl/buffer/gegl-buffer-private.h     | 15 ++++--
 gegl/buffer/gegl-tile-handler-empty.c |  5 +-
 gegl/buffer/gegl-tile.c               | 88 ++++++++++++++++++++++++++---------
 gegl/buffer/gegl-tile.h               | 10 ++++
 4 files changed, 89 insertions(+), 29 deletions(-)
---
diff --git a/gegl/buffer/gegl-buffer-private.h b/gegl/buffer/gegl-buffer-private.h
index f43b76ecc..c6e7d316f 100644
--- a/gegl/buffer/gegl-buffer-private.h
+++ b/gegl/buffer/gegl-buffer-private.h
@@ -157,11 +157,16 @@ struct _GeglTile
   guint            stored_rev;  /* what revision was we when we from tile_storage?
                                    (currently set to 1 when loaded from disk */
 
-  gint             lock_count;  /* number of outstanding write locks */
-  guint            is_zero_tile:1; /* whether the tile data is fully zeroed
-                                    * (allowing for false negatives, but not
-                                    * false positives)
-                                    */
+  gint             lock_count;       /* number of outstanding write locks */
+  gint             read_lock_count;  /* number of outstanding read locks */
+  guint            is_zero_tile:1;   /* whether the tile data is fully zeroed
+                                      * (allowing for false negatives, but not
+                                      * false positives)
+                                      */
+  guint            is_global_tile:1; /* whether the tile data is global (and
+                                      * therefore can never be owned by a
+                                      * single mutable tile)
+                                      */
 
   gint             clone_state; /* tile clone/unclone state & spinlock */
   gint            *n_clones;    /* an array of two atomic counters, shared
diff --git a/gegl/buffer/gegl-tile-handler-empty.c b/gegl/buffer/gegl-tile-handler-empty.c
index ec1f40136..6338472f9 100644
--- a/gegl/buffer/gegl-tile-handler-empty.c
+++ b/gegl/buffer/gegl-tile-handler-empty.c
@@ -51,7 +51,7 @@ _new_empty_tile (const gint tile_size)
       tile = gegl_tile_new (tile_size);
 
       memset (gegl_tile_get_data (tile), 0x00, tile_size);
-      tile->is_zero_tile = 1;
+      tile->is_zero_tile = TRUE;
     }
   else
     {
@@ -65,7 +65,8 @@ _new_empty_tile (const gint tile_size)
           allocated_tile->data           = allocated_buffer;
           allocated_tile->destroy_notify = NULL;
           allocated_tile->size           = common_empty_size;
-          allocated_tile->is_zero_tile   = 1;
+          allocated_tile->is_zero_tile   = TRUE;
+          allocated_tile->is_global_tile = TRUE;
 
           /* avoid counting duplicates of the empty tile towards the total
            * cache size, both since this is unnecessary, and since they may
diff --git a/gegl/buffer/gegl-tile.c b/gegl/buffer/gegl-tile.c
index 933cbaeea..f878d3c16 100644
--- a/gegl/buffer/gegl-tile.c
+++ b/gegl/buffer/gegl-tile.c
@@ -98,14 +98,15 @@ void gegl_tile_unref (GeglTile *tile)
 static inline GeglTile *
 gegl_tile_new_bare_internal (void)
 {
-  GeglTile *tile     = g_slice_new0 (GeglTile);
-  tile->ref_count    = 1;
-  tile->tile_storage = NULL;
-  tile->stored_rev   = 1;
-  tile->rev          = 1;
-  tile->lock_count   = 0;
-  tile->clone_state  = CLONE_STATE_UNCLONED;
-  tile->data         = NULL;
+  GeglTile *tile        = g_slice_new0 (GeglTile);
+  tile->ref_count       = 1;
+  tile->tile_storage    = NULL;
+  tile->stored_rev      = 1;
+  tile->rev             = 1;
+  tile->lock_count      = 0;
+  tile->read_lock_count = 0;
+  tile->clone_state     = CLONE_STATE_UNCLONED;
+  tile->data            = NULL;
 
   return tile;
 }
@@ -133,13 +134,14 @@ gegl_tile_dup (GeglTile *src)
   g_warn_if_fail (src->lock_count == 0);
   g_warn_if_fail (! src->damage);
 
-  src->clone_state   = CLONE_STATE_CLONED;
+  src->clone_state     = CLONE_STATE_CLONED;
 
-  tile->data         = src->data;
-  tile->size         = src->size;
-  tile->is_zero_tile = src->is_zero_tile;
-  tile->clone_state  = CLONE_STATE_CLONED;
-  tile->n_clones     = src->n_clones;
+  tile->data           = src->data;
+  tile->size           = src->size;
+  tile->is_zero_tile   = src->is_zero_tile;
+  tile->is_global_tile = src->is_global_tile;
+  tile->clone_state    = CLONE_STATE_CLONED;
+  tile->n_clones       = src->n_clones;
 
   /* if the tile is not empty, mark it as dirty, since, even though the in-
    * memory tile data is shared with the source tile, the stored tile data is
@@ -182,6 +184,17 @@ gegl_tile_unclone (GeglTile *tile)
     {
       GeglTileHandlerCache *notify_cache = NULL;
       gboolean              cached;
+      gboolean              global;
+
+      global               = tile->is_global_tile;
+      tile->is_global_tile = FALSE;
+
+      if (! global)
+        {
+          while (! g_atomic_int_compare_and_exchange (&tile->read_lock_count,
+                                                      0,
+                                                      -1));
+        }
 
       cached = tile->tile_storage && tile->tile_storage->cache;
 
@@ -205,9 +218,8 @@ gegl_tile_unclone (GeglTile *tile)
                */
               *gegl_tile_n_clones (tile)        = 1;
               *gegl_tile_n_cached_clones (tile) = cached;
-              if (notify_cache)
-                gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
-              return;
+
+              goto end;
             }
 
           tile->n_clones     = gegl_calloc (INLINE_N_ELEMENTS_DATA_OFFSET +
@@ -228,9 +240,8 @@ gegl_tile_unclone (GeglTile *tile)
               gegl_free (buf);
               *gegl_tile_n_clones (tile)        = 1;
               *gegl_tile_n_cached_clones (tile) = cached;
-              if (notify_cache)
-                gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
-              return;
+
+              goto end;
             }
 
           tile->n_clones = (gint *) buf;
@@ -238,14 +249,20 @@ gegl_tile_unclone (GeglTile *tile)
 
       *gegl_tile_n_clones (tile)        = 1;
       *gegl_tile_n_cached_clones (tile) = cached;
-      tile->data      = (guchar *) tile->n_clones +
-                        INLINE_N_ELEMENTS_DATA_OFFSET;
+
+      g_atomic_pointer_set (&tile->data,
+                            (guchar *) tile->n_clones +
+                            INLINE_N_ELEMENTS_DATA_OFFSET);
 
       tile->destroy_notify      = (void*)&free_n_clones_directly;
       tile->destroy_notify_data = NULL;
 
+end:
       if (notify_cache)
         gegl_tile_handler_cache_tile_uncloned (notify_cache, tile);
+
+      if (! global)
+        g_atomic_int_set (&tile->read_lock_count, 0);
     }
 }
 
@@ -377,6 +394,33 @@ gegl_tile_unlock_no_void (GeglTile *tile)
     }
 }
 
+void
+gegl_tile_read_lock (GeglTile *tile)
+{
+  while (TRUE)
+    {
+      gint count = g_atomic_int_get (&tile->read_lock_count);
+
+      if (count < 0)
+        {
+          continue;
+        }
+
+      if (g_atomic_int_compare_and_exchange (&tile->read_lock_count,
+                                             count,
+                                             count + 1))
+        {
+          break;
+        }
+    }
+}
+
+void
+gegl_tile_read_unlock (GeglTile *tile)
+{
+  g_atomic_int_dec_and_test (&tile->read_lock_count);
+}
+
 void
 gegl_tile_mark_as_stored (GeglTile *tile)
 {
diff --git a/gegl/buffer/gegl-tile.h b/gegl/buffer/gegl-tile.h
index 27b8b6ea9..16e69d149 100644
--- a/gegl/buffer/gegl-tile.h
+++ b/gegl/buffer/gegl-tile.h
@@ -38,6 +38,16 @@ void         gegl_tile_lock           (GeglTile         *tile);
  */
 void         gegl_tile_unlock         (GeglTile         *tile);
 
+/* lock a tile for reading, this would allow reading from buffers
+ * later gotten with get_data()
+ */
+void         gegl_tile_read_lock      (GeglTile         *tile);
+
+/* unlock the tile notifying the tile that we're done reading
+ * the data.
+ */
+void         gegl_tile_read_unlock    (GeglTile         *tile);
+
 
 void         gegl_tile_mark_as_stored (GeglTile         *tile);
 gboolean     gegl_tile_is_stored      (GeglTile         *tile);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]