From cb05ae43711960302948697f05c62f83f454b8b8 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 May 2005 13:04:53 +0000 Subject: [PATCH] * 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). --- ChangeLog | 8 +++- include/freetype/cache/ftccache.h | 44 ++++++++++++++++++ src/cache/ftccache.c | 74 ++++++++----------------------- src/cache/ftcsbits.c | 53 ++++++++++++++++++++-- 4 files changed, 120 insertions(+), 59 deletions(-) diff --git a/ChangeLog b/ChangeLog index b14f22a35..bfd27dab2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2005-05-23 David Turner + + * 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 * 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 7c7688dba..f44191581 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 3a8a40c00..8c7758a47 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 71599717c..51ec34c31 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; } }