[cairo] Multithreaded cairo-test

Chris Wilson chris at chris-wilson.co.uk
Fri Mar 23 05:29:04 PDT 2007


The first causality exposed by multi-threaded cairo-test is the
_cairo_xlib_screen_list which lacked reference counting of the
_cairo_xlib_screen_info_t.
--
Chris Wilson
-------------- next part --------------
diff --git a/src/cairo-xlib-private.h b/src/cairo-xlib-private.h
index 9ecf5fe..73a0466 100644
--- a/src/cairo-xlib-private.h
+++ b/src/cairo-xlib-private.h
@@ -48,6 +48,8 @@ struct _cairo_xlib_hook {
 
 struct _cairo_xlib_screen_info {
     cairo_xlib_screen_info_t *next;
+    cairo_xlib_screen_info_t *prev;
+    unsigned int ref_count;
 
     Display *display;
     Screen *screen;
@@ -61,6 +63,11 @@ struct _cairo_xlib_screen_info {
 cairo_private cairo_xlib_screen_info_t *
 _cairo_xlib_screen_info_get (Display *display, Screen *screen);
 
+cairo_private cairo_xlib_screen_info_t *
+_cairo_xlib_screen_info_reference (cairo_xlib_screen_info_t *info);
+cairo_private_no_warn void
+_cairo_xlib_screen_info_destroy (cairo_xlib_screen_info_t *info);
+
 cairo_private cairo_bool_t
 _cairo_xlib_add_close_display_hook (Display *display, void (*func) (Display *, void *), void *data, void *key);
 cairo_private_no_warn void
diff --git a/src/cairo-xlib-screen.c b/src/cairo-xlib-screen.c
index d27057e..f314aea 100644
--- a/src/cairo-xlib-screen.c
+++ b/src/cairo-xlib-screen.c
@@ -245,6 +245,50 @@ _cairo_xlib_init_screen_font_options (cairo_xlib_screen_info_t *info)
 
 static cairo_xlib_screen_info_t *_cairo_xlib_screen_list = NULL;
 
+cairo_xlib_screen_info_t *
+_cairo_xlib_screen_info_reference (cairo_xlib_screen_info_t *info)
+{
+    assert (info->ref_count > 0);
+    info->ref_count ++;
+    return info;
+}
+
+static void
+_cairo_xlib_screen_info_destroy_unlocked (cairo_xlib_screen_info_t *info)
+{
+    assert (info->ref_count > 0);
+    if (--info->ref_count)
+	return;
+
+    while (info->close_display_hooks) {
+	cairo_xlib_hook_t *hook = info->close_display_hooks;
+	info->close_display_hooks = hook->next;
+
+	hook->func (info->display, hook->data);
+
+	free (hook);
+    }
+
+    if (info->prev)
+	info->prev->next = info->next;
+    else
+	_cairo_xlib_screen_list = info->next;
+    if (info->next)
+	info->next->prev = info->prev;
+
+    free (info);
+}
+void
+_cairo_xlib_screen_info_destroy (cairo_xlib_screen_info_t *info)
+{
+    CAIRO_MUTEX_LOCK (_cairo_xlib_screen_mutex);
+
+    _cairo_xlib_screen_info_destroy_unlocked (info);
+
+    CAIRO_MUTEX_UNLOCK (_cairo_xlib_screen_mutex);
+}
+
+
 static int
 _cairo_xlib_close_display (Display *dpy, XExtCodes *codes)
 {
@@ -261,15 +305,7 @@ _cairo_xlib_close_display (Display *dpy, XExtCodes *codes)
 	if (info->display == dpy) {
 	    *prev = next;
 	    /* call all registered shutdown routines */
-	    while (info->close_display_hooks) {
-		cairo_xlib_hook_t *hook = info->close_display_hooks;
-		info->close_display_hooks = hook->next;
-
-		hook->func (dpy, hook->data);
-
-		free (hook);
-	    }
-	    free (info);
+	    _cairo_xlib_screen_info_destroy_unlocked (info);
 	} else {
 	    prev = &info->next;
 	}
@@ -294,40 +330,38 @@ _cairo_xlib_screen_info_reset (void)
 
     for (info = _cairo_xlib_screen_list; info; info = next) {
 	next = info->next;
-	while (info->close_display_hooks) {
-	    cairo_xlib_hook_t *hook = info->close_display_hooks;
-	    info->close_display_hooks = hook->next;
-	    free (hook);
-	}
-	free (info);
+	_cairo_xlib_screen_info_destroy_unlocked (info);
     }
 
     _cairo_xlib_screen_list = NULL;
 
     CAIRO_MUTEX_UNLOCK (_cairo_xlib_screen_mutex);
-
 }
 
 static cairo_xlib_screen_info_t *
 _cairo_xlib_screen_info_get_unlocked (Display *dpy, Screen *screen)
 {
     cairo_xlib_screen_info_t *info;
-    cairo_xlib_screen_info_t **prev;
     int event_base, error_base;
     XExtCodes *codes;
     cairo_bool_t seen_display = FALSE;
 
-    for (prev = &_cairo_xlib_screen_list; (info = *prev); prev = &(*prev)->next)
+    for (info = _cairo_xlib_screen_list; info; info = info->next)
     {
 	if (info->display == dpy) {
 	    seen_display = TRUE;
-	    if (info->screen == screen || screen == NULL) {
+	    if (info->screen == screen) {
 		/*
 		 * MRU the list
 		 */
-		if (prev != &_cairo_xlib_screen_list) {
-		    *prev = info->next;
+		if (info->prev) {
+		    info->prev->next = info->next;
+		    if (info->next)
+			info->next->prev = info->prev;
 		    info->next = _cairo_xlib_screen_list;
+		    info->prev = NULL;
+		    if (_cairo_xlib_screen_list)
+			_cairo_xlib_screen_list->prev = info;
 		    _cairo_xlib_screen_list = info;
 		}
 		break;
@@ -336,7 +370,7 @@ _cairo_xlib_screen_info_get_unlocked (Display *dpy, Screen *screen)
     }
 
     if (info)
-	return info;
+	return _cairo_xlib_screen_info_reference (info);
 
     info = malloc (sizeof (cairo_xlib_screen_info_t));
     if (!info)
@@ -352,6 +386,8 @@ _cairo_xlib_screen_info_get_unlocked (Display *dpy, Screen *screen)
 	XESetCloseDisplay (dpy, codes->extension, _cairo_xlib_close_display);
     }
 
+    info->ref_count = 1;
+
     info->display = dpy;
     info->screen = screen;
     info->has_render = (XRenderQueryExtension (dpy, &event_base, &error_base) &&
@@ -362,6 +398,9 @@ _cairo_xlib_screen_info_get_unlocked (Display *dpy, Screen *screen)
     _cairo_xlib_init_screen_font_options (info);
 
     info->next = _cairo_xlib_screen_list;
+    info->prev = NULL;
+    if (_cairo_xlib_screen_list)
+	_cairo_xlib_screen_list->prev = info;
     _cairo_xlib_screen_list = info;
 
     return info;
@@ -425,7 +464,9 @@ _cairo_xlib_add_close_display_hook (Display *dpy, void (*func) (Display *, void
 	hook->key = key;
 	hook->next = info->close_display_hooks;
 	info->close_display_hooks = hook;
+	info = _cairo_xlib_screen_info_reference (info);
     }
+    _cairo_xlib_screen_info_destroy_unlocked (info);
 
     success = TRUE;
  unlock:
@@ -449,11 +490,13 @@ _cairo_xlib_remove_close_display_hook (Display *dpy, void *key)
     for (prev = &info->close_display_hooks; (hook = *prev); prev = &hook->next)
     {
 	if (hook->key == key) {
+	    _cairo_xlib_screen_info_destroy_unlocked (info);
 	    *prev = hook->next;
 	    free (hook);
 	    break;
 	}
     }
+    _cairo_xlib_screen_info_destroy_unlocked (info);
 
 unlock:
     CAIRO_MUTEX_UNLOCK (_cairo_xlib_screen_mutex);
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 7f1392d..a85e700 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -341,6 +341,9 @@ _cairo_xlib_surface_finish (void *abstract_surface)
     if (surface->clip_rects != NULL)
 	free (surface->clip_rects);
 
+    if (surface->screen_info != NULL)
+	_cairo_xlib_screen_info_destroy (surface->screen_info);
+
     surface->dpy = NULL;
 
     return CAIRO_STATUS_SUCCESS;
@@ -1825,6 +1828,7 @@ _cairo_xlib_surface_create_internal (Display		       *dpy,
     surface = malloc (sizeof (cairo_xlib_surface_t));
     if (surface == NULL) {
 	_cairo_error (CAIRO_STATUS_NO_MEMORY);
+	_cairo_xlib_screen_info_destroy (screen_info);
 	return (cairo_surface_t*) &_cairo_surface_nil;
     }
 


More information about the cairo mailing list