Commit f9e05597780eae8db99a238319bbbee06f7cf738

David Turner 2005-05-23T13:04:53

* include/freetype/cache/ftcache.h, src/cache/ftccache.c, src/cache/ftcsbits.c: fixing bug #12213 (incorrect behaviour of the cache sub-system in low-memory conditions).

diff --git a/ChangeLog b/ChangeLog
index b14f22a..bfd27da 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2005-05-23  David Turner  <dturner@freetype.org>
+
+    * include/freetype/cache/ftcache.h, src/cache/ftccache.c,
+    src/cache/ftcsbits.c: fixing bug #12213 (incorrect behaviour
+    of the cache sub-system in low-memory conditions).
+
 2005-05-23  Werner Lemberg  <wl@gnu.org>
 
 	* src/base/rules.mk (BASE_SRC): Don't add ftsynth.c here but...
@@ -194,7 +200,7 @@
 	* include/freetype/internal/ftserv.h: Add compiler pragmas to get
 	rid of annoying warnings with Visual C++ compiler in maximum warning
 	mode.
-    
+
 	* src/autofit/afhints.c, src/autofit/aflatin.c, src/base/ftstroke.c,
 	src/bdf/bdfdrivr.c, src/cache/ftcbasic.c, src/cache/ftccmap.c,
 	src/cache/ftcmanag.c, src/cff/cffload.c, src/cid/cidload.c,
diff --git a/include/freetype/cache/ftccache.h b/include/freetype/cache/ftccache.h
index 7c7688d..f441915 100644
--- a/include/freetype/cache/ftccache.h
+++ b/include/freetype/cache/ftccache.h
@@ -261,6 +261,50 @@ FT_BEGIN_HEADER
 
 #endif /* !FTC_INLINE */
 
+
+/* use this macro with FTC_CACHE_TRYLOOP_END() in order to define
+ * a retry loop that will flush the cache repeatidely in case of
+ * memory overflows.
+ *
+ * this is used when creating a new cache node, or within a lookup
+ * that needs to allocate things (e.g. the sbit cache lookup)
+ * 
+ * here's an example:
+ *
+ * {
+ *   FTC_CACHE_TRYLOOP(cache)
+ *     error = load_data( ... );
+ *   FTC_CACHE_TRYLOOP_END()
+ * }
+ *
+ */
+#define  FTC_CACHE_TRYLOOP(cache)                                 \
+  {                                                               \
+    FTC_Manager   _try_manager = FTC_CACHE(cache)->manager;       \
+    FT_UInt       _try_count   = 4;                               \
+                                                                  \
+    for (;;)                                                      \
+    {                                                             \
+      FT_UInt  _try_done;
+
+
+#define  FTC_CACHE_TRYLOOP_END()                                              \
+      if ( !error || error != FT_Err_Out_Of_Memory )                          \
+        break;                                                                \
+                                                                              \
+      _try_done = FTC_Manager_FlushN( _try_manager, _try_count );             \
+      if ( _try_done == 0 )                                                   \
+        break;                                                                \
+                                                                              \
+      if ( _try_done == _try_count )                                          \
+      {                                                                       \
+        _try_count *= 2;                                                      \
+        if ( _try_count < _try_done || _try_count > _try_manager->num_nodes ) \
+          _try_count = _try_manager->num_nodes;                               \
+      }                                                                       \
+    }                                                                         \
+  }
+
  /* */
 
 FT_END_HEADER
diff --git a/src/cache/ftccache.c b/src/cache/ftccache.c
index 3a8a40c..8c7758a 100644
--- a/src/cache/ftccache.c
+++ b/src/cache/ftccache.c
@@ -424,65 +424,29 @@
     FT_Error  error;
     FTC_Node  node;
 
-    /*
-     *  Try to allocate a new cache node.  Note that in case of
-     *  out-of-memory error (OOM), we'll flush the cache a bit,
-     *  then try again.
-     *
-     *  On each try, the `tries' variable gives the number
-     *  of old nodes we want to flush from the manager's global list
-     *  before the next allocation attempt.  It barely doubles on
-     *  each iteration.
-     *
-     */
-    error = cache->clazz.node_new( &node, query, cache );
-    if ( error )
-      goto FlushCache;
+   /* we use the FTC_CACHE_TRYLOOP macros in order to
+    * support out-of-memory error (OOM) correctly, i.e.
+    * by flushing the cache progressively in order to
+    * make more room
+    */
+    FTC_CACHE_TRYLOOP(cache)
+    {
+      error = cache->clazz.node_new( &node, query, cache );
+    }
+    FTC_CACHE_TRYLOOP_END();
 
