[gegl] buffer: improve thread-safety of tile-storage hot-tile manipulation

[N/A <ell src gnome org>]

commit 1e2f7cbba76a478ef7bb8286e38fd8f25600995f
Author: Ell <ell_se yahoo com>
Date:   Tue May 15 14:11:01 2018 -0400

    buffer: improve thread-safety of tile-storage hot-tile manipulation
    
    When inspecting the tile-storage's hot-tile, during a 1x1
    gegl_buffer_{get,set}(), there is a window during which another
    thread can replace the storage's hot-tile, dropping the current
    hot-tile's reference, allowing it to be trimmed out from the
    cache and freed, leaving us we a dangling pointer.
    
    Add gegl_tile_storage_{steal,try_steal}_hot_tile(), which
    atomically swap the storage's hot-tile with NULL, returning the
    original tile, and gegl_tile_storage_take_hot_tile(), which
    atomically sets the storage's hot-tile if it's NULL, and use these
    functions to safely query and replace the storage's hot-tile.
    
    This change does have a negative impact on the performance of 1x1
    gegl_buffer_{get,set}() (as well as NEAREST gegl_buffer_sample()),
    although it doesn't affect independent NEAREST samplers, since they
    maintain a local hot-tile.

 gegl/buffer/gegl-buffer-access.c      |   20 ++++++---
 gegl/buffer/gegl-buffer.c             |   14 ++-----
 gegl/buffer/gegl-tile-handler-cache.c |   17 +------
 gegl/buffer/gegl-tile-storage.c       |   71 +++++++++++++++++++++++++++++++++
 gegl/buffer/gegl-tile-storage.h       |    6 +++
 5 files changed, 97 insertions(+), 31 deletions(-)
---
diff --git a/gegl/buffer/gegl-buffer-access.c b/gegl/buffer/gegl-buffer-access.c
index 36fbc52..1707f52 100644
--- a/gegl/buffer/gegl-buffer-access.c
+++ b/gegl/buffer/gegl-buffer-access.c
@@ -122,20 +122,21 @@ gegl_buffer_get_pixel (GeglBuffer     *buffer,
     gint indice_x    = gegl_tile_indice (tiledx, tile_width);
     gint indice_y    = gegl_tile_indice (tiledy, tile_height);
 
-    GeglTile *tile = buffer->tile_storage->hot_tile;
+    GeglTile *tile = gegl_tile_storage_steal_hot_tile (buffer->tile_storage);
 
     if (!(tile &&
           tile->x == indice_x &&
           tile->y == indice_y))
       {
         if (gegl_config_threads()>1)
-         g_rec_mutex_lock (&buffer->tile_storage->mutex);
+          g_rec_mutex_lock (&buffer->tile_storage->mutex);
+
+        if (tile)
+          gegl_tile_unref (tile);
 
-        _gegl_buffer_drop_hot_tile (buffer);
         tile = gegl_tile_source_get_tile ((GeglTileSource *) (buffer),
                                           indice_x, indice_y,
                                           0);
-        buffer->tile_storage->hot_tile = tile;
 
         if (gegl_config_threads()>1)
           g_rec_mutex_unlock (&buffer->tile_storage->mutex);
@@ -158,6 +159,8 @@ gegl_buffer_get_pixel (GeglBuffer     *buffer,
           {
             memcpy (buf, tp, px_size);
           }
+
+        gegl_tile_storage_take_hot_tile (buffer->tile_storage, tile);
       }
   }
 }
@@ -187,7 +190,7 @@ __gegl_buffer_set_pixel (GeglBuffer     *buffer,
     gint indice_x    = gegl_tile_indice (tiledx, tile_width);
     gint indice_y    = gegl_tile_indice (tiledy, tile_height);
 
-    GeglTile *tile = buffer->tile_storage->hot_tile;
+    GeglTile *tile = gegl_tile_storage_steal_hot_tile (buffer->tile_storage);
     const Babl *fish = NULL;
     gint px_size;
 
@@ -208,11 +211,12 @@ __gegl_buffer_set_pixel (GeglBuffer     *buffer,
         if (gegl_config_threads()>1)
           g_rec_mutex_lock (&buffer->tile_storage->mutex);
 
-        _gegl_buffer_drop_hot_tile (buffer);
+        if (tile)
+          gegl_tile_unref (tile);
+
         tile = gegl_tile_source_get_tile ((GeglTileSource *) (buffer),
                                           indice_x, indice_y,
                                           0);
-        buffer->tile_storage->hot_tile = tile;
 
         if (gegl_config_threads()>1)
           g_rec_mutex_unlock (&buffer->tile_storage->mutex);
@@ -235,6 +239,8 @@ __gegl_buffer_set_pixel (GeglBuffer     *buffer,
           memcpy (tp, buf, px_size);
 
         gegl_tile_unlock (tile);
+
+        gegl_tile_storage_take_hot_tile (buffer->tile_storage, tile);
       }
   }
 
diff --git a/gegl/buffer/gegl-buffer.c b/gegl/buffer/gegl-buffer.c
index b4fa962..074d678 100644
--- a/gegl/buffer/gegl-buffer.c
+++ b/gegl/buffer/gegl-buffer.c
@@ -358,18 +358,12 @@ void
 _gegl_buffer_drop_hot_tile (GeglBuffer *buffer)
 {
   GeglTileStorage *storage = buffer->tile_storage;
+  GeglTile        *tile;
 
-  if (gegl_config_threads()>1)
-    g_rec_mutex_lock (&storage->mutex);
+  tile = gegl_tile_storage_steal_hot_tile (storage);
 
-  if (storage->hot_tile)
-    {
-      gegl_tile_unref (storage->hot_tile);
-      storage->hot_tile = NULL;
-    }
-
-  if (gegl_config_threads()>1)
-    g_rec_mutex_unlock (&storage->mutex);
+  if (tile)
+    gegl_tile_unref (tile);
 }
 
 static void
diff --git a/gegl/buffer/gegl-tile-handler-cache.c b/gegl/buffer/gegl-tile-handler-cache.c
index cf76668..e7cdff9 100644
--- a/gegl/buffer/gegl-tile-handler-cache.c
+++ b/gegl/buffer/gegl-tile-handler-cache.c
@@ -126,21 +126,10 @@ drop_hot_tile (GeglTile *tile)
 
   if (storage)
     {
-#if 0 /* the storage's mutex should already be locked at this point */
-      if (gegl_config_threads()>1)
-        g_rec_mutex_lock (&storage->mutex);
-#endif
+      tile = gegl_tile_storage_try_steal_hot_tile (storage, tile);
 
-      if (storage->hot_tile == tile)
-        {
-          gegl_tile_unref (storage->hot_tile);
-          storage->hot_tile = NULL;
-        }
-
-#if 0
-      if (gegl_config_threads()>1)
-        g_rec_mutex_unlock (&storage->mutex);
-#endif
+      if (tile)
+        gegl_tile_unref (tile);
     }
 }
 
