avoid re-entrancy into libtiff using the mutex api

tonycoz · Nov 12, 2012 · c3994bc · c3994bc
1 parent bcf8804
commit c3994bc
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 2 deletions.
diff --git a/TIFF/TIFF.pm b/TIFF/TIFF.pm
@@ -4,7 +4,7 @@ use Imager;
 use vars qw($VERSION @ISA);
 
 BEGIN {
-  $VERSION = "0.84";
+  $VERSION = "0.85";
 
   require XSLoader;
   XSLoader::load('Imager::File::TIFF', $VERSION);

diff --git a/TIFF/TIFF.xs b/TIFF/TIFF.xs
@@ -143,3 +143,4 @@ i_tiff_has_compression(name)
 
 BOOT:
 	PERL_INITIALIZE_IMAGER_CALLBACKS;
+	i_tiff_init();
diff --git a/TIFF/imtiff.c b/TIFF/imtiff.c
@@ -266,6 +266,13 @@ static void warn_handler(char const *module, char const *fmt, va_list ap) {
 
 #endif
 
+static i_mutex_t mutex;
+
+void
+i_tiff_init(void) {
+  mutex = i_mutex_new();
+}
+
 static int save_tiff_tags(TIFF *tif, i_img *im);
 
 static void 
@@ -621,6 +628,8 @@ i_readtiff_wiol(io_glue *ig, int allow_incomplete, int page) {
   int current_page;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   i_clear_error();
   old_handler = TIFFSetErrorHandler(error_handler);
 #ifdef USE_EXT_WARN_HANDLER
@@ -658,6 +667,7 @@ i_readtiff_wiol(io_glue *ig, int allow_incomplete, int page) {
     TIFFSetWarningHandlerExt(old_ext_warn_handler);
 #endif
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return NULL;
   }
 
@@ -672,6 +682,7 @@ i_readtiff_wiol(io_glue *ig, int allow_incomplete, int page) {
 #endif
       TIFFClose(tif);
       tiffio_context_final(&ctx);
+      i_mutex_unlock(mutex);
       return NULL;
     }
   }
@@ -686,6 +697,7 @@ i_readtiff_wiol(io_glue *ig, int allow_incomplete, int page) {
 #endif
   TIFFClose(tif);
   tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
 
   return im;
 }
@@ -709,6 +721,8 @@ i_readtiff_multi_wiol(io_glue *ig, int *count) {
   int result_alloc = 0;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   i_clear_error();
   old_handler = TIFFSetErrorHandler(error_handler);
 #ifdef USE_EXT_WARN_HANDLER
@@ -747,6 +761,7 @@ i_readtiff_multi_wiol(io_glue *ig, int *count) {
     TIFFSetWarningHandlerExt(old_ext_warn_handler);
 #endif
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return NULL;
   }
 
@@ -781,6 +796,7 @@ i_readtiff_multi_wiol(io_glue *ig, int *count) {
 #endif
   TIFFClose(tif);
   tiffio_context_final(&ctx);
+  i_mutex_unlock(mutex);
 
   return results;
 }
@@ -1446,6 +1462,8 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
   int i;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   old_handler = TIFFSetErrorHandler(error_handler);
 
   i_clear_error();
@@ -1472,6 +1490,7 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
     i_push_error(0, "Could not create TIFF object");
     TIFFSetErrorHandler(old_handler);
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
@@ -1480,6 +1499,7 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
       TIFFClose(tif);
       TIFFSetErrorHandler(old_handler);
       tiffio_context_final(&ctx);
+      i_mutex_unlock(mutex);
       return 0;
     }
 
@@ -1488,6 +1508,7 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
       TIFFClose(tif);
       TIFFSetErrorHandler(old_handler);
       tiffio_context_final(&ctx);
+      i_mutex_unlock(mutex);
       return 0;
     }
   }
@@ -1496,6 +1517,8 @@ i_writetiff_multi_wiol(io_glue *ig, i_img **imgs, int count) {
   (void) TIFFClose(tif);
   tiffio_context_final(&ctx);
 
+  i_mutex_unlock(mutex);
+
   if (i_io_close(ig))
     return 0;
 
@@ -1522,6 +1545,8 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
   TIFFErrorHandler old_handler;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   old_handler = TIFFSetErrorHandler(error_handler);
 
   i_clear_error();
@@ -1548,6 +1573,7 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
     i_push_error(0, "Could not create TIFF object");
     TIFFSetErrorHandler(old_handler);
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
@@ -1556,6 +1582,7 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
       TIFFClose(tif);
       TIFFSetErrorHandler(old_handler);
       tiffio_context_final(&ctx);
+      i_mutex_unlock(mutex);
       return 0;
     }
 
