From a6f5c46836090d1197e453c15c7f04c3c796a7ab Mon Sep 17 00:00:00 2001
From: Seigo Nonaka <nona@google.com>
Date: Tue, 5 Jan 2016 15:20:39 +0900
Subject: [PATCH] Fix race condition in Paint.hasGlyph()

The caller of FontFamily::hasVariationSelector needs to acquire the
lock before calling it, but FontCollection::hasVariationSelector
didn't acquire the lock. This caused a race condition.
This CL fixes this race condition.

Also, it turned out that assertMinikinLocked didn't assert even on
eng or userdebug device. This CL enables assertion on eng and userdebug
device since this assertion must be treated as bug.

BUG: 26323806

Change-Id: I9c4b1e1f09c6793e387fbdb8bb654cc0a13c65d5
---
 libs/minikin/Android.mk          | 13 ++++++++++---
 libs/minikin/FontCollection.cpp  |  1 +
 libs/minikin/MinikinInternal.cpp |  4 +++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/libs/minikin/Android.mk b/libs/minikin/Android.mk
index 4c945a60c9..b2e1dc8293 100644
--- a/libs/minikin/Android.mk
+++ b/libs/minikin/Android.mk
@@ -47,11 +47,18 @@ minikin_shared_libraries := \
     libicuuc \
     libutils
 
+ifneq (,$(filter userdebug eng, $(TARGET_BUILD_VARIANT)))
+# Enable race detection on eng and userdebug build.
+enable_race_detection := -DENABLE_RACE_DETECTION
+else
+enable_race_detection :=
+endif
+
 LOCAL_MODULE := libminikin
 LOCAL_EXPORT_C_INCLUDE_DIRS := frameworks/minikin/include
 LOCAL_SRC_FILES := $(minikin_src_files)
 LOCAL_C_INCLUDES := $(minikin_c_includes)
-LOCAL_CPPFLAGS += -Werror -Wall -Wextra
+LOCAL_CPPFLAGS += -Werror -Wall -Wextra $(enable_race_detection)
 LOCAL_SHARED_LIBRARIES := $(minikin_shared_libraries)
 
 include $(BUILD_SHARED_LIBRARY)
@@ -63,7 +70,7 @@ LOCAL_MODULE_TAGS := optional
 LOCAL_EXPORT_C_INCLUDE_DIRS := frameworks/minikin/include
 LOCAL_SRC_FILES := $(minikin_src_files)
 LOCAL_C_INCLUDES := $(minikin_c_includes)
-LOCAL_CPPFLAGS += -Werror -Wall -Wextra
+LOCAL_CPPFLAGS += -Werror -Wall -Wextra $(enable_race_detection)
 LOCAL_SHARED_LIBRARIES := $(minikin_shared_libraries)
 
 include $(BUILD_STATIC_LIBRARY)
@@ -76,7 +83,7 @@ LOCAL_MODULE := libminikin_host
 LOCAL_MODULE_TAGS := optional
 LOCAL_EXPORT_C_INCLUDE_DIRS := frameworks/minikin/include
 LOCAL_C_INCLUDES := $(minikin_c_includes)
-LOCAL_CPPFLAGS += -Werror -Wall -Wextra
+LOCAL_CPPFLAGS += -Werror -Wall -Wextra $(enable_race_detection)
 LOCAL_SHARED_LIBRARIES := liblog libicuuc-host
 
 LOCAL_SRC_FILES := Hyphenator.cpp
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index bba2ef9535..529b5c03a3 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -223,6 +223,7 @@ bool FontCollection::hasVariationSelector(uint32_t baseCodepoint,
     // Currently mRanges can not be used here since it isn't aware of the variation sequence.
     // TODO: Use mRanges for narrowing down the search range.
     for (size_t i = 0; i < mFamilies.size(); i++) {
+        AutoMutex _l(gMinikinLock);
         if (mFamilies[i]->hasVariationSelector(baseCodepoint, variationSelector)) {
           return true;
         }
diff --git a/libs/minikin/MinikinInternal.cpp b/libs/minikin/MinikinInternal.cpp
index 5a21ef9862..c2aa01ac19 100644
--- a/libs/minikin/MinikinInternal.cpp
+++ b/libs/minikin/MinikinInternal.cpp
@@ -25,7 +25,9 @@ namespace android {
 Mutex gMinikinLock;
 
 void assertMinikinLocked() {
-    LOG_FATAL_IF(gMinikinLock.tryLock() == 0);
+#ifdef ENABLE_RACE_DETECTION
+    LOG_ALWAYS_FATAL_IF(gMinikinLock.tryLock() == 0);
+#endif
 }
 
 }
-- 
GitLab