From 17f8d4551b76df4cc6876e73a81b8519e942a3e2 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Sun, 9 Sep 2012 23:55:43 +1000 Subject: [PATCH] thread safety for the FT2 driver --- FT2/FT2.pm | 51 +++++++++++++++++++++++++++++++- FT2/FT2.xs | 1 + FT2/MANIFEST | 1 + FT2/Makefile.PL | 1 + FT2/freetyp2.c | 74 ++++++++++++++++++++++++++++++++++++++--------- FT2/imft2.h | 2 +- FT2/t/t20thread.t | 59 +++++++++++++++++++++++++++++++++++++ MANIFEST | 1 + 8 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 FT2/t/t20thread.t diff --git a/FT2/FT2.pm b/FT2/FT2.pm index aa83aab0..a3a8418a 100644 --- 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 ac8396ef..61a964ef 100644 --- 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 0f154ff5..4e4becf8 100644 --- 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 076e5a32..79e375f0 100644 --- 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 fc060efd..c6aa7270 100644 --- 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 57b71293..6c641c90 100644 --- 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 index 00000000..723189d0 --- /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 dcd28c2e..ba931424 100644 --- 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 -- 2.39.5