diff --git a/gegl/buffer/gegl-tile-storage.c b/gegl/buffer/gegl-tile-storage.c
index 30a5ee3..9d162d1 100644
--- a/gegl/buffer/gegl-tile-storage.c
+++ b/gegl/buffer/gegl-tile-storage.c
@@ -26,6 +26,7 @@
 #include "gegl-tile-handler-empty.h"
 #include "gegl-tile-handler-zoom.h"
 #include "gegl-tile-handler-private.h"
+#include "gegl-config.h"
 
 
 G_DEFINE_TYPE (GeglTileStorage, gegl_tile_storage, GEGL_TYPE_TILE_HANDLER_CHAIN)
@@ -119,6 +120,76 @@ gegl_tile_storage_remove_handler (GeglTileStorage *tile_storage,
   gegl_tile_handler_chain_bind (chain);
 }
 
+GeglTile *
+gegl_tile_storage_steal_hot_tile (GeglTileStorage *tile_storage)
+{
+  GeglTile *tile;
+
+  if (gegl_config_threads()>1)
+    {
+      tile = g_atomic_pointer_get (&tile_storage->hot_tile);
+
+      if (tile &&
+          ! g_atomic_pointer_compare_and_exchange (&tile_storage->hot_tile,
+                                                   tile, NULL))
+        {
+          tile = NULL;
+        }
+    }
+  else
+    {
+      tile                   = tile_storage->hot_tile;
+      tile_storage->hot_tile = NULL;
+    }
+
+  return tile;
+}
+
+GeglTile *
+gegl_tile_storage_try_steal_hot_tile (GeglTileStorage *tile_storage,
+                                      GeglTile        *tile)
+{
+  if (gegl_config_threads()>1)
+    {
+      if (tile &&
+          ! g_atomic_pointer_compare_and_exchange (&tile_storage->hot_tile,
+                                                   tile, NULL))
+        {
+          tile = NULL;
+        }
+    }
+  else
+    {
+      if (tile_storage->hot_tile == tile)
+        tile_storage->hot_tile = NULL;
+      else
+        tile                   = NULL;
+    }
+
+  return tile;
+}
+
+void
+gegl_tile_storage_take_hot_tile (GeglTileStorage *tile_storage,
+                                 GeglTile        *tile)
+{
+  if (gegl_config_threads()>1)
+    {
+      if (! g_atomic_pointer_compare_and_exchange (&tile_storage->hot_tile,
+                                                   NULL, tile))
+        {
+          gegl_tile_unref (tile);
+        }
+    }
+  else
+    {
+      if (! tile_storage->hot_tile)
+        tile_storage->hot_tile = tile;
+      else
+        gegl_tile_unref (tile);
+    }
+}
+
 static void
 gegl_tile_storage_dispose (GObject *object)
 {
diff --git a/gegl/buffer/gegl-tile-storage.h b/gegl/buffer/gegl-tile-storage.h
index 2e256d5..1ce2515 100644
--- a/gegl/buffer/gegl-tile-storage.h
+++ b/gegl/buffer/gegl-tile-storage.h
@@ -65,4 +65,10 @@ gegl_tile_storage_new (GeglTileBackend *backend);
 void gegl_tile_storage_add_handler (GeglTileStorage *tile_storage, GeglTileHandler *handler);
 void gegl_tile_storage_remove_handler (GeglTileStorage *tile_storage, GeglTileHandler *handler);
 
+GeglTile * gegl_tile_storage_steal_hot_tile     (GeglTileStorage *tile_storage);
+GeglTile * gegl_tile_storage_try_steal_hot_tile (GeglTileStorage *tile_storage,
+                                                 GeglTile        *tile);
+void       gegl_tile_storage_take_hot_tile      (GeglTileStorage *tile_storage,
+                                                 GeglTile        *tile);
+
 #endif