pango r2704 - in trunk: . pango

[behdad svn gnome org]

Author: behdad
Date: Fri Aug 22 07:01:35 2008
New Revision: 2704
URL: http://svn.gnome.org/viewvc/pango?rev=2704&view=rev

Log:
2008-08-22  Behdad Esfahbod  <behdad gnome org>

        Bug 143542 â PangoFT2Fontmap leak

        * pango/fonts.c:
        * pango/pangoatsui.c (pango_atsui_font_finalize),
        (_pango_atsui_font_set_font_map):
        * pango/pangocairo-font.c (_pango_cairo_font_get_metrics),
        (_pango_cairo_font_private_get_hex_box_info):
        * pango/pangocairo-win32font.c (_pango_cairo_win32_font_new):
        * pango/pangofc-font.c (pango_fc_font_get_metrics):
        * pango/pangofc-fontmap.c (pango_fc_font_map_add),
        (_pango_fc_font_map_remove), (cleanup_font):
        * pango/pangowin32-fontmap.c (pango_win32_font_neww),
        (pango_win32_font_map_real_find_font):
        * pango/pangowin32.c (pango_win32_font_finalize):
        * pango/pangox-fontmap.c (pango_x_font_map_load_font):
        * pango/pangox.c (pango_x_font_new), (pango_x_font_finalize):
        Make the reference the font->fontmap reference weak.

        The code for setting the reference must look like this:

          g_assert (font->fontmap == NULL);
          font->fontmap = (PangoFontMap *) fontmap;
          g_object_add_weak_pointer (G_OBJECT (font->fontmap), (gpointer *) (gpointer) &font->fontmap);

        And releasing it like:

          g_assert (font->fontmap != NULL);
          g_object_remove_weak_pointer (G_OBJECT (font->fontmap), (gpointer *) (gpointer) &font->fontmap);
          font->fontmap = NULL;

        I have converted all fontmaps.  The win32 and atsui ones can
        use some simple testing.

        The PangoFc fonts actually don't need the weakref as the fontmap
        already provides a similar link by itself.

Modified:
   trunk/ChangeLog
   trunk/pango/fonts.c
   trunk/pango/pangoatsui.c
   trunk/pango/pangocairo-font.c
   trunk/pango/pangocairo-win32font.c
   trunk/pango/pangofc-font.c
   trunk/pango/pangofc-fontmap.c
   trunk/pango/pangowin32-fontmap.c
   trunk/pango/pangowin32.c
   trunk/pango/pangox-fontmap.c
   trunk/pango/pangox.c

Modified: trunk/pango/fonts.c
==============================================================================
--- trunk/pango/fonts.c	(original)
+++ trunk/pango/fonts.c	Fri Aug 22 07:01:35 2008
@@ -1414,10 +1414,18 @@
 
 /**
  * pango_font_get_font_map:
- * @font: a #PangoFont
+ * @font: a #PangoFont, or %NULL
  *
  * Gets the font map for which the font was created.
  *
+ * Note that the font maintains a <firstterm>weak</firstterm> reference
+ * to the font map, so if all references to font map are dropped, the font
+ * map will be finalized even if there are fonts created with the font
+ * map that are still alive.  In that case this function will return %NULL.
+ * It is the responsibility of the user to ensure that the font map is kept
+ * alive.  In most uses this is not an issue as a #PangoContext holds
+ * a reference to the font map.
+ *
  * Return value: the #PangoFontMap for the font, or %NULL if @font is %NULL.
  *
  * Since: 1.10

Modified: trunk/pango/pangoatsui.c
==============================================================================
--- trunk/pango/pangoatsui.c	(original)
+++ trunk/pango/pangoatsui.c	Fri Aug 22 07:01:35 2008
@@ -45,7 +45,9 @@
 
   pango_font_description_free (priv->desc);
 
-  g_object_unref (priv->fontmap);
+  g_assert (priv->fontmap != NULL);
+  g_object_remove_weak_pointer (G_OBJECT (priv->fontmap), (gpointer *) (gpointer) &priv->fontmap);
+  priv->fontmap = NULL;
 
   G_OBJECT_CLASS (pango_atsui_font_parent_class)->finalize (object);
 }
@@ -135,8 +137,8 @@
   PangoATSUIFontPrivate *priv = font->priv;
 
   g_assert (priv->fontmap == NULL);
-  
-  priv->fontmap = g_object_ref (fontmap);
+  priv->fontmap = (PangoFontMap *) fontmap;
+  g_object_add_weak_pointer (G_OBJECT (prev->fontmap), (gpointer *) (gpointer) &priv->fontmap);
 }
 
 void

Modified: trunk/pango/pangocairo-font.c
==============================================================================
--- trunk/pango/pangocairo-font.c	(original)
+++ trunk/pango/pangocairo-font.c	Fri Aug 22 07:01:35 2008
@@ -223,17 +223,24 @@
 
   if (!tmp_list)
     {
+      PangoFontMap *fontmap;
       PangoContext *context;
       cairo_font_options_t *font_options;
       int height, shift;
 
+      /* XXX this is racy.  need a ref'ing getter... */
+      fontmap = pango_font_get_font_map (font);
+      if (!fontmap)
+        return pango_font_metrics_new ();
+      fontmap = g_object_ref (fontmap);
+
       info = g_slice_new0 (PangoCairoFontMetricsInfo);
 
       cf_priv->metrics_by_lang = g_slist_prepend (cf_priv->metrics_by_lang, info);
 
       info->sample_str = sample_str;
 