-  AddNode:
-    /* don't assume that the cache has the same number of buckets, since
-     * our allocation request might have triggered global cache flushing
-     */
-    ftc_cache_add( cache, hash, node );
+    if ( error )
+      node = NULL;
+    else
+    {
+     /* don't assume that the cache has the same number of buckets, since
+      * our allocation request might have triggered global cache flushing
+      */
+      ftc_cache_add( cache, hash, node );
+    }
 
-  Exit:
     *anode = node;
     return error;
-
-  FlushCache:
-    node = NULL;
-    if ( error != FT_Err_Out_Of_Memory )
-      goto Exit;
-
-    {
-      FTC_Manager  manager = cache->manager;
-      FT_UInt      count, tries = 1;
-
-
-      for (;;)
-      {
-        error = cache->clazz.node_new( &node, query, cache );
-        if ( !error )
-          break;
-
-        node = NULL;
-        if ( error != FT_Err_Out_Of_Memory )
-          goto Exit;
-
-        count = FTC_Manager_FlushN( manager, tries );
-        if ( count == 0 )
-          goto Exit;
-
-        if ( count == tries )
-        {
-          count = tries * 2;
-          if ( count < tries || count > manager->num_nodes )
-            count = manager->num_nodes;
-        }
-        tries = count;
-      }
-    }
-    goto AddNode;
   }
 
 
diff --git a/src/cache/ftcsbits.c b/src/cache/ftcsbits.c
index 7159971..51ec34c 100644
--- a/src/cache/ftcsbits.c
+++ b/src/cache/ftcsbits.c
@@ -85,6 +85,15 @@
   }
 
 
+ /* this function tries to load a small bitmap within a given FTC_SNode
+  * note that it will return a non-zero error code _only_ in the case
+  * of out-of-memory condition. For all other errors (e.g. corresponding
+  * to a bad font file), this function will mark the sbit as "unavailable"
+  * and return a value of 0.
+  *
+  * you should also read the comment within the @ftc_snode_compare function
+  * below to see how out-of-memory is handled during a lookup
+  */
   static FT_Error
   ftc_snode_load( FTC_SNode    snode,
                   FTC_Manager  manager,
@@ -315,16 +324,54 @@
       FTC_SBit  sbit = snode->sbits + ( gindex - gnode->gindex );
 
 
+     /* the following code illustrates what to do when you want to
+      * perform operations that may fail within a lookup function.
+      *
+      * here, we want to load a small bitmap on-demand, we thus
+      * need to call the 'ftc_snode_load' function which may return
+      * a non-zero error code only when we're out of memory
+      *
+      * the correct thing to do is to use @FTC_CACHE_TRYLOOP and
+      * @FTC_CACHE_TRYLOOP_END in order to implement a retry loop
+      * that is capable of flushing the cache incrementally when
+      * OOM errors occur.
+      *
+      * however, we previously need to 'lock' the node to prevent it
+      * from being flushed in the loop.
+      *
+      * when we exit the loop, we unlock the node then check the 'error' 
+      * variable. If it is non-zero, this means that the cache was
+      * completely flushed and that no usable memory was found to load
+      * the bitmap.
+      *
+      * we then prefer to return a value of 0 (i.e. NO MATCH). This
+      * will ensure that the caller will try to allocate a new node.
+      * this operation _will_ fail and the lookup function will return
+      * the OOM error code appropriately.
+      *
+      * note that 'buffer == NULL && width == 255' is a hack used to
+      * tag "unavailable" bitmaps in the array. We should never try
+      * to load these.
+      */
       if ( sbit->buffer == NULL && sbit->width != 255 )
       {
         FT_ULong  size;
+        FT_Error  error;
 
+        ftcsnode->ref_count++;  /* lock node, prevent flushing in retry loop */
 
-        if ( !ftc_snode_load( snode, cache->manager,
-                              gindex, &size ) )
+        FTC_CACHE_TRYLOOP(cache)
         {
-          cache->manager->cur_weight += size;
+          error = ftc_snode_load( snode, cache->manager, gindex, &size );
         }
+        FTC_CACHE_TRYLOOP_END();
+
+        ftcsnode->ref_count--;  /* unlock the node */
+
+        if ( error )
+          result = 0;
+        else
+          cache->manager->cur_weight += size;
       }
     }