From 05db4c452cbf6d314be60b86524629ef22798adf Mon Sep 17 00:00:00 2001
From: Harry Jeffery <harry@exec64.co.uk>
Date: Mon, 4 Feb 2019 02:34:46 +0000
Subject: Make sources thread-safe

---
 src/backend_freeimage.c | 33 ++++++++++++++++++++++++++-------
 src/backend_libjpeg.c   | 23 ++++++++++++++++++-----
 src/backend_libpng.c    | 18 +++++++++++++++---
 src/backend_librsvg.c   | 19 ++++++++++++++++---
 src/backend_libtiff.c   | 23 ++++++++++++++++++-----
 src/source.h            | 16 ++++++++++++----
 6 files changed, 105 insertions(+), 27 deletions(-)

diff --git a/src/backend_freeimage.c b/src/backend_freeimage.c
index 8a24725..c0fb655 100644
--- a/src/backend_freeimage.c
+++ b/src/backend_freeimage.c
@@ -19,6 +19,7 @@ struct private {
 
 static void source_free(struct imv_source *src)
 {
+  pthread_mutex_lock(&src->busy);
   free(src->name);
   src->name = NULL;
 
@@ -42,6 +43,8 @@ static void source_free(struct imv_source *src)
   free(private);
   src->private = NULL;
 
+  pthread_mutex_unlock(&src->busy);
+  pthread_mutex_destroy(&src->busy);
   free(src);
 }
 
@@ -71,6 +74,7 @@ static void report_error(struct imv_source *src)
   msg.bitmap = NULL;
   msg.error = "Internal error";
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
@@ -89,11 +93,17 @@ static void send_bitmap(struct imv_source *src, FIBITMAP *fibitmap, int frametim
   msg.frametime = frametime;
   msg.error = NULL;
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
-static void first_frame(struct imv_source *src)
+static int first_frame(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   FIBITMAP *bmp = NULL;
 
   struct private *private = src->private;
@@ -113,12 +123,12 @@ static void first_frame(struct imv_source *src)
           /* flags */ GIF_LOAD256);
     } else {
       report_error(src);
-      return;
+      return -1;
     }
 
     if (!private->multibitmap) {
       report_error(src);
-      return;
+      return -1;
     }
 
     FIBITMAP *frame = FreeImage_LockPage(private->multibitmap, 0);
@@ -147,7 +157,7 @@ static void first_frame(struct imv_source *src)
     }
     if (!fibitmap) {
       report_error(src);
-      return;
+      return -1;
     }
     bmp = FreeImage_ConvertTo32Bits(fibitmap);
     FreeImage_Unload(fibitmap);
@@ -155,16 +165,22 @@ static void first_frame(struct imv_source *src)
 
   src->width = FreeImage_GetWidth(bmp);
   src->height = FreeImage_GetHeight(bmp);
-  send_bitmap(src, bmp, frametime);
   private->last_frame = bmp;
+  send_bitmap(src, bmp, frametime);
+  return 0;
 }
 
-static void next_frame(struct imv_source *src)
+static int next_frame(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   struct private *private = src->private;
   if (src->num_frames == 1) {
     send_bitmap(src, private->last_frame, 0);
-    return;
+    return 0;
   }
 
   FITAG *tag = NULL;
@@ -250,6 +266,7 @@ static void next_frame(struct imv_source *src)
   src->next_frame = (src->next_frame + 1) % src->num_frames;
 
   send_bitmap(src, private->last_frame, frametime);
+  return 0;
 }
 
 static enum backend_result open_path(const char *path, struct imv_source **src)
@@ -268,6 +285,7 @@ static enum backend_result open_path(const char *path, struct imv_source **src)
   source->width = 0;
   source->height = 0;
   source->num_frames = 0;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &first_frame;
   source->load_next_frame = &next_frame;
   source->free = &source_free;
