thread safety for the FT2 driver
authorTony Cook <tony@develop-help.com>
Mon, 15 Oct 2012 13:12:29 +0000 (00:12 +1100)
committerTony Cook <tony@develop-help.com>
Sat, 24 Nov 2012 04:05:19 +0000 (15:05 +1100)
FT2/FT2.pm
FT2/FT2.xs
FT2/MANIFEST
FT2/Makefile.PL
FT2/freetyp2.c
FT2/imft2.h
FT2/t/t20thread.t [new file with mode: 0644]
MANIFEST

index 1178700..c67434c 100644 (file)
@@ -1,6 +1,7 @@
 package Imager::Font::FT2;
 use strict;
 use Imager;
+use Scalar::Util ();
 use vars qw($VERSION @ISA);
 @ISA = qw(Imager::Font);
 
@@ -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__
index 95bf495..631b147 100644 (file)
@@ -369,3 +369,4 @@ i_ft2_set_mm_coords(handle, ...)
 
 BOOT:
        PERL_INITIALIZE_IMAGER_CALLBACKS;
+       i_ft2_start();
index 0f154ff..4e4becf 100644 (file)
@@ -16,4 +16,5 @@ MANIFEST                      This list of files
 MANIFEST.SKIP
 README
 t/t10ft2.t
+t/t20thread.t
 typemap
index 81d8176..79b45ef 100644 (file)
@@ -61,6 +61,7 @@ else {
     $opts{PREREQ_PM} =
       {
        @Imager_req,
+       'Scalar::Util' => 1.00,
        XSLoader => 0,
       };
   }
index e7bd850..6431ad2 100644 (file)
@@ -50,8 +50,18 @@ 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 ft2_state *
+i_ft2_init(void);
 
 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);
@@ -59,22 +69,25 @@ static i_img_dim i_max(i_img_dim a, i_img_dim b);
 int
 i_ft2_version(int runtime, char *buf, size_t buf_size) {
   char work[100];
-  i_clear_error();
 
-  if (!ft2_initialized && !i_ft2_init())
-    return NULL;
+  i_clear_error();
 
   if (buf_size == 0) {
     i_push_error(0, "zero size buffer supplied");
     return 0;
   }
   if (runtime) {
+    ft2_state *ft2;
     /* initialized to work around a bug in FT2
        http://lists.nongnu.org/archive/html/freetype-devel/2002-09/msg00058.html
        Though I don't know why I still see this in 2.4.2
      */
     FT_Int major = 1, minor = 1, patch = 1;
-    FT_Library_Version(library, &major, &minor, &patch);
+
+    if ((ft2 = i_ft2_init()) == NULL)
+      return 0;
+
+    FT_Library_Version(ft2->library, &major, &minor, &patch);
     sprintf(work, "%d.%d.%d", (int)major, (int)minor, (int)patch);
   }
   else {
@@ -86,34 +99,71 @@ i_ft2_version(int runtime, char *buf, size_t buf_size) {
   return 1;
 }
 
+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;
+}
 
-  return 1;
+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;
+  }
+
+  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;
@@ -169,14 +219,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");
@@ -204,6 +255,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;
 
@@ -275,7 +327,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");
index 842af88..2153a33 100644 (file)
@@ -7,8 +7,8 @@ typedef struct FT2_Fonthandle FT2_Fonthandle;
 
 typedef FT2_Fonthandle* Imager__Font__FT2x;
 
-extern int i_ft2_init(void);
 extern int i_ft2_version(int runtime, char *buf, size_t buf_size);
+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
@@ -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();
index 63f6fec..bfc74c9 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