From 7ed3cb89923c376d8b30ae3f977ab9e1a231e430 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 20 Dec 2012 00:51:55 -0500 Subject: Bug 689342 - Lifetime of patterns in pattern_hash may use too much memory Refcount cached patterns. --- pango/pangofc-fontmap.c | 207 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 161 insertions(+), 46 deletions(-) (limited to 'pango/pangofc-fontmap.c') diff --git a/pango/pangofc-fontmap.c b/pango/pangofc-fontmap.c index 78ce5d73..017fe44c 100644 --- a/pango/pangofc-fontmap.c +++ b/pango/pangofc-fontmap.c @@ -44,7 +44,10 @@ * - All FcPattern's referenced by any object in the fontmap are uniquified * and cached in the fontmap. This both speeds lookups based on patterns * faster, and saves memory. This is handled by fontmap->priv->pattern_hash. - * The patterns are cached indefinitely. + * The patterns are refcounted via PangoFcUPattern and removed from + * pattern_hash when the reference count reaches zero. This avoids an ever + * growing pattern_hash when a very large number of patterns are used within + * the same font map (think an infinitely varied zoom factor on the text). * * - The results of a FcFontSort() are used to populate fontsets. However, * FcFontSort() relies on the search pattern only, which includes the font @@ -89,6 +92,7 @@ typedef struct _PangoFcFontFaceData PangoFcFontFaceData; typedef struct _PangoFcFace PangoFcFace; typedef struct _PangoFcFamily PangoFcFamily; typedef struct _PangoFcFindFuncInfo PangoFcFindFuncInfo; +typedef struct _PangoFcUPattern PangoFcUPattern; typedef struct _PangoFcPatterns PangoFcPatterns; typedef struct _PangoFcFontset PangoFcFontset; @@ -111,7 +115,7 @@ struct _PangoFcFontMapPrivate GHashTable *font_hash; /* Maps PangoFcFontKey -> PangoFcFont */ - GHashTable *patterns_hash; /* Maps FcPattern -> PangoFcPatterns */ + GHashTable *patterns_hash; /* Maps PangoFcUPattern -> PangoFcPatterns */ /* pattern_hash is used to make sure we only store one copy of * each identical pattern. (Speeds up lookup). @@ -215,13 +219,24 @@ static gboolean pango_fc_fontset_key_equal (const PangoFcFontsetKey *k static void pango_fc_font_key_init (PangoFcFontKey *key, PangoFcFontMap *fcfontmap, PangoFcFontsetKey *fontset_key, - FcPattern *pattern); + PangoFcUPattern *pattern); static PangoFcFontKey *pango_fc_font_key_copy (const PangoFcFontKey *key); static void pango_fc_font_key_free (PangoFcFontKey *key); static guint pango_fc_font_key_hash (const PangoFcFontKey *key); static gboolean pango_fc_font_key_equal (const PangoFcFontKey *key_a, const PangoFcFontKey *key_b); +static guint pango_fc_upattern_hash (const PangoFcUPattern *upattern); +static gboolean pango_fc_upattern_equal (const PangoFcUPattern *upattern_a, + const PangoFcUPattern *upattern_b); +static void pango_fc_upattern_free (PangoFcUPattern *upattern); +static PangoFcUPattern *pango_fc_upattern_create (FcPattern *pattern); +static PangoFcUPattern *pango_fc_upattern_init_key (PangoFcUPattern *upattern, + FcPattern *pattern); +static PangoFcUPattern *pango_fc_upattern_ref (PangoFcUPattern *upattern); +static void pango_fc_upattern_unref (PangoFcUPattern *upattern, + PangoFcFontMap *fcfontmap); + static PangoFcPatterns *pango_fc_patterns_new (FcPattern *pat, PangoFcFontMap *fontmap); static PangoFcPatterns *pango_fc_patterns_ref (PangoFcPatterns *pats); @@ -229,8 +244,8 @@ static void pango_fc_patterns_unref (PangoFcPatterns *pats); static FcPattern *pango_fc_patterns_get_pattern (PangoFcPatterns *pats); static FcPattern *pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i); -static FcPattern *uniquify_pattern (PangoFcFontMap *fcfontmap, - FcPattern *pattern); +static PangoFcUPattern *uniquify_pattern (PangoFcFontMap *fcfontmap, + FcPattern *pattern); static gpointer get_gravity_class (void) @@ -324,6 +339,87 @@ get_scaled_size (PangoFcFontMap *fcfontmap, } +/* + * PangoFcUPattern + */ + +struct _PangoFcUPattern { + FcPattern *pattern; /* only element that matters for equality in pattern_hash */ + guint ref_count; /* references from outside the pattern_hash */ +}; + +static guint +pango_fc_upattern_hash (const PangoFcUPattern *upattern) +{ + return FcPatternHash(upattern->pattern); +} + +static gboolean +pango_fc_upattern_equal (const PangoFcUPattern *upattern_a, const PangoFcUPattern *upattern_b) +{ + return FcPatternEqual(upattern_a->pattern, upattern_b->pattern); +} + +static void +pango_fc_upattern_free (PangoFcUPattern *upattern) +{ + if (upattern->ref_count == 0) + { + FcPatternDestroy(upattern->pattern); + g_slice_free(PangoFcUPattern, upattern); + } +} + +static PangoFcUPattern * +pango_fc_upattern_create(FcPattern *pattern) +{ + PangoFcUPattern *upat; + + upat = g_slice_new(PangoFcUPattern); + + FcPatternReference(pattern); + + upat->pattern = pattern; + upat->ref_count = 1; + + return upat; +} + +static PangoFcUPattern * +pango_fc_upattern_init_key (PangoFcUPattern *upattern, FcPattern *pattern) +{ + upattern->pattern = pattern; + upattern->ref_count = 0; + + return upattern; +} + +static PangoFcUPattern * +pango_fc_upattern_ref (PangoFcUPattern *upattern) +{ + g_return_val_if_fail (upattern->ref_count > 0, NULL); + + upattern->ref_count++; + + return upattern; +} + +static void +pango_fc_upattern_unref (PangoFcUPattern *upattern, PangoFcFontMap *fcfontmap) +{ + g_return_if_fail (upattern->ref_count > 0); + + if ( --upattern->ref_count > 0 ) + return; + + /* Only remove from pattern_hash if we are really in it (and not just the same + * pattern). This is not necessarily the case after a cache_clear() call. */ + if (fcfontmap && fcfontmap->priv->pattern_hash && + upattern == g_hash_table_lookup(fcfontmap->priv->pattern_hash, upattern)) + g_hash_table_remove (fcfontmap->priv->pattern_hash, upattern); + else + pango_fc_upattern_free(upattern); +} struct _PangoFcFontsetKey { PangoFcFontMap *fontmap; @@ -337,7 +433,7 @@ struct _PangoFcFontsetKey { struct _PangoFcFontKey { PangoFcFontMap *fontmap; - FcPattern *pattern; + PangoFcUPattern *upattern; PangoMatrix matrix; gpointer context_key; }; @@ -544,7 +640,7 @@ static gboolean pango_fc_font_key_equal (const PangoFcFontKey *key_a, const PangoFcFontKey *key_b) { - if (key_a->pattern == key_b->pattern && + if (key_a->upattern == key_b->upattern && 0 == memcmp (&key_a->matrix, &key_b->matrix, 4 * sizeof (double))) { if (key_a->context_key && key_b->context_key) @@ -570,14 +666,14 @@ pango_fc_font_key_hash (const PangoFcFontKey *key) hash ^= PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_hash (key->fontmap, key->context_key); - return (hash ^ GPOINTER_TO_UINT (key->pattern)); + return (hash ^ GPOINTER_TO_UINT (key->upattern)); } static void pango_fc_font_key_free (PangoFcFontKey *key) { - if (key->pattern) - FcPatternDestroy (key->pattern); + if (key->upattern) + pango_fc_upattern_unref(key->upattern, key->fontmap); if (key->context_key) PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_free (key->fontmap, @@ -592,8 +688,8 @@ pango_fc_font_key_copy (const PangoFcFontKey *old) PangoFcFontKey *key = g_slice_new (PangoFcFontKey); key->fontmap = old->fontmap; - FcPatternReference (old->pattern); - key->pattern = old->pattern; + pango_fc_upattern_ref(old->upattern); + key->upattern = old->upattern; key->matrix = old->matrix; if (old->context_key) key->context_key = PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_copy (key->fontmap, @@ -608,10 +704,10 @@ static void pango_fc_font_key_init (PangoFcFontKey *key, PangoFcFontMap *fcfontmap, PangoFcFontsetKey *fontset_key, - FcPattern *pattern) + PangoFcUPattern *upattern) { key->fontmap = fcfontmap; - key->pattern = pattern; + key->upattern = upattern; key->matrix = *pango_fc_fontset_key_get_matrix (fontset_key); key->context_key = pango_fc_fontset_key_get_context_key (fontset_key); } @@ -631,7 +727,7 @@ pango_fc_font_key_init (PangoFcFontKey *key, const FcPattern * pango_fc_font_key_get_pattern (const PangoFcFontKey *key) { - return key->pattern; + return key->upattern->pattern; } /** @@ -676,7 +772,7 @@ struct _PangoFcPatterns { PangoFcFontMap *fontmap; - FcPattern *pattern; + PangoFcUPattern *upattern; FcPattern *match; FcFontSet *fontset; }; @@ -685,22 +781,25 @@ static PangoFcPatterns * pango_fc_patterns_new (FcPattern *pat, PangoFcFontMap *fontmap) { PangoFcPatterns *pats; + PangoFcUPattern *upat; - pat = uniquify_pattern (fontmap, pat); - pats = g_hash_table_lookup (fontmap->priv->patterns_hash, pat); + upat = uniquify_pattern (fontmap, pat); + pats = g_hash_table_lookup (fontmap->priv->patterns_hash, upat); if (pats) - return pango_fc_patterns_ref (pats); + { + pango_fc_upattern_unref(upat, fontmap); + return pango_fc_patterns_ref (pats); + } pats = g_slice_new0 (PangoFcPatterns); pats->fontmap = fontmap; pats->ref_count = 1; - FcPatternReference (pat); - pats->pattern = pat; + pats->upattern = upat; g_hash_table_insert (fontmap->priv->patterns_hash, - pats->pattern, pats); + pats->upattern, pats); return pats; } @@ -728,12 +827,12 @@ pango_fc_patterns_unref (PangoFcPatterns *pats) /* Only remove from fontmap hash if we are in it. This is not necessarily * the case after a cache_clear() call. */ if (pats->fontmap->priv->patterns_hash && - pats == g_hash_table_lookup (pats->fontmap->priv->patterns_hash, pats->pattern)) + pats == g_hash_table_lookup (pats->fontmap->priv->patterns_hash, pats->upattern)) g_hash_table_remove (pats->fontmap->priv->patterns_hash, - pats->pattern); + pats->upattern); - if (pats->pattern) - FcPatternDestroy (pats->pattern); + if (pats->upattern) + pango_fc_upattern_unref (pats->upattern, pats->fontmap); if (pats->match) FcPatternDestroy (pats->match); @@ -747,7 +846,7 @@ pango_fc_patterns_unref (PangoFcPatterns *pats) static FcPattern * pango_fc_patterns_get_pattern (PangoFcPatterns *pats) { - return pats->pattern; + return pats->upattern->pattern; } static FcPattern * @@ -758,7 +857,7 @@ pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i) FcResult result; if (!pats->match && !pats->fontset) { - pats->match = FcFontMatch (NULL, pats->pattern, &result); + pats->match = FcFontMatch (NULL, pats->upattern->pattern, &result); } if (pats->match) @@ -769,7 +868,7 @@ pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i) if (!pats->fontset) { FcResult result; - pats->fontset = FcFontSort (NULL, pats->pattern, FcTrue, NULL, &result); + pats->fontset = FcFontSort (NULL, pats->upattern->pattern, FcTrue, NULL, &result); if (pats->match) { FcPatternDestroy (pats->match); @@ -1045,9 +1144,9 @@ pango_fc_font_map_init (PangoFcFontMap *fcfontmap) priv->patterns_hash = g_hash_table_new (NULL, NULL); - priv->pattern_hash = g_hash_table_new_full ((GHashFunc) FcPatternHash, - (GEqualFunc) FcPatternEqual, - (GDestroyNotify) FcPatternDestroy, + priv->pattern_hash = g_hash_table_new_full ((GHashFunc) pango_fc_upattern_hash, + (GEqualFunc) pango_fc_upattern_equal, + (GDestroyNotify) pango_fc_upattern_free, NULL); priv->font_face_data_hash = g_hash_table_new_full ((GHashFunc)pango_fc_font_face_data_hash, @@ -1482,23 +1581,27 @@ pango_fc_make_pattern (const PangoFontDescription *description, return pattern; } -static FcPattern * +/* The returned PangoFcUPattern should be unreferenced via + * pango_fc_upattern_unref() when done with it. */ +static PangoFcUPattern * uniquify_pattern (PangoFcFontMap *fcfontmap, FcPattern *pattern) { PangoFcFontMapPrivate *priv = fcfontmap->priv; - FcPattern *old_pattern; + PangoFcUPattern upat_key; + PangoFcUPattern *upattern; - old_pattern = g_hash_table_lookup (priv->pattern_hash, pattern); - if (old_pattern) + upattern = g_hash_table_lookup (priv->pattern_hash, + pango_fc_upattern_init_key(&upat_key, pattern)); + if ( upattern ) { - return old_pattern; + return pango_fc_upattern_ref(upattern); } else { - FcPatternReference (pattern); - g_hash_table_insert (priv->pattern_hash, pattern, pattern); - return pattern; + upattern = pango_fc_upattern_create(pattern); + g_hash_table_insert (priv->pattern_hash, upattern, upattern); + return upattern; } } @@ -1509,6 +1612,7 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, { PangoFcFontMapClass *class; PangoFcFontMapPrivate *priv = fcfontmap->priv; + PangoFcUPattern *umatch; FcPattern *pattern; PangoFcFont *fcfont; PangoFcFontKey key; @@ -1516,13 +1620,16 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, if (priv->closed) return NULL; - match = uniquify_pattern (fcfontmap, match); + umatch = uniquify_pattern (fcfontmap, match); - pango_fc_font_key_init (&key, fcfontmap, fontset_key, match); + pango_fc_font_key_init (&key, fcfontmap, fontset_key, umatch); fcfont = g_hash_table_lookup (priv->font_hash, &key); if (fcfont) - return g_object_ref (fcfont); + { + pango_fc_upattern_unref(umatch, fcfontmap); + return g_object_ref (fcfont); + } class = PANGO_FC_FONT_MAP_GET_CLASS (fcfontmap); @@ -1534,6 +1641,7 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, { const PangoMatrix *pango_matrix = pango_fc_fontset_key_get_matrix (fontset_key); FcMatrix fc_matrix, *fc_matrix_val; + PangoFcUPattern *upattern; int i; /* Fontconfig has the Y axis pointing up, Pango, down. @@ -1543,7 +1651,7 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, fc_matrix.yx = - pango_matrix->yx; fc_matrix.yy = pango_matrix->yy; - pattern = FcPatternDuplicate (match); + pattern = FcPatternDuplicate (umatch->pattern); for (i = 0; FcPatternGetMatrix (pattern, FC_MATRIX, i, &fc_matrix_val) == FcResultMatch; i++) FcMatrixMultiply (&fc_matrix, &fc_matrix, fc_matrix_val); @@ -1551,13 +1659,18 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, FcPatternDel (pattern, FC_MATRIX); FcPatternAddMatrix (pattern, FC_MATRIX, &fc_matrix); - fcfont = class->new_font (fcfontmap, uniquify_pattern (fcfontmap, pattern)); + upattern = uniquify_pattern (fcfontmap, pattern); + fcfont = class->new_font (fcfontmap, upattern->pattern); + pango_fc_upattern_unref(upattern, fcfontmap); FcPatternDestroy (pattern); } if (!fcfont) - return NULL; + { + pango_fc_upattern_unref(umatch, fcfontmap); + return NULL; + } fcfont->matrix = key.matrix; /* In case the backend didn't set the fontmap */ @@ -1569,6 +1682,8 @@ pango_fc_font_map_new_font (PangoFcFontMap *fcfontmap, /* cache it on fontmap */ pango_fc_font_map_add (fcfontmap, &key, fcfont); + pango_fc_upattern_unref(umatch, fcfontmap); + return (PangoFont *)fcfont; } -- cgit v1.2.1