[cairo] [PATCH] Do not hold mutexes when calling destructors.

[Kristian Høgsberg]

On Fri, May 9, 2008 at 6:15 PM, Jeff Muizelaar <jeff at infidigm.net> wrote:
> cairo_surface_destroy and _cairo_scaled_font_fini will call destroy closures
>  which may call functions that attempt to acquire the mutex resulting in a
>  deadlock. We fix this by releasing the lock for the call to
>  cairo_surface_destroy or _cairo_scaled_font_fini.
>  ---
>
>   src/cairo-pattern.c     |   17 ++++++++++++-----
>   src/cairo-scaled-font.c |   14 +++++++++++---
>   2 files changed, 23 insertions(+), 8 deletions(-)
>
>
>  diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
>  index 0f2eb3e..903d81f 100644
>  --- a/src/cairo-pattern.c
>  +++ b/src/cairo-pattern.c
>  @@ -1515,13 +1515,20 @@ UNLOCK:
>   static void
>   _cairo_pattern_reset_solid_surface_cache (void)
>   {
>  -    int i;
>  -
>      CAIRO_MUTEX_LOCK (_cairo_pattern_solid_surface_cache_lock);
>
>  -    for (i = 0; i < solid_surface_cache.size; i++)
>  -       cairo_surface_destroy (solid_surface_cache.cache[i].surface);
>  -    solid_surface_cache.size = 0;
>  +    /* remove surfaces starting from the end so that solid_surface_cache.cache
>  +     * is always in a consistent state when we release the mutex. */
>  +    while (solid_surface_cache.size) {
>  +       cairo_surface_t *surface = solid_surface_cache.cache[solid_surface_cache.size-1].surface;
>  +       solid_surface_cache.size--;
>  +
>  +       /* release the lock to avoid the possibility of a recursive
>  +        * deadlock when the scaled font destroy closure gets called */
>  +       CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_surface_cache_lock);
>  +       cairo_surface_destroy (surface);
>  +       CAIRO_MUTEX_LOCK (_cairo_pattern_solid_surface_cache_lock);
>  +    }

Would it be better to say:

    struct solid_surface_cache copy;

    CAIRO_MUTEX_LOCK (_cairo_pattern_solid_surface_cache_lock);
    copy = solid_surface_cache;
    solid_surface_cache.size = 0;
    CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_surface_cache_lock);

    for (i = 0; i < copy.size; i++)
        cairo_surface_destroy (copy.cache[i].surface);

Kristian