-      context = pango_font_map_create_context (pango_font_get_font_map (font));
+      context = pango_font_map_create_context (fontmap);
       pango_context_set_language (context, language);
       font_options = cairo_font_options_create ();
       cairo_scaled_font_get_font_options (_pango_cairo_font_private_get_scaled_font (cf_priv), font_options);
@@ -271,6 +278,7 @@
       info->metrics->ascent = height - info->metrics->descent;
 
       g_object_unref (context);
+      g_object_unref (fontmap);
     }
 
   return pango_font_metrics_ref (info->metrics);
@@ -356,10 +364,14 @@
 
   /* create mini_font description */
   {
-    PangoContext *context;
     PangoFontMap *fontmap;
+    PangoContext *context;
 
+    /* XXX this is racy.  need a ref'ing getter... */
     fontmap = pango_font_get_font_map ((PangoFont *)cf_priv->cfont);
+    if (!fontmap)
+      return NULL;
+    fontmap = g_object_ref (fontmap);
 
     /* we inherit most font properties for the mini font.  just
      * change family and size.  means, you get bold hex digits
@@ -396,6 +408,7 @@
     mini_font = pango_font_map_load_font (fontmap, context, desc);
 
     g_object_unref (context);
+    g_object_unref (fontmap);
   }
 
   pango_font_description_free (desc);

Modified: trunk/pango/pangocairo-win32font.c
==============================================================================
--- trunk/pango/pangocairo-win32font.c	(original)
+++ trunk/pango/pangocairo-win32font.c	Fri Aug 22 07:01:35 2008
@@ -288,8 +288,9 @@
   cwfont = g_object_new (PANGO_TYPE_CAIRO_WIN32_FONT, NULL);
   win32font = PANGO_WIN32_FONT (cwfont);
 
-  win32font->fontmap = PANGO_FONT_MAP (cwfontmap);
-  g_object_ref (cwfontmap);
+  g_assert (win32font->fontmap == NULL);
+  win32font->fontmap = (PangoFontMap *) cwfontmap;
+  g_object_add_weak_pointer (G_OBJECT (win32font->fontmap), (gpointer *) (gpointer) &win32font->fontmap);
 
   win32font->win32face = face;
 

Modified: trunk/pango/pangofc-font.c
==============================================================================
--- trunk/pango/pangofc-font.c	(original)
+++ trunk/pango/pangofc-font.c	Fri Aug 22 07:01:35 2008
@@ -529,10 +529,14 @@
 
   if (!tmp_list)
     {
+      PangoFontMap *fontmap;
       PangoContext *context;
 
-      if (!fcfont->fontmap)
+      /* XXX this is racy.  because weakref's are racy... */
+      fontmap = fcfont->fontmap;
+      if (!fontmap)
 	return pango_font_metrics_new ();
+      fontmap = g_object_ref (fontmap);
 
       info = g_slice_new0 (PangoFcMetricsInfo);
 
@@ -541,12 +545,13 @@
 
       info->sample_str = sample_str;
 
-      context = pango_font_map_create_context (fcfont->fontmap);
+      context = pango_font_map_create_context (fontmap);
       pango_context_set_language (context, language);
 
       info->metrics = pango_fc_font_create_metrics_for_context (fcfont, context);
 
       g_object_unref (context);
+      g_object_unref (fontmap);
     }
 
   return pango_font_metrics_ref (info->metrics);

Modified: trunk/pango/pangofc-fontmap.c
==============================================================================
--- trunk/pango/pangofc-fontmap.c	(original)
+++ trunk/pango/pangofc-fontmap.c	Fri Aug 22 07:01:35 2008
@@ -554,8 +554,11 @@
   FontHashKey *key_copy;
 
   g_assert (fcfont->fontmap == NULL);
