Bug 1471261 - Fix race condition in cairo-ft when checking whether face is being used. r=lsalzman draft
authorRyan Hunt <rhunt@eqrion.net>
Tue, 26 Jun 2018 11:37:09 -0500
changeset 810901 26b0dde99e35fa216c8ad72c8d1d570f729d1af3
parent 810823 348090c6b5c421c67b9dccc48742b54a854d6d0e
child 810902 bec9491d4191f153d1ebd8a72428b5aac8ddc726
push id114157
push userbmo:rhunt@eqrion.net
push dateTue, 26 Jun 2018 18:54:49 +0000
reviewerslsalzman
bugs1471261
milestone63.0a1
Bug 1471261 - Fix race condition in cairo-ft when checking whether face is being used. r=lsalzman MozReview-Commit-ID: JizcCY7evq2
gfx/cairo/cairo/src/cairo-ft-font.c
gfx/cairo/cairo/src/cairo-mutex-impl-private.h
gfx/cairo/cairo/src/cairo-mutex-type-private.h
--- a/gfx/cairo/cairo/src/cairo-ft-font.c
+++ b/gfx/cairo/cairo/src/cairo-ft-font.c
@@ -706,17 +706,30 @@ cairo_warn FT_Face
 	{
 	    cairo_ft_unscaled_font_t *entry;
 
 	    entry = _cairo_hash_table_random_entry (font_map->hash_table,
 						    _has_unlocked_face);
 	    if (entry == NULL)
 		break;
 
-	    _font_map_release_face_lock_held (font_map, entry);
+	    /* Must use try-lock here to avoid deadlock on multiple threads trying to
+	     * acquire the font map lock inside the unscaled font mutexes. In the worst
+	     * case, this may just cause a spin in the extremely unlikely circumstance
+	     * that all open faces are currently locked.
+	     */
+	    if (CAIRO_MUTEX_TRY_LOCK (entry->mutex))
+	    {
+		/* Verify the lock count is still actually 0 inside the mutex before
+		 * trying to free the entry.
+		 */
+		if (_has_unlocked_face (entry))
+		    _font_map_release_face_lock_held (font_map, entry);
+		CAIRO_MUTEX_UNLOCK (entry->mutex);
+	    }
 	}
     }
     _cairo_ft_unscaled_font_map_unlock ();
 
     face = mozilla_NewFTFace (font_map->ft_library, unscaled->filename, unscaled->id);
     if (!face)
     {
 	unscaled->lock_count--;
--- a/gfx/cairo/cairo/src/cairo-mutex-impl-private.h
+++ b/gfx/cairo/cairo/src/cairo-mutex-impl-private.h
@@ -231,16 +231,17 @@
   typedef pthread_mutex_t cairo_recursive_mutex_impl_t;
 
 # define CAIRO_MUTEX_IMPL_PTHREAD 1
 #if HAVE_LOCKDEP
 /* expose all mutexes to the validator */
 # 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)))
 # define CAIRO_MUTEX_IMPL_UNLOCK(mutex) pthread_mutex_unlock (&(mutex))
 #if HAVE_LOCKDEP
 # define CAIRO_MUTEX_IS_LOCKED(mutex) LOCKDEP_IS_LOCKED (&(mutex))
 # define CAIRO_MUTEX_IS_UNLOCKED(mutex) LOCKDEP_IS_UNLOCKED (&(mutex))
 #endif
 # define CAIRO_MUTEX_IMPL_FINI(mutex) pthread_mutex_destroy (&(mutex))
 #if ! HAVE_LOCKDEP
 # define CAIRO_MUTEX_IMPL_FINALIZE() CAIRO_MUTEX_IMPL_NOOP
--- a/gfx/cairo/cairo/src/cairo-mutex-type-private.h
+++ b/gfx/cairo/cairo/src/cairo-mutex-type-private.h
@@ -162,16 +162,17 @@ typedef cairo_mutex_impl_t cairo_mutex_t
 typedef cairo_recursive_mutex_impl_t cairo_recursive_mutex_t;
 #else
 # define cairo_mutex_t			cairo_mutex_impl_t
 #endif
 
 #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
 #define CAIRO_MUTEX_NIL_INITIALIZER	CAIRO_MUTEX_IMPL_NIL_INITIALIZER
 
 #define CAIRO_RECURSIVE_MUTEX_INIT		CAIRO_RECURSIVE_MUTEX_IMPL_INIT
 #define CAIRO_RECURSIVE_MUTEX_NIL_INITIALIZER	CAIRO_RECURSIVE_MUTEX_IMPL_NIL_INITIALIZER