[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

@@ -1,3 +1,37 @@
+2015-01-14  Behdad Esfahbod  <behdad@behdad.org>
+
+	[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  <behdad@behdad.org>
 
 	[autofit] Allocate AF_Loader on the stack instead of AF_Module.

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 ) &&
+      error = TT_Run_Context( loader->exec );
-                       ((TT_Size)loader->size)->debug             );
-
-      error = TT_Run_Context( loader->exec, debug );
       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
+      exec = size->context;
-                         : ( (TT_Driver)FT_FACE_DRIVER( face ) )->context;
       if ( !exec )
         return FT_THROW( Could_Not_Find_Context );
 

src/truetype/ttinterp.c

@@ -544,12 +544,8 @@
   /* <Return>                                                              */
   /*    TrueType error code.  0 means success.                             */
   /*                                                                       */
-  /* <Note>                                                                */
-  /*    Only the glyph loader and debugger should call this function.      */
-  /*                                                                       */
   FT_LOCAL_DEF( FT_Error )
-  TT_Run_Context( TT_ExecContext  exec,
+  TT_Run_Context( TT_ExecContext  exec )
-                  FT_Bool         debug )
   {
     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 )
+    /* allocate object */
-    {
+    if ( FT_NEW( exec ) )
-      FT_Error        error;
+      goto Fail;
-      TT_ExecContext  exec;
 
+    /* initialize it; in case of error this deallocates `exec' too */
+    error = Init_Context( exec, memory );
+    if ( error )
+      goto Fail;
 
-      /* allocate object */
+    return exec;
-      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;
 
   Fail:
     return NULL;

src/truetype/ttinterp.h

@@ -327,6 +327,7 @@ FT_BEGIN_HEADER
   /*                                                                       */
   /* <Note>                                                                */
   /*    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,
+  TT_Run_Context( TT_ExecContext  exec );
-                  FT_Bool         debug );
 #endif /* TT_USE_BYTECODE_INTERPRETER */
 
 

src/truetype/ttobjs.c

@@ -751,14 +751,7 @@
     FT_Error        error;
 
 
-    /* debugging instances have their own context */
+    exec = size->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 );
 
     error = TT_Load_Context( exec, face, size );
     if ( error )
@@ -845,14 +838,7 @@
     FT_Error        error;
 
 
-    /* debugging instances have their own context */
+    exec = size->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 );
 
     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->context )
-    if ( size->debug )
     {
-      /* 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
-
   }
 
 

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;