-
-  fcfont->fontmap = g_object_ref (fcfontmap);
+  fcfont->fontmap = (PangoFontMap *) fcfontmap;
+  /* In other fontmaps we add a weak pointer on ->fontmap so the
+   * field is unset when fontmap is finalized.  We don't need it
+   * here though as PangoFcFontMap already cleans up fcfont->fontmap
+   * as part of it's caching scheme. */
 
   font_hash_key_for_context (fcfontmap, context, &key);
   key.pattern = fcfont->font_pattern;
@@ -597,7 +600,6 @@
   g_hash_table_remove (priv->font_hash, &key);
   fcfont->fontmap = NULL;
   _pango_fc_font_set_context_key (fcfont, NULL);
-  g_object_unref (fcfontmap);
 }
 
 static PangoFcFamily *
@@ -1435,7 +1437,6 @@
 {
   _pango_fc_font_shutdown (fcfont);
 
-  g_object_unref (fcfont->fontmap);
   fcfont->fontmap = NULL;
 }
 

Modified: trunk/pango/pangowin32-fontmap.c
==============================================================================
--- trunk/pango/pangowin32-fontmap.c	(original)
+++ trunk/pango/pangowin32-fontmap.c	Fri Aug 22 07:01:35 2008
@@ -667,8 +667,9 @@
 
   result = (PangoWin32Font *)g_object_new (PANGO_TYPE_WIN32_FONT, NULL);
 
+  g_assert (result->fontmap == NULL);
   result->fontmap = fontmap;
-  g_object_ref (fontmap);
+  g_object_add_weak_pointer (G_OBJECT (result->fontmap), (gpointer *) (gpointer) &result->fontmap);
 
   result->size = size;
   _pango_win32_make_matching_logfontw (fontmap, lfp, size, &result->logfontw);
@@ -713,7 +714,6 @@
   if (!win32font)
     return NULL;
 
-  win32font->fontmap = fontmap;
   win32font->win32face = face;
   face->cached_fonts = g_slist_prepend (face->cached_fonts, win32font);
 

Modified: trunk/pango/pangowin32.c
==============================================================================
--- trunk/pango/pangowin32.c	(original)
+++ trunk/pango/pangowin32.c	Fri Aug 22 07:01:35 2008
@@ -835,7 +835,9 @@
 
   g_hash_table_destroy (win32font->glyph_info);
 
-  g_object_unref (win32font->fontmap);
+  g_assert (win32font->fontmap != NULL);
+  g_object_remove_weak_pointer (G_OBJECT (win32font->fontmap), (gpointer *) (gpointer) &win32font->fontmap);
+  win32font->fontmap = NULL;
 
   G_OBJECT_CLASS (_pango_win32_font_parent_class)->finalize (object);
 }

Modified: trunk/pango/pangox-fontmap.c
==============================================================================
--- trunk/pango/pangox-fontmap.c	(original)
+++ trunk/pango/pangox-fontmap.c	Fri Aug 22 07:01:35 2008
@@ -519,7 +519,6 @@
 	    {
 	      PangoXFont *xfont = pango_x_font_new (fontmap, best_match->xlfd, size);
 
-	      xfont->fontmap = fontmap;
 	      xfont->xface = best_match;
 	      best_match->cached_fonts = g_slist_prepend (best_match->cached_fonts, xfont);
 

Modified: trunk/pango/pangox.c
==============================================================================
--- trunk/pango/pangox.c	(original)
+++ trunk/pango/pangox.c	Fri Aug 22 07:01:35 2008
@@ -298,8 +298,10 @@
 
   result = g_object_new (PANGO_TYPE_X_FONT, NULL);
 
+  g_assert (result->fontmap == NULL);
   result->fontmap = fontmap;
-  g_object_ref (fontmap);
+  g_object_add_weak_pointer (G_OBJECT (result->fontmap), (gpointer *) (gpointer) &result->fontmap);
+
   result->display = pango_x_fontmap_get_display (fontmap);
 
   result->fonts = g_strsplit(spec, ",", -1);
@@ -1318,7 +1320,9 @@
   if (xfont->xface)
     pango_x_face_remove (xfont->xface, (PangoFont *)xfont);
 
-  g_object_unref (xfont->fontmap);
+  g_assert (xfont->fontmap != NULL);
+  g_object_remove_weak_pointer (G_OBJECT (xfont->fontmap), (gpointer *) (gpointer) &xfont->fontmap);
+  xfont->fontmap = NULL;
 
   g_strfreev (xfont->fonts);