Attachment 7562 Details for Bug 8801 – add locking to surface/pattern/cache refcounting

[patch] add locking to surface/pattern/cache refcounting

s-when-using-text-rendering-in-multithreaded-apps-even-when-a-surface-is-used-by-only-one-thread.txt (text/plain), 6.14 KB, created by Monty Montgomery on 2006-10-27 17:45:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Monty Montgomery

Created: 2006-10-27 17:45:53 UTC

Size: 6.14 KB

patch

obsolete

>From 3c19951ec117cdefe7380498f000cb53b2bfa222 Mon Sep 17 00:00:00 2001
>From: Monty <xiphmont@deep-space-bananafish.mit.edu>
>Date: Fri, 27 Oct 2006 20:39:18 -0400
>Subject: [PATCH] Add locking around cache, surface refcounts and pattern refcounts to eliminate concurrency problems when using text rendering in multithreaded apps (even when a surface is used by only one thread)
>---
> src/cairo-cache.c       |    6 ++++++
> src/cairo-pattern.c     |   19 ++++++++++++++++---
> src/cairo-scaled-font.c |   10 ++++++----
> src/cairo-surface.c     |   20 +++++++++++++++++---
> 4 files changed, 45 insertions(+), 10 deletions(-)
>
>diff --git a/src/cairo-cache.c b/src/cairo-cache.c
>index feac8c7..9d64d27 100644
>--- a/src/cairo-cache.c
>+++ b/src/cairo-cache.c
>@@ -38,6 +38,8 @@
> 
> #include "cairoint.h"
> 
>+CAIRO_MUTEX_DECLARE (cairo_cache_refcount_mutex);
>+
> struct _cairo_cache {
>     cairo_hash_table_t *hash_table;
> 
>@@ -188,9 +190,11 @@ _cairo_cache_destroy (cairo_cache_t *cac
> void
> _cairo_cache_freeze (cairo_cache_t *cache)
> {
>+    CAIRO_MUTEX_LOCK (cairo_cache_refcount_mutex);
>     assert (cache->freeze_count >= 0);
> 
>     cache->freeze_count++;
>+    CAIRO_MUTEX_UNLOCK (cairo_cache_refcount_mutex);
> }
> 
> /**
>@@ -210,12 +214,14 @@ _cairo_cache_freeze (cairo_cache_t *cach
> void
> _cairo_cache_thaw (cairo_cache_t *cache)
> {
>+    CAIRO_MUTEX_LOCK (cairo_cache_refcount_mutex);
>     assert (cache->freeze_count > 0);
> 
>     cache->freeze_count--;
> 
>     if (cache->freeze_count == 0)
> 	_cairo_cache_shrink_to_accomodate (cache, 0);
>+    CAIRO_MUTEX_UNLOCK (cairo_cache_refcount_mutex);
> }
> 
> /**
>diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
>index 9589b34..58a89c7 100644
>--- a/src/cairo-pattern.c
>+++ b/src/cairo-pattern.c
>@@ -30,6 +30,8 @@
> 
> #include "cairoint.h"
> 
>+CAIRO_MUTEX_DECLARE (cairo_pattern_refcount_mutex);
>+
> const cairo_solid_pattern_t cairo_pattern_nil = {
>     { CAIRO_PATTERN_TYPE_SOLID, 	/* type */
>       CAIRO_REF_COUNT_INVALID,		/* ref_count */
>@@ -498,13 +500,18 @@ cairo_pattern_reference (cairo_pattern_t
>     if (pattern == NULL)
> 	return NULL;
> 
>-    if (pattern->ref_count == CAIRO_REF_COUNT_INVALID)
>+    CAIRO_MUTEX_LOCK (cairo_pattern_refcount_mutex);
>+
>+    if (pattern->ref_count == CAIRO_REF_COUNT_INVALID){
>+	CAIRO_MUTEX_UNLOCK (cairo_pattern_refcount_mutex);
> 	return pattern;
>+    }
> 
>     assert (pattern->ref_count > 0);
> 
>     pattern->ref_count++;
> 
>+    CAIRO_MUTEX_UNLOCK (cairo_pattern_refcount_mutex);
>     return pattern;
> }
> slim_hidden_def (cairo_pattern_reference);
>@@ -558,17 +565,23 @@ cairo_pattern_destroy (cairo_pattern_t *
>     if (pattern == NULL)
> 	return;
> 
>-    if (pattern->ref_count == CAIRO_REF_COUNT_INVALID)
>+    CAIRO_MUTEX_LOCK (cairo_pattern_refcount_mutex);
>+    if (pattern->ref_count == CAIRO_REF_COUNT_INVALID){
>+	CAIRO_MUTEX_UNLOCK (cairo_pattern_refcount_mutex);
> 	return;
>+    }
> 
>     assert (pattern->ref_count > 0);
> 
>     pattern->ref_count--;
>-    if (pattern->ref_count)
>+    if (pattern->ref_count){
>+	CAIRO_MUTEX_UNLOCK (cairo_pattern_refcount_mutex);
> 	return;
>+    }
> 
>     _cairo_pattern_fini (pattern);
>     free (pattern);
>+    CAIRO_MUTEX_UNLOCK (cairo_pattern_refcount_mutex);
> }
> slim_hidden_def (cairo_pattern_destroy);
> 
>diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
>index 0a96b9d..49a9c88 100755
>--- a/src/cairo-scaled-font.c
>+++ b/src/cairo-scaled-font.c
>@@ -906,6 +906,8 @@ _cairo_scaled_font_show_glyphs (cairo_sc
>     if (scaled_font->status)
> 	return scaled_font->status;
> 
>+    _cairo_scaled_font_freeze_cache (scaled_font);
>+
>     if (scaled_font->backend->show_glyphs != NULL) {
> 	status = scaled_font->backend->show_glyphs (scaled_font,
> 						    op, pattern,
>@@ -914,16 +916,16 @@ _cairo_scaled_font_show_glyphs (cairo_sc
> 						    dest_x, dest_y,
> 						    width, height,
> 						    glyphs, num_glyphs);
>-	if (status != CAIRO_INT_STATUS_UNSUPPORTED)
>+	if (status != CAIRO_INT_STATUS_UNSUPPORTED){
>+	    _cairo_scaled_font_thaw_cache (scaled_font);
> 	    return status;
>+	}
>     }
> 
>     /* Font display routine either does not exist or failed. */
> 
>     status = CAIRO_STATUS_SUCCESS;
> 
>-    _cairo_cache_freeze (scaled_font->glyphs);
>-
>     for (i = 0; i < num_glyphs; i++) {
> 	int x, y;
> 	cairo_surface_pattern_t glyph_pattern;
>@@ -1005,7 +1007,7 @@ _cairo_scaled_font_show_glyphs (cairo_sc
>     }
> 
> CLEANUP_MASK:
>-    _cairo_cache_thaw (scaled_font->glyphs);
>+    _cairo_scaled_font_thaw_cache (scaled_font);
> 
>     if (mask != NULL)
> 	cairo_surface_destroy (mask);
>diff --git a/src/cairo-surface.c b/src/cairo-surface.c
>index dfcf39d..f62f072 100644
>--- a/src/cairo-surface.c
>+++ b/src/cairo-surface.c
>@@ -41,6 +41,8 @@ #include "cairoint.h"
> #include "cairo-surface-fallback-private.h"
> #include "cairo-clip-private.h"
> 
>+CAIRO_MUTEX_DECLARE (cairo_surface_refcount_mutex);
>+
> const cairo_surface_t _cairo_surface_nil = {
>     &cairo_image_surface_backend,	/* backend */
>     CAIRO_SURFACE_TYPE_IMAGE,
>@@ -410,13 +412,18 @@ cairo_surface_reference (cairo_surface_t
>     if (surface == NULL)
> 	return NULL;
> 
>-    if (surface->ref_count == CAIRO_REF_COUNT_INVALID)
>+    CAIRO_MUTEX_LOCK (cairo_surface_refcount_mutex);
>+
>+    if (surface->ref_count == CAIRO_REF_COUNT_INVALID){
>+        CAIRO_MUTEX_UNLOCK (cairo_surface_refcount_mutex);
> 	return surface;
>+    }
> 
>     assert (surface->ref_count > 0);
> 
>     surface->ref_count++;
> 
>+    CAIRO_MUTEX_UNLOCK (cairo_surface_refcount_mutex);
>     return surface;
> }
> slim_hidden_def (cairo_surface_reference);
>@@ -435,20 +442,27 @@ cairo_surface_destroy (cairo_surface_t *
>     if (surface == NULL)
> 	return;
> 
>-    if (surface->ref_count == CAIRO_REF_COUNT_INVALID)
>+    CAIRO_MUTEX_LOCK (cairo_surface_refcount_mutex);
>+
>+    if (surface->ref_count == CAIRO_REF_COUNT_INVALID){
>+        CAIRO_MUTEX_UNLOCK (cairo_surface_refcount_mutex);
> 	return;
>+    }
> 
>     assert (surface->ref_count > 0);
> 
>     surface->ref_count--;
>-    if (surface->ref_count)
>+    if (surface->ref_count){
>+        CAIRO_MUTEX_UNLOCK (cairo_surface_refcount_mutex);
> 	return;
>+    }
> 
>     cairo_surface_finish (surface);
> 
>     _cairo_user_data_array_fini (&surface->user_data);
> 
>     free (surface);
>+    CAIRO_MUTEX_UNLOCK (cairo_surface_refcount_mutex);
> }
> slim_hidden_def(cairo_surface_destroy);
> 
>-- 
>1.4.1.1
>

Actions: View | Splinter Review

Attachments on bug 8801: 7562 | 7563 | 8470 | 8471

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.