[cairo] Clear the glyph caches on XCloseDisplay

Chris Wilson chris at chris-wilson.co.uk
Tue Mar 13 13:57:33 PDT 2007


After introducing the use of ->cleanup() for cairo-perf, failures were
noticed in the xlib backend. These were due to not releasing the XRender
glyph cache after closing the display, and reusing the old data with a
new display.

This series of patches introduces a hook into the
_cairo_xlib_close_display() routine and uses it to reset the cached
data.
-------------- next part --------------
>From b83a8768857e13a380b5b13367246d88e2930806 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue, 13 Mar 2007 20:11:49 +0000
Subject: [PATCH] Introduce hooks for _cairo_xlib_close_display()

This patch adds a simple hook data type for a notifier style callback
and introduces two functions to manipulate a list of callbacks for
cleaning up on display closure.
---
 src/cairo-xlib-private.h |   15 +++++
 src/cairo-xlib-screen.c  |  153 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 136 insertions(+), 32 deletions(-)

diff --git a/src/cairo-xlib-private.h b/src/cairo-xlib-private.h
index ddac05a..8ba46a2 100644
--- a/src/cairo-xlib-private.h
+++ b/src/cairo-xlib-private.h
@@ -37,6 +37,14 @@
 #include "cairo-xlib.h"
 
 typedef struct _cairo_xlib_screen_info cairo_xlib_screen_info_t;
