[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cairo
Subject:    Re: [cairo] 2 Coverity defects for mutex lock and resource leak
From:       Uli Schlachter <psychon () znc ! in>
Date:       2018-01-14 9:32:08
Message-ID: 55e054e1-d4fa-4d35-7cdf-569f52218b70 () znc ! in
[Download RAW message or body]

Hi,

On 14.01.2018 05:24, Bryce Harrington wrote:
[...]
> ** CID 1160664:    (ORDER_REVERSAL)
> /src/cairo-scaled-font.c: 408 in _cairo_scaled_font_map_destroy()
> /src/cairo-scaled-font.c: 419 in _cairo_scaled_font_map_destroy()

Could you explain what this actually means?

"ORDER_REVERSAL" sounds like one piece of code does lock(a) lock(b)
while another does lock(b) lock(a) so you get a chance for a deadlock.
However, it points at two places locking the same mutex in the same
function?!?

> ________________________________________________________________________________________________________
>                 
> *** CID 1160664:    (ORDER_REVERSAL)
> /src/cairo-scaled-font.c: 408 in _cairo_scaled_font_map_destroy()
> 402     void
> 403     _cairo_scaled_font_map_destroy (void)
> 404     {
> 405         cairo_scaled_font_map_t *font_map;
> 406         cairo_scaled_font_t *scaled_font;
> 407     
> > > > CID 1160664:    (ORDER_REVERSAL)
> > > > Calling "pthread_mutex_lock" acquires lock "_cairo_scaled_font_map_mutex".
> 408         CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
> 409     
> 410         font_map = cairo_scaled_font_map;
> 411         if (unlikely (font_map == NULL)) {
> 412             goto CLEANUP_MUTEX_LOCK;
> 413         }
> /src/cairo-scaled-font.c: 419 in _cairo_scaled_font_map_destroy()
> 413         }
> 414     
> 415         scaled_font = font_map->mru_scaled_font;
> 416         if (scaled_font != NULL) {
> 417     	CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
> 418     	cairo_scaled_font_destroy (scaled_font);
> > > > CID 1160664:    (ORDER_REVERSAL)
> > > > Calling "pthread_mutex_lock" acquires lock "_cairo_scaled_font_map_mutex".
> 419     	CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
> 420         }
> 421     
> 422         /* remove scaled_fonts starting from the end so that \
> font_map->holdovers 423          * is always in a consistent state when we release \
> the mutex. */ 424         while (font_map->num_holdovers) {
> 
> ** CID 1384409:    (RESOURCE_LEAK)
> /test/font-variations.c: 79 in test_variation()
> /test/font-variations.c: 90 in test_variation()

This one seems trivial and not all that important.

Cheers,
Uli
-- 
Happiness can't be found -- it finds you.
 - Majic
-- 
cairo mailing list
cairo@cairographics.org
https://lists.cairographics.org/mailman/listinfo/cairo


[prev in list] [next in list] [prev in thread] [next in thread]