Fix deadlock in cairo-scaled-font.c

When cairo_scaled_glyph_page_cache needs to remove entries,
cairo-cache calls _cairo_hash_table_random_entry() with the predicate
_cairo_scaled_glyph_page_can_remove(). This function checks that the
glyph_page scaled_font is not locked by testing
scaled_font->cache_frozen. The scaled font is locked in the
cache-cache destroy entry callback: _cairo_scaled_glyph_page_pluck().

There is a race condition here between testing
scaled_font->cache_frozen and locking the font. Fix this by adding a
new CAIRO_MUTEX_TRY_LOCK mutex operation, and using it to test and
lock the scaled font in _cairo_scaled_glyph_page_can_remove().

Fixes the multithreaded case in #440
This commit is contained in:
Adrian Johnson 2022-05-27 21:36:31 +09:30
parent 5dafd74116
commit a8c1858cf2
3 changed files with 19 additions and 3 deletions

View file

@ -87,6 +87,9 @@
* No trailing semicolons are needed (in any macro you define here).
* You should be able to compile the following snippet:
*
* - #define CAIRO_MUTEX_IMPL_TRY_LOCK(mutex) to try locking the mutex object,
* returning TRUE if the lock is acquired, FALSE if the mutex could not be locked.
*
* <programlisting>
* cairo_mutex_impl_t _cairo_some_mutex;
*
@ -163,6 +166,7 @@
# define CAIRO_MUTEX_IMPL_NO 1
# define CAIRO_MUTEX_IMPL_INITIALIZE() CAIRO_MUTEX_IMPL_NOOP
# define CAIRO_MUTEX_IMPL_LOCK(mutex) CAIRO_MUTEX_IMPL_NOOP1(mutex)
# define CAIRO_MUTEX_IMPL_TRY_LOCK(mutex) (CAIRO_MUTEX_IMPL_NOOP1(mutex), TRUE)
# define CAIRO_MUTEX_IMPL_UNLOCK(mutex) CAIRO_MUTEX_IMPL_NOOP1(mutex)
# define CAIRO_MUTEX_IMPL_NIL_INITIALIZER 0
@ -190,6 +194,7 @@
# define CAIRO_MUTEX_IMPL_WIN32 1
# define CAIRO_MUTEX_IMPL_LOCK(mutex) EnterCriticalSection (&(mutex))
# define CAIRO_MUTEX_IMPL_TRY_LOCK(mutex) TryEnterCriticalSection (&(mutex))
# define CAIRO_MUTEX_IMPL_UNLOCK(mutex) LeaveCriticalSection (&(mutex))
# define CAIRO_MUTEX_IMPL_INIT(mutex) InitializeCriticalSection (&(mutex))
# define CAIRO_MUTEX_IMPL_FINI(mutex) DeleteCriticalSection (&(mutex))
@ -208,6 +213,7 @@
# define CAIRO_MUTEX_IMPL_INIT(mutex) pthread_mutex_init (&(mutex), NULL)
#endif
# define CAIRO_MUTEX_IMPL_LOCK(mutex) pthread_mutex_lock (&(mutex))
# define CAIRO_MUTEX_IMPL_TRY_LOCK(mutex) (pthread_mutex_trylock (&(mutex)) == 0)
# define CAIRO_MUTEX_IMPL_UNLOCK(mutex) pthread_mutex_unlock (&(mutex))
#if HAVE_LOCKDEP
# define CAIRO_MUTEX_IS_LOCKED(mutex) LOCKDEP_IS_LOCKED (&(mutex))

View file

@ -48,6 +48,9 @@
#ifndef CAIRO_MUTEX_IMPL_LOCK
# error "CAIRO_MUTEX_IMPL_LOCK not defined. Check cairo-mutex-impl-private.h."
#endif
#ifndef CAIRO_MUTEX_IMPL_TRY_LOCK
# error "CAIRO_MUTEX_IMPL_TRY_LOCK not defined. Check cairo-mutex-impl-private.h."
#endif
#ifndef CAIRO_MUTEX_IMPL_UNLOCK
# error "CAIRO_MUTEX_IMPL_UNLOCK not defined. Check cairo-mutex-impl-private.h."
#endif
@ -138,6 +141,9 @@
#ifndef CAIRO_MUTEX_IMPL_LOCK
# error "CAIRO_MUTEX_IMPL_LOCK not defined"
#endif
#ifndef CAIRO_MUTEX_IMPL_TRY_LOCK
# error "CAIRO_MUTEX_IMPL_TRY_LOCK not defined"
#endif
#ifndef CAIRO_MUTEX_IMPL_UNLOCK
# error "CAIRO_MUTEX_IMPL_UNLOCK not defined"
#endif
@ -167,6 +173,7 @@ typedef cairo_recursive_mutex_impl_t cairo_recursive_mutex_t;
#define CAIRO_MUTEX_INITIALIZE CAIRO_MUTEX_IMPL_INITIALIZE
#define CAIRO_MUTEX_FINALIZE CAIRO_MUTEX_IMPL_FINALIZE
#define CAIRO_MUTEX_LOCK CAIRO_MUTEX_IMPL_LOCK
#define CAIRO_MUTEX_TRY_LOCK CAIRO_MUTEX_IMPL_TRY_LOCK
#define CAIRO_MUTEX_UNLOCK CAIRO_MUTEX_IMPL_UNLOCK
#define CAIRO_MUTEX_INIT CAIRO_MUTEX_IMPL_INIT
#define CAIRO_MUTEX_FINI CAIRO_MUTEX_IMPL_FINI

View file

@ -476,7 +476,7 @@ _cairo_scaled_glyph_page_pluck (void *closure)
scaled_font = page->scaled_font;
CAIRO_MUTEX_LOCK (scaled_font->mutex);
/* The font is locked in _cairo_scaled_glyph_page_can_remove () */
_cairo_scaled_glyph_page_destroy (scaled_font, page);
CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
}
@ -2855,14 +2855,17 @@ _cairo_scaled_glyph_set_color_surface (cairo_scaled_glyph_t *scaled_glyph,
scaled_glyph->has_info &= ~CAIRO_SCALED_GLYPH_INFO_COLOR_SURFACE;
}
/* _cairo_hash_table_random_entry () predicate. To avoid race conditions,
* the font is locked when tested. The font is unlocked in
* _cairo_scaled_glyph_page_pluck. */
static cairo_bool_t
_cairo_scaled_glyph_page_can_remove (const void *closure)
{
const cairo_scaled_glyph_page_t *page = closure;
const cairo_scaled_font_t *scaled_font;
cairo_scaled_font_t *scaled_font;
scaled_font = page->scaled_font;
return scaled_font->cache_frozen == 0;
return CAIRO_MUTEX_TRY_LOCK (scaled_font->mutex);
}
static cairo_status_t