thread safety for the FT2 driver

diff --git a/FT2/FT2.pm b/FT2/FT2.pm
index aa83aab0fcae0d20cc67f14feddd705a236021b4..a3a8418a219992bb1d10d4853884e3cada51d2f5 100644 (file)
--- a/FT2/FT2.pm
+++ b/FT2/FT2.pm
@@ -1,11 +1,12 @@
 package Imager::Font::FT2;
 use strict;
 use Imager;
+use Scalar::Util ();
 use vars qw($VERSION @ISA);
 @ISA = qw(Imager::Font);
 
 BEGIN {
-  $VERSION = "0.85";
+  $VERSION = "0.86";
 
   require XSLoader;
   XSLoader::load('Imager::Font::FT2', $VERSION);
@@ -50,6 +51,10 @@ sub new {
 
 sub _draw {
   my $self = shift;
+
+  $self->_valid
+    or return;
+
   my %input = @_;
   if (exists $input{channel}) {
     i_ft2_cp($self->{id}, $input{image}{IMG}, $input{'x'}, $input{'y'},
@@ -69,12 +74,19 @@ sub _bounding_box {
   my $self = shift;
   my %input = @_;
 
+  $self->_valid
+    or return;
+
   return i_ft2_bbox($self->{id}, $input{size}, $input{sizew}, $input{string}, 
                    $input{utf8});
 }
 
 sub dpi {
   my $self = shift;
+
+  $self->_valid
+    or return;
+
   my @old = i_ft2_getdpi($self->{id});
   if (@_) {
     my %hsh = @_;
@@ -97,12 +109,18 @@ sub dpi {
 sub hinting {
   my ($self, %opts) = @_;
 
+  $self->_valid
+    or return;
+
   i_ft2_sethinting($self->{id}, $opts{hinting} || 0);
 }
 
 sub _transform {
   my $self = shift;
 
+  $self->_valid
+    or return;
+
   my %hsh = @_;
   my $matrix = $hsh{matrix} or return undef;
 
@@ -117,6 +135,9 @@ sub utf8 {
 sub has_chars {
   my ($self, %hsh) = @_;
 
+  $self->_valid
+    or return;
+
   unless (defined $hsh{string} && length $hsh{string}) {
     $Imager::ERRSTR = "No string supplied to \$font->has_chars()";
     return;
@@ -128,6 +149,9 @@ sub has_chars {
 sub face_name {
   my ($self) = @_;
 
+  $self->_valid
+    or return;
+
   i_ft2_face_name($self->{id});
 }
 
@@ -138,6 +162,9 @@ sub can_glyph_names {
 sub glyph_names {
   my ($self, %input) = @_;
 
+  $self->_valid
+    or return;
+
   my $string = $input{string};
   defined $string
     or return Imager->_set_error("no string parameter passed to glyph_names");
@@ -154,12 +181,18 @@ sub glyph_names {
 sub is_mm {
   my ($self) = @_;
 
+  $self->_valid
+    or return;
+
   i_ft2_is_multiple_master($self->{id});
 }
 
 sub mm_axes {
   my ($self) = @_;
 
+  $self->_valid
+    or return;
+
   my ($num_axis, $num_design, @axes) =
     i_ft2_get_multiple_masters($self->{id})
       or return Imager->_set_error(Imager->_error_as_msg);
@@ -170,6 +203,9 @@ sub mm_axes {
 sub set_mm_coords {
   my ($self, %opts) = @_;
 
+  $self->_valid
+    or return;
+
   $opts{coords}
     or return Imager->_set_error("Missing coords parameter");
   ref($opts{coords}) && $opts{coords} =~ /ARRAY\(0x[\da-f]+\)$/
@@ -180,6 +216,19 @@ sub set_mm_coords {
 
   return 1;
 }
+
+# objects may be invalidated on thread creation (or Win32 fork emulation)
+sub _valid {
+  my $self = shift;
+
+  unless ($self->{id} && Scalar::Util::blessed($self->{id})) {
+    Imager->_set_error("font object was created in another thread");
+    return;
+  }
+
+  return 1;
+}
+
 1;
 
 __END__

diff --git a/FT2/FT2.xs b/FT2/FT2.xs
index ac8396efbd7779de4fd19e0cd90323435efc184b..61a964efb1edb380d86690e8c2035ed91a125a5d 100644 (file)
--- a/FT2/FT2.xs
+++ b/FT2/FT2.xs
@@ -356,3 +356,4 @@ i_ft2_set_mm_coords(handle, ...)
 
 BOOT:
        PERL_INITIALIZE_IMAGER_CALLBACKS;
+       i_ft2_start();

diff --git a/FT2/MANIFEST b/FT2/MANIFEST
index 0f154ff5b4618c4582d210a2cb7c2b4c2bf65330..4e4becf8637d202ac54b0d2fdc873fb2be306c91 100644 (file)
--- a/FT2/MANIFEST
+++ b/FT2/MANIFEST
@@ -16,4 +16,5 @@ MANIFEST                      This list of files
 MANIFEST.SKIP
 README
 t/t10ft2.t
+t/t20thread.t
 typemap

diff --git a/FT2/Makefile.PL b/FT2/Makefile.PL
index 076e5a32656e06468749f3ab0883fdf9aa46b6c9..79e375f034416cfc685ec6964ac9a68bda8f8499 100644 (file)
--- a/FT2/Makefile.PL
+++ b/FT2/Makefile.PL
@@ -61,6 +61,7 @@ else {
     $opts{PREREQ_PM} =
       {
        @Imager_req,
+       'Scalar::Util' => 1.00,
        XSLoader => 0,
       };
   }

diff --git a/FT2/freetyp2.c b/FT2/freetyp2.c
index fc060efdb1c521a091f1017210af146868338325..c6aa727038d426603ab12672820d45357fa36c6a 100644 (file)
--- a/FT2/freetyp2.c
+++ b/FT2/freetyp2.c
@@ -49,40 +49,84 @@ Truetype, Type1 and Windows FNT.
 
 static void ft2_push_message(int code);
 
-static int ft2_initialized = 0;
-static FT_Library library;
+static void ft2_final(void *);
+
+static im_slot_t slot = -1;
+
+typedef struct {
+  int initialized;
+  FT_Library library;
+  im_context_t ctx;
+} ft2_state;
 
 static i_img_dim i_min(i_img_dim a, i_img_dim b);
 static i_img_dim i_max(i_img_dim a, i_img_dim b);
 
+void
+i_ft2_start(void) {
+  if (slot == -1)
+    slot = im_context_slot_new(ft2_final);
+}
+
 /*
 =item i_ft2_init(void)
 
 Initializes the Freetype 2 library.
 
-Returns true on success, false on failure.
+Returns ft2_state * on success or NULL on failure.
 
 =cut
 */
-int
+
+static ft2_state *
 i_ft2_init(void) {
   FT_Error error;
+  im_context_t ctx = im_get_context();
+  ft2_state *ft2 = im_context_slot_get(ctx, slot);
+
+  if (ft2 == NULL) {
+    ft2 = mymalloc(sizeof(ft2_state));
+    ft2->initialized = 0;
+    ft2->library = NULL;
+    ft2->ctx = ctx;
+    im_context_slot_set(ctx, slot, ft2);
+    mm_log((1, "created FT2 state %p for context %p\n", ft2, ctx));
+  }
 
   i_clear_error();
-  error = FT_Init_FreeType(&library);
-  if (error) {
-    ft2_push_message(error);
-    i_push_error(0, "Initializing Freetype2");
-    return 0;
+  if (!ft2->initialized) {
+    error = FT_Init_FreeType(&ft2->library);
+    if (error) {
+      ft2_push_message(error);
+      i_push_error(0, "Initializing Freetype2");
+      return NULL;
+    }
+    mm_log((1, "initialized FT2 state %p\n", ft2));
+
+    ft2->initialized = 1;
   }
 
-  ft2_initialized = 1;
+  return ft2;
+}
+
+static void
+ft2_final(void *state) {
+  ft2_state *ft2 = state;
+
+  if (ft2->initialized) {
+    mm_log((1, "finalizing FT2 state %p\n", state));
+    FT_Done_FreeType(ft2->library);
+    ft2->library = NULL;
+    ft2->initialized = 0;
+  }
 
-  return 1;
+  mm_log((1, "freeing FT2 state %p\n", state));
+  myfree(state);
 }
 
 struct FT2_Fonthandle {
   FT_Face face;
+  ft2_state *state;
   int xdpi, ydpi;
   int hint;
   FT_Encoding encoding;
@@ -138,14 +182,15 @@ i_ft2_new(const char *name, int index) {
   int i, j;
   FT_Encoding encoding;
   int score;
+  ft2_state *ft2;
 
   mm_log((1, "i_ft2_new(name %p, index %d)\n", name, index));
 
-  if (!ft2_initialized && !i_ft2_init())
+  if ((ft2 = i_ft2_init()) == NULL)
     return NULL;
 
   i_clear_error();
-  error = FT_New_Face(library, name, index, &face);
+  error = FT_New_Face(ft2->library, name, index, &face);
   if (error) {
     ft2_push_message(error);
     i_push_error(error, "Opening face");
@@ -173,6 +218,7 @@ i_ft2_new(const char *name, int index) {
 
   result = mymalloc(sizeof(FT2_Fonthandle));
   result->face = face;
+  result->state = ft2;
   result->xdpi = result->ydpi = 72;
   result->encoding = encoding;
 
@@ -244,7 +290,7 @@ i_ft2_setdpi(FT2_Fonthandle *handle, int xdpi, int ydpi) {
   if (xdpi > 0 && ydpi > 0) {
     handle->xdpi = xdpi;
     handle->ydpi = ydpi;
-    return 0;
+    return 1;
   }
   else {
     i_push_error(0, "resolutions must be positive");

diff --git a/FT2/imft2.h b/FT2/imft2.h
index 57b71293732e2e9fa9889ac6a605e9983dd254bd..6c641c90ca7d77691eb0f3cc2219fc0e124112e7 100644 (file)
--- a/FT2/imft2.h
+++ b/FT2/imft2.h
@@ -7,7 +7,7 @@ typedef struct FT2_Fonthandle FT2_Fonthandle;
 
 typedef FT2_Fonthandle* Imager__Font__FT2x;
 
-extern int i_ft2_init(void);
+extern void i_ft2_start(void);
 extern FT2_Fonthandle * i_ft2_new(const char *name, int index);
 extern void i_ft2_destroy(FT2_Fonthandle *handle);
 extern int i_ft2_setdpi(FT2_Fonthandle *handle, int xdpi, int ydpi);

diff --git a/FT2/t/t20thread.t b/FT2/t/t20thread.t
new file mode 100644 (file)
index 0000000..723189d
--- /dev/null
+++ b/FT2/t/t20thread.t
@@ -0,0 +1,59 @@
+#!perl -w
+use strict;
+use Imager;
+
+use Config;
+my $loaded_threads;
+BEGIN {
+  if ($Config{useithreads} && $] > 5.008007) {
+    $loaded_threads =
+      eval {
+       require threads;
+       threads->import;
+       1;
+      };
+  }
+}
+
+use Test::More;
+
+$Config{useithreads}
+  or plan skip_all => "can't test Imager's lack of threads support with no threads";
+$] > 5.008007
+  or plan skip_all => "require a perl with CLONE_SKIP to test Imager's lack of threads support";
+$loaded_threads
+  or plan skip_all => "couldn't load threads";
+
+$INC{"Devel/Cover.pm"}
+  and plan skip_all => "threads and Devel::Cover don't get along";
+
+# https://rt.cpan.org/Ticket/Display.html?id=65812
+# https://github.com/schwern/test-more/issues/labels/Test-Builder2#issue/100
+$Test::More::VERSION =~ /^2\.00_/
+  and plan skip_all => "threads are hosed in 2.00_06 and presumably all 2.00_*";
+
+plan tests => 8;
+
+Imager->open_log(log => "testout/t20thread.log");
+
+my $ft1 = Imager::Font->new(file => "fontfiles/dodge.ttf", type => "ft2");
+ok($ft1, "make a font");
+ok($ft1->_valid, "and it's valid");
+my $ft2;
+
+my $thr = threads->create
+  (
+   sub {
+     ok(!$ft1->_valid, "first font no longer valid");
+     $ft2 = Imager::Font->new(file => "fontfiles/dodge.ttf", type => "ft2");
+     ok($ft2, "make a new font in thread");
+     ok($ft2->_valid, "and it's valid");
+     1;
+   },
+  );
+
+ok($thr->join, "join the thread");
+ok($ft1->_valid, "original font still valid in main thread");
+is($ft2, undef, "font created in thread shouldn't be set in main thread");
+
+Imager->close_log();

diff --git a/MANIFEST b/MANIFEST
index dcd28c2ec34c4ebdcde4d7cbd573d7c28d8d3a73..ba931424ca97ccca3cba4c6e8d4e1b9337ac8c41 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -69,6 +69,7 @@ FT2/imft2.h
 FT2/Makefile.PL
 FT2/README
 FT2/t/t10ft2.t
+FT2/t/t20thread.t
 FT2/typemap
 gaussian.im
 GIF/GIF.pm