avoid re-entrancy into giflib using the mutex API

diff --git a/GIF/GIF.pm b/GIF/GIF.pm
index 5d92f7a953342569fadb7c69e167165453a98356..05fb1f95f5a3dd39455e61b50c189c4f5f474c46 100644 (file)
--- a/GIF/GIF.pm
+++ b/GIF/GIF.pm
@@ -4,7 +4,7 @@ use Imager;
 use vars qw($VERSION @ISA);
 
 BEGIN {
-  $VERSION = "0.85";
+  $VERSION = "0.86";
 
   require XSLoader;
   XSLoader::load('Imager::File::GIF', $VERSION);

diff --git a/GIF/GIF.xs b/GIF/GIF.xs
index 75687b8217fa585801bcc19b5a59b05f411a71bf..7641b8c3fcc28bf9f66bf0a96535b789515574cb 100644 (file)
--- a/GIF/GIF.xs
+++ b/GIF/GIF.xs
@@ -147,3 +147,4 @@ i_readgif_multi_wiol(ig)
 BOOT:
        PERL_INITIALIZE_IMAGER_CALLBACKS;
        PERL_INITIALIZE_IMAGER_PERL_CALLBACKS;
+       i_init_gif();

diff --git a/GIF/imgif.c b/GIF/imgif.c
index 8dfe83454fea5e990c49530d41533d90242c16e5..9b6e6b1bf6791c6d370e1d15bd01b8254830e1f6 100644 (file)
--- a/GIF/imgif.c
+++ b/GIF/imgif.c
@@ -66,7 +66,8 @@ functionality with giflib3.
 #define myGifError(gif) ((gif)->Error)
 #define MakeMapObject GifMakeMapObject
 #define FreeMapObject GifFreeMapObject
-
+#define gif_mutex_lock(mutex)
+#define gif_mutex_unlock(mutex)
 #else
 #define PRE_SET_VERSION
 static GifFileType *
@@ -86,6 +87,8 @@ myEGifOpen(void *userPtr, OutputFunc outputFunc, int *error) {
   return result;
 }
 #define myGifError(gif) GifLastError()
+#define gif_mutex_lock(mutex) i_mutex_lock(mutex)
+#define gif_mutex_unlock(mutex) i_mutex_unlock(mutex)
 
 #endif
 
@@ -98,6 +101,17 @@ static const int
   InterlacedOffset[] = { 0, 4, 2, 1 }, /* The way Interlaced image should. */
   InterlacedJumps[] = { 8, 8, 4, 2 };    /* be read - offsets and jumps... */
 
+#if !defined(GIFLIB_MAJOR) || GIFLIB_MAJOR < 5
+static i_mutex_t mutex;
+#endif
+
+void
+i_init_gif(void) {
+#if !defined(GIFLIB_MAJOR) || GIFLIB_MAJOR < 5
+  mutex = i_mutex_new();
+#endif
+}
+
 static
 void
 i_colortable_copy(int **colour_table, int *colours, ColorMapObject *colourmap) {
@@ -877,6 +891,9 @@ i_img **
 i_readgif_multi_wiol(io_glue *ig, int *count) {
   GifFileType *GifFile;
   int gif_error;
+  i_img **result;
+
+  gif_mutex_lock(mutex);
 
   i_clear_error();
   
@@ -884,10 +901,15 @@ i_readgif_multi_wiol(io_glue *ig, int *count) {
     gif_push_error(gif_error);
     i_push_error(0, "Cannot create giflib callback object");
     mm_log((1,"i_readgif_multi_wiol: Unable to open callback datasource.\n"));
+    gif_mutex_unlock(mutex);
     return NULL;
   }
     
-  return i_readgif_multi_low(GifFile, count, -1);
+  result = i_readgif_multi_low(GifFile, count, -1);
+
+  gif_mutex_unlock(mutex);
+
+  return result;
 }
 
 static int
@@ -901,6 +923,9 @@ i_img *
 i_readgif_wiol(io_glue *ig, int **color_table, int *colors) {
   GifFileType *GifFile;
   int gif_error;
+  i_img *result;
+
+  gif_mutex_lock(mutex);
 
   i_clear_error();
 
@@ -908,10 +933,15 @@ i_readgif_wiol(io_glue *ig, int **color_table, int *colors) {
     gif_push_error(gif_error);
     i_push_error(0, "Cannot create giflib callback object");
     mm_log((1,"i_readgif_wiol: Unable to open callback datasource.\n"));
+    gif_mutex_unlock(mutex);
     return NULL;
   }
     
-  return i_readgif_low(GifFile, color_table, colors);
+  result = i_readgif_low(GifFile, color_table, colors);
+
+  gif_mutex_unlock(mutex);
+
+  return result;
 }
 
 /*
@@ -957,6 +987,7 @@ i_img *
 i_readgif_single_wiol(io_glue *ig, int page) {
   GifFileType *GifFile;
   int gif_error;
+  i_img *result;
 
   i_clear_error();
   if (page < 0) {
@@ -964,15 +995,21 @@ i_readgif_single_wiol(io_glue *ig, int page) {
     return NULL;
   }
 
+  gif_mutex_lock(mutex);
 
   if ((GifFile = myDGifOpen((void *)ig, io_glue_read_cb, &gif_error )) == NULL) {
     gif_push_error(gif_error);
     i_push_error(0, "Cannot create giflib callback object");
     mm_log((1,"i_readgif_wiol: Unable to open callback datasource.\n"));
+    gif_mutex_unlock(mutex);
     return NULL;
   }
     
-  return i_readgif_single_low(GifFile, page);
+  result = i_readgif_single_low(GifFile, page);
+
+  gif_mutex_unlock(mutex);
+
+  return result;
 }
 
 /*
@@ -1830,6 +1867,8 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
   int gif_error;
   int result;
 
+  gif_mutex_lock(mutex);
+
   i_clear_error();
 
 #ifdef PRE_SET_VERSION
@@ -1840,6 +1879,7 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
     gif_push_error(gif_error);
     i_push_error(0, "Cannot create giflib callback object");
     mm_log((1,"i_writegif_wiol: Unable to open callback datasource.\n"));
+    gif_mutex_unlock(mutex);
     return 0;
   }
   
@@ -1849,6 +1889,8 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
 
   result = i_writegif_low(quant, GifFile, imgs, count);
   
+  gif_mutex_unlock(mutex);
+
   if (i_io_close(ig))
     return 0;
   
@@ -1970,6 +2012,8 @@ gif_push_error(int code) {
 
 Arnar M. Hrafnkelsson, addi@umich.edu
 
+Tony Cook <tonyc@cpan.org>
+
 =head1 SEE ALSO
 
 perl(1), Imager(3)

diff --git a/GIF/imgif.h b/GIF/imgif.h
index ca92af5d3fd1e6519bcba9b781388cf2327dfae2..9a92e8c21ae7972b41da5ce3d830a93cdf67a440 100644 (file)
--- a/GIF/imgif.h
+++ b/GIF/imgif.h
@@ -3,6 +3,7 @@
 
 #include "imext.h"
 
+void i_init_gif(void);
 double i_giflib_version(void);
 i_img *i_readgif_wiol(io_glue *ig, int **colour_table, int *colours);
 i_img *i_readgif_single_wiol(io_glue *ig, int page);

diff --git a/bench/.gitignore b/bench/.gitignore
index 4865f40120081751dfac57e5342ede2cf437d211..595be542b891605da733c621fbe9b52415cfa619 100644 (file)
--- a/bench/.gitignore
+++ b/bench/.gitignore
@@ -1 +1,2 @@
 /largish.tif
+/largish.gif

diff --git a/bench/gifthread.pl b/bench/gifthread.pl
new file mode 100644 (file)
index 0000000..e22a242
--- /dev/null
+++ b/bench/gifthread.pl
@@ -0,0 +1,55 @@
+#!perl -w
+use strict;
+use threads;
+use Imager;
+
+++$|;
+Imager->preload;
+
+# as a TIFF this file is large, build it from largeish.jpg if it
+# doesn't exist
+unless (-f "bench/largish.gif") {
+  my $im = Imager->new(file => "bench/largish.jpg")
+    or die "Cannot read bench/largish.jpg:", Imager->errstr;
+  $im->write(file => "bench/largish.gif")
+    or die "Cannot write bench/largish.gif:", $im->errstr;
+}
+
+my @tests =
+  (
+   [ "bench/largish.gif", "" ],
+   [ "GIF/testimg/scale.gif", "" ],
+   [ "GIF/testimg/nocmap.gif", "Image does not have a local or a global color map" ],
+  );
+
+my @threads;
+my $name = "A";
+for my $test (@tests) {
+  push @threads,
+    threads->create
+      (
+       sub  {
+        my ($file, $result, $name) = @_;
+        for (1 .. 100000) {
+          print $name;
+          my $im = Imager->new(file => $file);
+          if ($result) {
+            $im and die "Expected error from $file, got image";
+            Imager->errstr eq $result
+              or die "Expected error '$result', got '",Imager->errstr, "'"
+          }
+          else {
+            $im or die "Expected image got error '", Imager->errstr, "'";
+          }
+        }
+        return;
+       },
+       @$test,
+       $name
+      );
+  ++$name;
+}
+
+for my $t (@threads) {
+  $t->join();
+}