@@ -1564,6 +1591,7 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
       TIFFClose(tif);
       TIFFSetErrorHandler(old_handler);
       tiffio_context_final(&ctx);
+      i_mutex_unlock(mutex);
       return 0;
     }
   }
@@ -1572,6 +1600,8 @@ i_writetiff_multi_wiol_faxable(io_glue *ig, i_img **imgs, int count, int fine) {
   TIFFSetErrorHandler(old_handler);
   tiffio_context_final(&ctx);
 
+  i_mutex_unlock(mutex);
+
   if (i_io_close(ig))
     return 0;
 
@@ -1594,6 +1624,8 @@ i_writetiff_wiol(i_img *img, io_glue *ig) {
   TIFFErrorHandler old_handler;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   old_handler = TIFFSetErrorHandler(error_handler);
 
   i_clear_error();
@@ -1619,19 +1651,22 @@ i_writetiff_wiol(i_img *img, io_glue *ig) {
     i_push_error(0, "Could not create TIFF object");
     tiffio_context_final(&ctx);
     TIFFSetErrorHandler(old_handler);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
   if (!i_writetiff_low(tif, img)) {
     TIFFClose(tif);
     tiffio_context_final(&ctx);
     TIFFSetErrorHandler(old_handler);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
   (void) TIFFClose(tif);
   TIFFSetErrorHandler(old_handler);
-    tiffio_context_final(&ctx);
+  tiffio_context_final(&ctx);
+  i_mutex_unlock(mutex);
 
   if (i_io_close(ig))
     return 0;
@@ -1662,6 +1697,8 @@ i_writetiff_wiol_faxable(i_img *im, io_glue *ig, int fine) {
   TIFFErrorHandler old_handler;
   tiffio_context_t ctx;
 
+  i_mutex_lock(mutex);
+
   old_handler = TIFFSetErrorHandler(error_handler);
 
   i_clear_error();
@@ -1687,19 +1724,22 @@ i_writetiff_wiol_faxable(i_img *im, io_glue *ig, int fine) {
     i_push_error(0, "Could not create TIFF object");
     TIFFSetErrorHandler(old_handler);
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
   if (!i_writetiff_low_faxable(tif, im, fine)) {
     TIFFClose(tif);
     TIFFSetErrorHandler(old_handler);
     tiffio_context_final(&ctx);
+    i_mutex_unlock(mutex);
     return 0;
   }
 
   (void) TIFFClose(tif);
   TIFFSetErrorHandler(old_handler);
   tiffio_context_final(&ctx);
+  i_mutex_unlock(mutex);
 
   if (i_io_close(ig))
     return 0;

diff --git a/TIFF/imtiff.h b/TIFF/imtiff.h
@@ -3,6 +3,7 @@
 
 #include "imdatatypes.h"
 
+void i_tiff_init(void);
 i_img   * i_readtiff_wiol(io_glue *ig, int allow_incomplete, int page);
 i_img  ** i_readtiff_multi_wiol(io_glue *ig, int *count);
 undef_int i_writetiff_wiol(i_img *im, io_glue *ig);

diff --git a/bench/.gitignore b/bench/.gitignore
@@ -0,0 +1 @@
+/largish.tif
diff --git a/bench/largish.jpg b/bench/largish.jpg
diff --git a/bench/tifthread.pl b/bench/tifthread.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.tif") {
+  my $im = Imager->new(file => "bench/largish.jpg")
+    or die "Cannot read bench/largish.jpg:", Imager->errstr;
+  $im->write(file => "bench/largish.tif")
+    or die "Cannot write bench.largish.tif:", $im->errstr;
+}
+
+my @tests =
+  (
+   [ "bench/largish.tif", "" ],
+   [ "TIFF/testimg/grey16.tif", "" ],
+   [ "TIFF/testimg/comp4bad.tif", "(Iolayer): Read error at scanline 120; got 0 bytes, expected 32" ],
+  );
+
+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();
+}