+typedef struct _cairo_xlib_hook cairo_xlib_hook_t;
+
+struct _cairo_xlib_hook {
+    cairo_xlib_hook_t *next;
+    void (*func) (Display *display, void *data);
+    void *data;
+    void *key;
+};
 
 struct _cairo_xlib_screen_info {
     cairo_xlib_screen_info_t *next;
@@ -46,11 +54,18 @@ struct _cairo_xlib_screen_info {
     cairo_bool_t has_render;
 
     cairo_font_options_t font_options;
+
+    cairo_xlib_hook_t *close_display_hooks;
 };
 
 cairo_private cairo_xlib_screen_info_t *
 _cairo_xlib_screen_info_get (Display *display, Screen *screen);
 
+cairo_private cairo_bool_t
+_cairo_xlib_add_close_display_hook (Display *display, void (*func) (Display *, void *), void *data, void *key);
+cairo_private void
+_cairo_xlib_remove_close_display_hook (Display *display, void *key);
+
 #if CAIRO_HAS_XLIB_XRENDER_SURFACE
 
 #include "cairo-xlib-xrender.h"
diff --git a/src/cairo-xlib-screen.c b/src/cairo-xlib-screen.c
index 2316fe1..4c34608 100644
--- a/src/cairo-xlib-screen.c
+++ b/src/cairo-xlib-screen.c
@@ -247,31 +247,36 @@ CAIRO_MUTEX_DECLARE(_xlib_screen_mutex);
 
 static cairo_xlib_screen_info_t *_cairo_xlib_screen_list = NULL;
 
-/* XXX: From this function we should also run through and cleanup
- * anything else that still has a pointer to this Display*. For
- * example, we should clean up any Xlib-specific glyph caches. */
 static int
 _cairo_xlib_close_display (Display *dpy, XExtCodes *codes)
 {
-    cairo_xlib_screen_info_t *info, *prev;
+    cairo_xlib_screen_info_t *info, **prev, *next;
 
     /*
      * Unhook from the global list
      */
     CAIRO_MUTEX_LOCK (_xlib_screen_mutex);
 
-    prev = NULL;
-    for (info = _cairo_xlib_screen_list; info; info = info->next) {
+    prev = &_cairo_xlib_screen_list;
+    for (info = _cairo_xlib_screen_list; info; info = next) {
+	next = info->next;
 	if (info->display == dpy) {
-	    if (prev)
-		prev->next = info->next;
-	    else
-		_cairo_xlib_screen_list = info->next;
+	    *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);
-	    break;
+	} else {
+	    prev = &info->next;
 	}
-	prev = info;
     }
+    *prev = NULL;
     CAIRO_MUTEX_UNLOCK (_xlib_screen_mutex);
 
     /* Return value in accordance with requirements of
@@ -291,6 +296,11 @@ _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);
     }
 
@@ -300,8 +310,8 @@ _cairo_xlib_screen_info_reset (void)
 
 }
 
-cairo_xlib_screen_info_t *
-_cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
+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;
@@ -309,26 +319,15 @@ _cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
     XExtCodes *codes;
     cairo_bool_t seen_display = FALSE;
 
-    /* There is an apparent deadlock between this mutex and the
-     * mutex for the display, but it's actually safe. For the
-     * app to call XCloseDisplay() while any other thread is
-     * inside this function would be an error in the logic
-     * app, and the CloseDisplay hook is the only other place we
-     * acquire this mutex.
-     */
-    CAIRO_MUTEX_LOCK (_xlib_screen_mutex);
-
     for (prev = &_cairo_xlib_screen_list; (info = *prev); prev = &(*prev)->next)
     {
 	if (info->display == dpy) {
 	    seen_display = TRUE;
-	    if (info->screen == screen)
-	    {
+	    if (info->screen == screen || screen == NULL) {
 		/*
 		 * MRU the list
 		 */
-		if (prev != &_cairo_xlib_screen_list)
-		{
+		if (prev != &_cairo_xlib_screen_list) {
 		    *prev = info->next;
 		    info->next = _cairo_xlib_screen_list;
 		    _cairo_xlib_screen_list = info;
@@ -339,18 +338,17 @@ _cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
     }
 
     if (info)
-	goto out;
+	return info;
 
     info = malloc (sizeof (cairo_xlib_screen_info_t));
     if (!info)
-	goto out;
+	return NULL;
 
     if (!seen_display) {
 	codes = XAddExtension (dpy);
 	if (!codes) {
 	    free (info);
-	    info = NULL;
-	    goto out;
+	    return NULL;
 	}
 
 	XESetCloseDisplay (dpy, codes->extension, _cairo_xlib_close_display);
@@ -361,17 +359,108 @@ _cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
     info->has_render = (XRenderQueryExtension (dpy, &event_base, &error_base) &&
 			(XRenderFindVisualFormat (dpy, DefaultVisual (dpy, DefaultScreen (dpy))) != 0));
 
+    info->close_display_hooks = NULL;
+
     _cairo_xlib_init_screen_font_options (info);
 
     info->next = _cairo_xlib_screen_list;
     _cairo_xlib_screen_list = info;
 
- out:
+    return info;
+}
+cairo_xlib_screen_info_t *
+_cairo_xlib_screen_info_get (Display *dpy, Screen *screen)
+{
+    cairo_xlib_screen_info_t *info;
+
+    /* There is an apparent deadlock between this mutex and the
+     * mutex for the display, but it's actually safe. For the
+     * app to call XCloseDisplay() while any other thread is
+     * inside this function would be an error in the logic
+     * app, and the CloseDisplay hook is the only other place we
+     * acquire this mutex.
+     */
+    CAIRO_MUTEX_LOCK (_xlib_screen_mutex);
+
+    info = _cairo_xlib_screen_info_get_unlocked (dpy, screen);
+
     CAIRO_MUTEX_UNLOCK (_xlib_screen_mutex);
 
     return info;
 }
 
+cairo_bool_t
+_cairo_xlib_add_close_display_hook (Display *dpy, void (*func) (Display *, void *), void *data, void *key)
+{
+    cairo_xlib_screen_info_t *info;
+    cairo_xlib_hook_t *hook;
+    cairo_xlib_hook_t **prev;
+    cairo_bool_t success = FALSE;
+
+    CAIRO_MUTEX_LOCK (_xlib_screen_mutex);
+
+    info = _cairo_xlib_screen_info_get_unlocked (dpy,  NULL);
+    if (!info)
+	goto unlock;
+
+    for (prev = &info->close_display_hooks; (hook = *prev); prev = &hook->next)
+    {
+	if (hook->key == key) {
+	    /*
+	     * MRU the list
+	     */
+	    if (prev != &info->close_display_hooks) {
+		*prev = hook->next;
+		hook->next = info->close_display_hooks;
+		info->close_display_hooks = hook;
+	    }
+	    break;
+	}
+    }
+
+    if (!hook) {
+	hook = malloc (sizeof (cairo_xlib_hook_t));
+	if (!hook)
+	    goto unlock;
+	hook->func = func;
+	hook->data = data;
+	hook->key = key;
+	hook->next = info->close_display_hooks;
+	info->close_display_hooks = hook;
+    }
+
+    success = TRUE;
+ unlock:
+    CAIRO_MUTEX_UNLOCK (_xlib_screen_mutex);
+    return success;
+}
+
+void
+_cairo_xlib_remove_close_display_hook (Display *dpy, void *key)
+{
+    cairo_xlib_screen_info_t *info;
+    cairo_xlib_hook_t *hook;
+    cairo_xlib_hook_t **prev;
+
+    CAIRO_MUTEX_LOCK (_xlib_screen_mutex);
+
+    info = _cairo_xlib_screen_info_get_unlocked (dpy, NULL);
+    if (!info)
+	goto unlock;
+
+    for (prev = &info->close_display_hooks; (hook = *prev); prev = &hook->next)
+    {
+	if (hook->key == key) {
+	    *prev = hook->next;
+	    free (hook);
+	    break;
+	}
+    }
+
+unlock:
+    CAIRO_MUTEX_UNLOCK (_xlib_screen_mutex);
+}
+
 void
 _cairo_xlib_screen_reset_static_data (void)
 {
-- 
1.4.4.2

-------------- next part --------------
>From 9da9ef046952f3ba4b3c76c2106e2ec4d357bbce Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue, 13 Mar 2007 20:42:09 +0000
Subject: [PATCH] Privately export a function to reset the scaled font's glyph caches.

---
 src/cairo-scaled-font.c |    9 +++++++++
 src/cairoint.h          |    3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index 0162a0d..6efde80 100755
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -391,6 +391,15 @@ _cairo_scaled_font_thaw_cache (cairo_scaled_font_t *scaled_font)
 }
 
 void
+_cairo_scaled_font_reset_cache (cairo_scaled_font_t *scaled_font)
+{
+    _cairo_cache_destroy (scaled_font->glyphs);
+    scaled_font->glyphs = _cairo_cache_create (_cairo_scaled_glyph_keys_equal,
+					       _cairo_scaled_glyph_destroy,
+					       max_glyphs_cached_per_font);
+}
+
+void
 _cairo_scaled_font_set_metrics (cairo_scaled_font_t	    *scaled_font,
 				cairo_font_extents_t	    *fs_metrics)
 {
diff --git a/src/cairoint.h b/src/cairoint.h
index 3251950..10c546d 100755
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -1628,6 +1628,9 @@ cairo_private void
 _cairo_scaled_font_thaw_cache (cairo_scaled_font_t *scaled_font);
 
 cairo_private void
+_cairo_scaled_font_reset_cache (cairo_scaled_font_t *scaled_font);
+
+cairo_private void
 _cairo_scaled_font_set_error (cairo_scaled_font_t *scaled_font,
 			      cairo_status_t status);
 
-- 
1.4.4.2

-------------- next part --------------
>From 7980bd3c65ad7540706eacb3f4d40d9c66a2b776 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue, 13 Mar 2007 20:17:22 +0000
Subject: [PATCH] Clear the XRender data on display closure.

Use the new hook functions to register a callback for xlib to clear
the private glyph data when the display is closed. In order to do this
we need to reset the glyph cache inside the generic scaled font as well.
---
 src/cairo-xlib-surface.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 6a0d3e4..49cb00f 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -2268,6 +2268,24 @@ typedef struct _cairo_xlib_surface_font_private {
     XRenderPictFormat	*xrender_format;
 } cairo_xlib_surface_font_private_t;
 
+static void
+_cairo_xlib_surface_remove_scaled_font (Display *dpy,
+	                               void    *data)
+{
+    cairo_scaled_font_t *scaled_font = data;
+    cairo_xlib_surface_font_private_t	*font_private = scaled_font->surface_private;
+
+    _cairo_scaled_font_reset_cache (scaled_font);
+
+    /* separate function to avoid deadlock if we tried to remove the
+     * close display hook ala _cairo_xlib_surface_scaled_font_fini() */
+    if (font_private) {
+	XRenderFreeGlyphSet (font_private->dpy, font_private->glyphset);
+	free (font_private);
+	scaled_font->surface_private = NULL;
+    }
+}
+
 static cairo_status_t
 _cairo_xlib_surface_font_init (Display		    *dpy,
 			       cairo_scaled_font_t  *scaled_font,
@@ -2275,6 +2293,11 @@ _cairo_xlib_surface_font_init (Display		    *dpy,
 {
     cairo_xlib_surface_font_private_t	*font_private;
 
+    if (!_cairo_xlib_add_close_display_hook (dpy,
+	       	_cairo_xlib_surface_remove_scaled_font,
+	       	scaled_font, scaled_font))
+	return CAIRO_STATUS_NO_MEMORY;
+
     font_private = malloc (sizeof (cairo_xlib_surface_font_private_t));
     if (!font_private)
 	return CAIRO_STATUS_NO_MEMORY;
@@ -2285,6 +2308,7 @@ _cairo_xlib_surface_font_init (Display		    *dpy,
     font_private->glyphset = XRenderCreateGlyphSet (dpy, font_private->xrender_format);
     scaled_font->surface_private = font_private;
     scaled_font->surface_backend = &cairo_xlib_surface_backend;
+
     return CAIRO_STATUS_SUCCESS;
 }
 
@@ -2294,6 +2318,7 @@ _cairo_xlib_surface_scaled_font_fini (cairo_scaled_font_t *scaled_font)
     cairo_xlib_surface_font_private_t	*font_private = scaled_font->surface_private;
 
     if (font_private) {
+	_cairo_xlib_remove_close_display_hook (font_private->dpy, scaled_font);
 	XRenderFreeGlyphSet (font_private->dpy, font_private->glyphset);
 	free (font_private);
     }
-- 
1.4.4.2

-------------- next part --------------
>From b8d739bf3c29dfe9c7479bd3dcadef52fcc63a87 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue, 13 Mar 2007 20:45:39 +0000
Subject: [PATCH] Whitespace.

---
 src/cairo-xlib-surface.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 49cb00f..49bdaae 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -2294,8 +2294,8 @@ _cairo_xlib_surface_font_init (Display		    *dpy,
     cairo_xlib_surface_font_private_t	*font_private;
 
     if (!_cairo_xlib_add_close_display_hook (dpy,
-	       	_cairo_xlib_surface_remove_scaled_font,
-	       	scaled_font, scaled_font))
+		_cairo_xlib_surface_remove_scaled_font,
+		scaled_font, scaled_font))
 	return CAIRO_STATUS_NO_MEMORY;
 
     font_private = malloc (sizeof (cairo_xlib_surface_font_private_t));
-- 
1.4.4.2



More information about the cairo mailing list