From 531d463aed365b9790f6065b98e94b9bb14289bb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 14 Jan 2015 17:46:55 +0100 Subject: [PATCH] [truetype] Allocate TT_ExecContext in TT_Size instead of TT_Driver. Previously the code had stipulation for using a per-TT_Size exec context if `size->debug' was true. But there was no way that `size->debug' could *ever* be true. As such, the code was always using the singleton `TT_ExecContext' that was stored in `TT_Driver'. This was, clearly, not threadsafe. With this patch, loading glyphs from different faces from different threads doesn't crash in the bytecode loader code. * src/truetype/ttobjs.h (TT_SizeRec): Remove `debug' member. (TT_DriverRec): Remove `context' member. * src/truetype/ttobjs.c (tt_size_run_fpgm, tt_size_run_prep): Remove `TT_ExecContext' code related to a global `TT_Driver' object. (tt_driver_done): Don't remove `TT_ExecContext' object here but ... (tt_size_done_bytecode): ... here. (tt_driver_init): Don't create `TT_ExecContext' object here but ... (tt_size_init_bytecode): ... here, only on demand. * src/truetype/ttinterp.c (TT_Run_Context): Remove defunct debug code. (TT_New_Context): Remove `TT_ExecContext' code related to a global `TT_Driver' object. * src/truetype/ttinterp.h: Updated. * src/truetype/ttgload.c (TT_Hint_Glyph, tt_loader_init): Updated. --- ChangeLog | 34 ++++++++++++++++++++++++++++ src/truetype/ttgload.c | 9 ++------ src/truetype/ttinterp.c | 44 ++++++++++-------------------------- src/truetype/ttinterp.h | 4 ++-- src/truetype/ttobjs.c | 50 +++++++---------------------------------- src/truetype/ttobjs.h | 8 ------- 6 files changed, 58 insertions(+), 91 deletions(-) diff --git a/ChangeLog b/ChangeLog index bc8c72561..5f34ee411 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,37 @@ +2015-01-14 Behdad Esfahbod + + [truetype] Allocate TT_ExecContext in TT_Size instead of TT_Driver. + + Previously the code had stipulation for using a per-TT_Size exec + context if `size->debug' was true. But there was no way that + `size->debug' could *ever* be true. As such, the code was always + using the singleton `TT_ExecContext' that was stored in `TT_Driver'. + This was, clearly, not threadsafe. + + With this patch, loading glyphs from different faces from different + threads doesn't crash in the bytecode loader code. + + * src/truetype/ttobjs.h (TT_SizeRec): Remove `debug' member. + (TT_DriverRec): Remove `context' member. + + * src/truetype/ttobjs.c (tt_size_run_fpgm, tt_size_run_prep): Remove + `TT_ExecContext' code related to a global `TT_Driver' object. + + (tt_driver_done): Don't remove `TT_ExecContext' object here but ... + (tt_size_done_bytecode): ... here. + + (tt_driver_init): Don't create `TT_ExecContext' object here but ... + (tt_size_init_bytecode): ... here, only on demand. + + * src/truetype/ttinterp.c (TT_Run_Context): Remove defunct debug + code. + (TT_New_Context): Remove `TT_ExecContext' code related to a global + `TT_Driver' object. + + * src/truetype/ttinterp.h: Updated. + + * src/truetype/ttgload.c (TT_Hint_Glyph, tt_loader_init): Updated. + 2015-01-14 Behdad Esfahbod [autofit] Allocate AF_Loader on the stack instead of AF_Module. diff --git a/src/truetype/ttgload.c b/src/truetype/ttgload.c index c5841c301..c78027534 100644 --- a/src/truetype/ttgload.c +++ b/src/truetype/ttgload.c @@ -794,7 +794,6 @@ if ( n_ins > 0 ) { - FT_Bool debug; FT_Error error; FT_GlyphLoader gloader = loader->gloader; @@ -807,10 +806,7 @@ loader->exec->is_composite = is_composite; loader->exec->pts = *zone; - debug = FT_BOOL( !( loader->load_flags & FT_LOAD_NO_SCALE ) && - ((TT_Size)loader->size)->debug ); - - error = TT_Run_Context( loader->exec, debug ); + error = TT_Run_Context( loader->exec ); if ( error && loader->exec->pedantic_hinting ) return error; @@ -2137,8 +2133,7 @@ return size->cvt_ready; /* query new execution context */ - exec = size->debug ? size->context - : ( (TT_Driver)FT_FACE_DRIVER( face ) )->context; + exec = size->context; if ( !exec ) return FT_THROW( Could_Not_Find_Context ); diff --git a/src/truetype/ttinterp.c b/src/truetype/ttinterp.c index 293af9d5d..feaf8c7bf 100644 --- a/src/truetype/ttinterp.c +++ b/src/truetype/ttinterp.c @@ -544,12 +544,8 @@ /* */ /* TrueType error code. 0 means success. */ /* */ - /* */ - /* Only the glyph loader and debugger should call this function. */ - /* */ FT_LOCAL_DEF( FT_Error ) - TT_Run_Context( TT_ExecContext exec, - FT_Bool debug ) + TT_Run_Context( TT_ExecContext exec ) { TT_Goto_CodeRange( exec, tt_coderange_glyph, 0 ); @@ -579,16 +575,7 @@ exec->top = 0; exec->callTop = 0; -#if 1 - FT_UNUSED( debug ); - return exec->face->interpreter( exec ); -#else - if ( !debug ) - return TT_RunIns( exec ); - else - return FT_Err_Ok; -#endif } @@ -622,6 +609,9 @@ TT_New_Context( TT_Driver driver ) { FT_Memory memory; + FT_Error error; + + TT_ExecContext exec; if ( !driver ) @@ -629,26 +619,16 @@ memory = driver->root.root.memory; - if ( !driver->context ) - { - FT_Error error; - TT_ExecContext exec; + /* allocate object */ + if ( FT_NEW( exec ) ) + goto Fail; + /* initialize it; in case of error this deallocates `exec' too */ + error = Init_Context( exec, memory ); + if ( error ) + goto Fail; - /* allocate object */ - if ( FT_NEW( exec ) ) - goto Fail; - - /* initialize it; in case of error this deallocates `exec' too */ - error = Init_Context( exec, memory ); - if ( error ) - goto Fail; - - /* store it into the driver */ - driver->context = exec; - } - - return driver->context; + return exec; Fail: return NULL; diff --git a/src/truetype/ttinterp.h b/src/truetype/ttinterp.h index 2893c56a7..8f213be41 100644 --- a/src/truetype/ttinterp.h +++ b/src/truetype/ttinterp.h @@ -327,6 +327,7 @@ FT_BEGIN_HEADER /* */ /* */ /* Only the glyph loader and debugger should call this function. */ + /* (And right now only the glyph loader uses it.) */ /* */ FT_EXPORT( TT_ExecContext ) TT_New_Context( TT_Driver driver ); @@ -346,8 +347,7 @@ FT_BEGIN_HEADER TT_Size ins ); FT_LOCAL( FT_Error ) - TT_Run_Context( TT_ExecContext exec, - FT_Bool debug ); + TT_Run_Context( TT_ExecContext exec ); #endif /* TT_USE_BYTECODE_INTERPRETER */ diff --git a/src/truetype/ttobjs.c b/src/truetype/ttobjs.c index 4707dfe15..8877c4d26 100644 --- a/src/truetype/ttobjs.c +++ b/src/truetype/ttobjs.c @@ -751,14 +751,7 @@ FT_Error error; - /* debugging instances have their own context */ - if ( size->debug ) - exec = size->context; - else - exec = ( (TT_Driver)FT_FACE_DRIVER( face ) )->context; - - if ( !exec ) - return FT_THROW( Could_Not_Find_Context ); + exec = size->context; error = TT_Load_Context( exec, face, size ); if ( error ) @@ -845,14 +838,7 @@ FT_Error error; - /* debugging instances have their own context */ - if ( size->debug ) - exec = size->context; - else - exec = ( (TT_Driver)FT_FACE_DRIVER( face ) )->context; - - if ( !exec ) - return FT_THROW( Could_Not_Find_Context ); + exec = size->context; error = TT_Load_Context( exec, face, size ); if ( error ) @@ -876,12 +862,9 @@ { TT_Goto_CodeRange( exec, tt_coderange_cvt, 0 ); - if ( !size->debug ) - { - FT_TRACE4(( "Executing `prep' table.\n" )); + FT_TRACE4(( "Executing `prep' table.\n" )); - error = face->interpreter( exec ); - } + error = face->interpreter( exec ); } else error = FT_Err_Ok; @@ -924,12 +907,10 @@ TT_Face face = (TT_Face)ftsize->face; FT_Memory memory = face->root.memory; - - if ( size->debug ) + if ( size->context ) { - /* the debug context must be deleted by the debugger itself */ + TT_Done_Context( size->context ); size->context = NULL; - size->debug = FALSE; } FT_FREE( size->cvt ); @@ -976,6 +957,8 @@ size->bytecode_ready = -1; size->cvt_ready = -1; + size->context = TT_New_Context( (TT_Driver)face->root.driver ); + size->max_function_defs = maxp->maxFunctionDefs; size->max_instruction_defs = maxp->maxInstructionDefs; @@ -1259,10 +1242,6 @@ TT_Driver driver = (TT_Driver)ttdriver; - - if ( !TT_New_Context( driver ) ) - return FT_THROW( Could_Not_Find_Context ); - #ifdef TT_CONFIG_OPTION_SUBPIXEL_HINTING driver->interpreter_version = TT_INTERPRETER_VERSION_38; #else @@ -1293,20 +1272,7 @@ FT_LOCAL_DEF( void ) tt_driver_done( FT_Module ttdriver ) /* TT_Driver */ { -#ifdef TT_USE_BYTECODE_INTERPRETER - TT_Driver driver = (TT_Driver)ttdriver; - - - /* destroy the execution context */ - if ( driver->context ) - { - TT_Done_Context( driver->context ); - driver->context = NULL; - } -#else FT_UNUSED( ttdriver ); -#endif - } diff --git a/src/truetype/ttobjs.h b/src/truetype/ttobjs.h index 859164f86..782255c0a 100644 --- a/src/truetype/ttobjs.h +++ b/src/truetype/ttobjs.h @@ -324,13 +324,6 @@ FT_BEGIN_HEADER TT_GlyphZoneRec twilight; /* The instance's twilight zone */ - /* debugging variables */ - - /* When using the debugger, we must keep the */ - /* execution context tied to the instance */ - /* object rather than asking it on demand. */ - - FT_Bool debug; TT_ExecContext context; /* if negative, `fpgm' (resp. `prep'), wasn't executed yet; */ @@ -351,7 +344,6 @@ FT_BEGIN_HEADER { FT_DriverRec root; - TT_ExecContext context; /* execution context */ TT_GlyphZoneRec zone; /* glyph loader points zone */ FT_UInt interpreter_version;