Attachment 3260 Details for Bug 4299 – BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy

[patch] BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy

4299-branch-1.0-1.patch (text/plain), 4.40 KB, created by Carl Worth on 2005-09-13 15:39:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Carl Worth

Created: 2005-09-13 15:39:52 UTC

Size: 4.40 KB

patch

obsolete

>Index: src/cairo-font.c
>===================================================================
>RCS file: /mirrors/freedesktop/cairo/cairo/src/cairo-font.c,v
>retrieving revision 1.76
>retrieving revision 1.76.2.1
>diff -u -p -r1.76 -r1.76.2.1
>--- src/cairo-font.c	24 Aug 2005 04:36:08 -0000	1.76
>+++ src/cairo-font.c	13 Sep 2005 19:28:00 -0000	1.76.2.1
>@@ -762,6 +762,8 @@ UNWIND:
> cairo_scaled_font_t *
> cairo_scaled_font_reference (cairo_scaled_font_t *scaled_font)
> {
>+    cairo_scaled_font_map_t *font_map;
>+
>     if (scaled_font == NULL)
> 	return NULL;
> 
>@@ -772,16 +774,19 @@ cairo_scaled_font_reference (cairo_scale
>      * we are using ref_count == 0 as a legitimate case for the
>      * holdovers array. See below. */
> 
>-    /* If the original reference count is 0, then this font must have
>-     * been found in font_map->holdovers, (which means this caching is
>-     * actually working). So now we remove it from the holdovers
>-     * array. */
>-    if (scaled_font->ref_count == 0) {
>-	cairo_scaled_font_map_t *font_map;
>-	int i;
>+    /* cairo_scaled_font_t objects are cached and shared between
>+     * threads. This works because these objects are immutable. Except
>+     * that the reference count is mutable, so we have to do locking
>+     * around any modification of the reference count. */
>+    font_map = _cairo_scaled_font_map_lock ();
>+    {
>+	/* If the original reference count is 0, then this font must have
>+	 * been found in font_map->holdovers, (which means this caching is
>+	 * actually working). So now we remove it from the holdovers
>+	 * array. */
>+	if (scaled_font->ref_count == 0) {
>+	    int i;
> 
>-	font_map = _cairo_scaled_font_map_lock ();
>-	{
> 	    for (i = 0; i < font_map->num_holdovers; i++)
> 		if (font_map->holdovers[i] == scaled_font)
> 		    break;
>@@ -792,10 +797,11 @@ cairo_scaled_font_reference (cairo_scale
> 		     &font_map->holdovers[i+1],
> 		     (font_map->num_holdovers - i) * sizeof (cairo_scaled_font_t*));
> 	}
>-	_cairo_scaled_font_map_unlock ();
>-    }
> 
>-    scaled_font->ref_count++;
>+	scaled_font->ref_count++;
>+
>+    }
>+    _cairo_scaled_font_map_unlock ();
> 
>     return scaled_font;
> }
>@@ -819,38 +825,43 @@ cairo_scaled_font_destroy (cairo_scaled_
>     if (scaled_font->ref_count == (unsigned int)-1)
> 	return;
> 
>-    assert (scaled_font->ref_count > 0);
>-
>-    if (--(scaled_font->ref_count) > 0)
>-	return;
>-
>+    /* cairo_scaled_font_t objects are cached and shared between
>+     * threads. This works because these objects are immutable. Except
>+     * that the reference count is mutable, so we have to do locking
>+     * around any modification of the reference count. */
>     font_map = _cairo_scaled_font_map_lock ();
>-    assert (font_map != NULL);
>     {
>-	/* Rather than immediately destroying this object, we put it into
>-	 * the font_map->holdovers array in case it will get used again
>-	 * soon. To make room for it, we do actually destroy the
>-	 * least-recently-used holdover.
>-	 */
>-	if (font_map->num_holdovers == CAIRO_SCALED_FONT_MAX_HOLDOVERS) {
>-	    cairo_scaled_font_t *lru;
>+	assert (font_map != NULL);
>+
>+	assert (scaled_font->ref_count > 0);
>+
>+	if (--(scaled_font->ref_count) == 0)
>+	{
>+	    /* Rather than immediately destroying this object, we put it into
>+	     * the font_map->holdovers array in case it will get used again
>+	     * soon. To make room for it, we do actually destroy the
>+	     * least-recently-used holdover.
>+	     */
>+	    if (font_map->num_holdovers == CAIRO_SCALED_FONT_MAX_HOLDOVERS) {
>+		cairo_scaled_font_t *lru;
> 
>-	    lru = font_map->holdovers[0];
>-	    assert (lru->ref_count == 0);
>+		lru = font_map->holdovers[0];
>+		assert (lru->ref_count == 0);
> 	
>-	    _cairo_hash_table_remove (font_map->hash_table, &lru->hash_entry);
>+		_cairo_hash_table_remove (font_map->hash_table, &lru->hash_entry);
> 
>-	    _cairo_scaled_font_fini (lru);
>-	    free (lru);
>+		_cairo_scaled_font_fini (lru);
>+		free (lru);
> 	
>-	    font_map->num_holdovers--;
>-	    memmove (&font_map->holdovers[0],
>-		     &font_map->holdovers[1],
>-		     font_map->num_holdovers * sizeof (cairo_scaled_font_t*));
>-	}
>+		font_map->num_holdovers--;
>+		memmove (&font_map->holdovers[0],
>+			 &font_map->holdovers[1],
>+			 font_map->num_holdovers * sizeof (cairo_scaled_font_t*));
>+	    }
> 
>-	font_map->holdovers[font_map->num_holdovers] = scaled_font;
>-	font_map->num_holdovers++;
>+	    font_map->holdovers[font_map->num_holdovers] = scaled_font;
>+	    font_map->num_holdovers++;
>+	}
>     }
>     _cairo_scaled_font_map_unlock ();
> }

Actions: View | Splinter Review

Attachments on bug 4299: 3146 | 3147 | 3148 | 3202 | 3243 | 3253 | 3254 | 3260 | 3261 | 3274 | 3275

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.