@@ -299,6 +317,7 @@ static enum backend_result open_memory(void *data, size_t len, struct imv_source
   source->width = 0;
   source->height = 0;
   source->num_frames = 0;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &first_frame;
   source->load_next_frame = &next_frame;
   source->free = &source_free;
diff --git a/src/backend_libjpeg.c b/src/backend_libjpeg.c
index 3fde6c2..e83ddc3 100644
--- a/src/backend_libjpeg.c
+++ b/src/backend_libjpeg.c
@@ -21,6 +21,7 @@ struct private {
 
 static void source_free(struct imv_source *src)
 {
+  pthread_mutex_lock(&src->busy);
   free(src->name);
   src->name = NULL;
 
@@ -37,6 +38,8 @@ static void source_free(struct imv_source *src)
   free(src->private);
   src->private = NULL;
 
+  pthread_mutex_unlock(&src->busy);
+  pthread_mutex_destroy(&src->busy);
   free(src);
 }
 
@@ -64,6 +67,7 @@ static void report_error(struct imv_source *src)
   msg.bitmap = NULL;
   msg.error = "Internal error";
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
@@ -82,24 +86,31 @@ static void send_bitmap(struct imv_source *src, void *bitmap)
   msg.frametime = 0;
   msg.error = NULL;
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
-static void load_image(struct imv_source *src)
+static int load_image(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   struct private *private = src->private;
 
   void *bitmap = malloc(src->height * src->width * 4);
   int rcode = tjDecompress2(private->jpeg, private->data, private->len,
       bitmap, src->width, 0, src->height, TJPF_RGBA, TJFLAG_FASTDCT);
 
-  if (!rcode) {
-    send_bitmap(src, bitmap);
-  } else {
+  if (rcode) {
     free(bitmap);
     report_error(src);
-    return;
+    return -1;
   }
+
+  send_bitmap(src, bitmap);
+  return 0;
 }
 
 static enum backend_result open_path(const char *path, struct imv_source **src)
@@ -148,6 +159,7 @@ static enum backend_result open_path(const char *path, struct imv_source **src)
   source->height = height;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
@@ -187,6 +199,7 @@ static enum backend_result open_memory(void *data, size_t len, struct imv_source
   source->height = height;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
diff --git a/src/backend_libpng.c b/src/backend_libpng.c
index 2c3b1d7..3713ae8 100644
--- a/src/backend_libpng.c
+++ b/src/backend_libpng.c
@@ -18,6 +18,7 @@ struct private {
 
 static void source_free(struct imv_source *src)
 {
+  pthread_mutex_lock(&src->busy);
   free(src->name);
   src->name = NULL;
 
@@ -30,6 +31,8 @@ static void source_free(struct imv_source *src)
   free(src->private);
   src->private = NULL;
 
+  pthread_mutex_unlock(&src->busy);
+  pthread_mutex_destroy(&src->busy);
   free(src);
 }
 
@@ -57,6 +60,7 @@ static void report_error(struct imv_source *src)
   msg.bitmap = NULL;
   msg.error = "Internal error";
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
@@ -76,16 +80,22 @@ static void send_bitmap(struct imv_source *src, void *bitmap)
   msg.frametime = 0;
   msg.error = NULL;
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
-static void load_image(struct imv_source *src)
+static int load_image(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   struct private *private = src->private;
 
   if (setjmp(png_jmpbuf(private->png))) {
     report_error(src);
-    return;
+    return -1;
   }
 
   png_bytep *rows = alloca(sizeof(png_bytep) * src->height);
@@ -98,13 +108,14 @@ static void load_image(struct imv_source *src)
   if (setjmp(png_jmpbuf(private->png))) {
     free(rows[0]);
     report_error(src);
-    return;
+    return -1;
   }
 
   png_read_image(private->png, rows);
   fclose(private->file);
   private->file = NULL;
   send_bitmap(src, rows[0]);
+  return 0;
 }
 
 static enum backend_result open_path(const char *path, struct imv_source **src)
@@ -169,6 +180,7 @@ static enum backend_result open_path(const char *path, struct imv_source **src)
   source->height = png_get_image_height(private->png, private->info);
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
diff --git a/src/backend_librsvg.c b/src/backend_librsvg.c
index 083fb9c..c9a91ff 100644
--- a/src/backend_librsvg.c
+++ b/src/backend_librsvg.c
@@ -21,6 +21,7 @@ struct private {
 
 static void source_free(struct imv_source *src)
 {
+  pthread_mutex_lock(&src->busy);
   free(src->name);
   src->name = NULL;
 
@@ -28,6 +29,8 @@ static void source_free(struct imv_source *src)
   free(private);
   src->private = NULL;
 
+  pthread_mutex_unlock(&src->busy);
+  pthread_mutex_destroy(&src->busy);
   free(src);
 }
 
@@ -57,6 +60,7 @@ static void report_error(struct imv_source *src)
   msg.bitmap = NULL;
   msg.error = "Internal error";
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
@@ -76,11 +80,17 @@ static void send_bitmap(struct imv_source *src, GdkPixbuf *bitmap)
   msg.frametime = 0;
   msg.error = NULL;
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
-static void load_image(struct imv_source *src)
+static int load_image(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   RsvgHandle *handle = NULL;
   GError *error = NULL;
 
@@ -95,7 +105,7 @@ static void load_image(struct imv_source *src)
 
   if (!handle) {
     report_error(src);
-    return;
+    return -1;
   }
 
   RsvgDimensionData dim;
@@ -107,11 +117,12 @@ static void load_image(struct imv_source *src)
   if (!buf) {
     rsvg_handle_close(handle, &error);
     report_error(src);
-    return;
+    return -1;
   }
 
   rsvg_handle_close(handle, &error);
   send_bitmap(src, buf);
+  return 0;
 }
 
 static enum backend_result open_path(const char *path, struct imv_source **src)
@@ -141,6 +152,7 @@ static enum backend_result open_path(const char *path, struct imv_source **src)
   source->height = 1024;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
@@ -173,6 +185,7 @@ static enum backend_result open_memory(void *data, size_t len, struct imv_source
   source->height = 1024;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
diff --git a/src/backend_libtiff.c b/src/backend_libtiff.c
index fcfdc7f..e837a46 100644
--- a/src/backend_libtiff.c
+++ b/src/backend_libtiff.c
@@ -63,6 +63,7 @@ static toff_t mem_size(thandle_t data)
 
 static void source_free(struct imv_source *src)
 {
+  pthread_mutex_lock(&src->busy);
   free(src->name);
   src->name = NULL;
 
@@ -73,6 +74,8 @@ static void source_free(struct imv_source *src)
   free(src->private);
   src->private = NULL;
 
+  pthread_mutex_unlock(&src->busy);
+  pthread_mutex_destroy(&src->busy);
   free(src);
 }
 
@@ -100,6 +103,7 @@ static void report_error(struct imv_source *src)
   msg.bitmap = NULL;
   msg.error = "Internal error";
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
@@ -118,11 +122,17 @@ static void send_bitmap(struct imv_source *src, void *bitmap)
   msg.frametime = 0;
   msg.error = NULL;
 
+  pthread_mutex_unlock(&src->busy);
   src->callback(&msg);
 }
 
-static void load_image(struct imv_source *src)
+static int load_image(struct imv_source *src)
 {
+  /* Don't run if this source is already active */
+  if (pthread_mutex_trylock(&src->busy)) {
+    return -1;
+  }
+
   struct private *private = src->private;
 
   /* libtiff suggests using their own allocation routines to support systems
@@ -135,13 +145,14 @@ static void load_image(struct imv_source *src)
       bitmap, ORIENTATION_TOPLEFT, 0);
 
   /* 1 = success, unlike the rest of *nix */
-  if (rcode == 1) {
-    send_bitmap(src, bitmap);
-  } else {
+  if (rcode != 1) {
     free(bitmap);
     report_error(src);
-    return;
+    return -1;
   }
+
+  send_bitmap(src, bitmap);
+  return 0;
 }
 
 static enum backend_result open_path(const char *path, struct imv_source **src)
@@ -164,6 +175,7 @@ static enum backend_result open_path(const char *path, struct imv_source **src)
   source->height = height;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
@@ -201,6 +213,7 @@ static enum backend_result open_memory(void *data, size_t len, struct imv_source
   source->height = height;
   source->num_frames = 1;
   source->next_frame = 1;
+  pthread_mutex_init(&source->busy, NULL);
   source->load_first_frame = &load_image;
   source->load_next_frame = NULL;
   source->free = &source_free;
diff --git a/src/source.h b/src/source.h
index 74501ed..4f1a0dd 100644
--- a/src/source.h
+++ b/src/source.h
@@ -1,6 +1,7 @@
 #ifndef IMV_SOURCE_H
 #define IMV_SOURCE_H
 
+#include <pthread.h>
 #include <stdbool.h>
 #include "bitmap.h"
 
@@ -36,11 +37,18 @@ struct imv_source {
   /* Next frame to be loaded, 0-indexed */
   int next_frame;
 
-  /* Trigger loading of the first frame. */
-  void (*load_first_frame)(struct imv_source *src); 
+  /* Attempted to be locked by load_first_frame or load_next_frame.
+   * If the mutex can't be locked, the call is aborted.
+   * Used to prevent the source from having multiple worker threads at once.
+   * Released by the source before calling the message callback with a result.
+   */
+  pthread_mutex_t busy;
+
+  /* Trigger loading of the first frame. Returns 0 on success. */
+  int (*load_first_frame)(struct imv_source *src);
 
-  /* Trigger loading of next frame.  */
-  void (*load_next_frame)(struct imv_source *src); 
+  /* Trigger loading of next frame. Returns 0 on success. */
+  int (*load_next_frame)(struct imv_source *src);
 
   /* Safely free contents of this source. After this returns
    * it is safe to dealocate/overwrite the imv_source instance.
-- 
cgit v1.2.3