From 22723ffabaf37a23c39870f4ffc4794c76b3a755 Mon Sep 17 00:00:00 2001
From: Johan Redestig <johan.redestig@sonymobile.com>
Date: Wed, 10 Aug 2016 14:57:27 +0200
Subject: [PATCH] Fix mStringBlocks race in the AssetManager

There were a few places where access to the mStringBlocks were
not protected.

The crashes seen where similar to:

  java.lang.NullPointerException: Attempt to invoke virtual method \
    'java.lang.CharSequence android.content.res.StringBlock.get(int)' on a null object reference
  at android.content.res.AssetManager.getResourceValue(AssetManager.java:222)
  at android.content.res.ResourcesImpl.getValue(ResourcesImpl.java:188)
  at android.content.res.Resources.loadXmlResourceParser(Resources.java:2110)
  at android.content.res.Resources.getLayout(Resources.java:1111)

  java.lang.NullPointerException: Attempt to invoke virtual method \
    'java.lang.CharSequence android.content.res.StringBlock.get(int)' on a null object reference
  at android.content.res.AssetManager.getPooledStringForCookie(AssetManager.java:312)
  at android.content.res.TypedArray.loadStringValueAt(TypedArray.java:1212)
  at android.content.res.TypedArray.getValueAt(TypedArray.java:1198)
  at android.content.res.TypedArray.getColor(TypedArray.java:446)

What happened was that thread 1 was creating a new mStringBlocks in
makeStringBlocks while thread 2 was accessing mStringBlocks. The
makeStringBlocks starts off by overwriting mStringBlocks with a new
empty array and when thread 2 accessed its content NPE happened.

Bug: 30802713
Test: None (just added synchronization to help prevent races)
Change-Id: I810da26b161a6528b0dd241048dde5b239089244
---
 core/java/android/content/res/AssetManager.java | 52 ++++++++++++++-----------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java
index 37e32ff2130a..4cf55ba27eec 100644
--- a/core/java/android/content/res/AssetManager.java
+++ b/core/java/android/content/res/AssetManager.java
@@ -222,19 +222,21 @@ public final class AssetManager implements AutoCloseable {
      */
     final boolean getResourceValue(@AnyRes int resId, int densityDpi, @NonNull TypedValue outValue,
             boolean resolveRefs) {
-        final int block = loadResourceValue(resId, (short) densityDpi, outValue, resolveRefs);
-        if (block < 0) {
-            return false;
-        }
+        synchronized (this) {
+            final int block = loadResourceValue(resId, (short) densityDpi, outValue, resolveRefs);
+            if (block < 0) {
+                return false;
+            }
 
-        // Convert the changing configurations flags populated by native code.
-        outValue.changingConfigurations = ActivityInfo.activityInfoConfigNativeToJava(
-                outValue.changingConfigurations);
+            // Convert the changing configurations flags populated by native code.
+            outValue.changingConfigurations = ActivityInfo.activityInfoConfigNativeToJava(
+                    outValue.changingConfigurations);
 
-        if (outValue.type == TypedValue.TYPE_STRING) {
-            outValue.string = mStringBlocks[block].get(outValue.data);
+            if (outValue.type == TypedValue.TYPE_STRING) {
+                outValue.string = mStringBlocks[block].get(outValue.data);
+            }
+            return true;
         }
-        return true;
     }
 
     /**
@@ -244,18 +246,20 @@ public final class AssetManager implements AutoCloseable {
      * @param resId the resource id of the string array
      */
     final CharSequence[] getResourceTextArray(@ArrayRes int resId) {
-        final int[] rawInfoArray = getArrayStringInfo(resId);
-        final int rawInfoArrayLen = rawInfoArray.length;
-        final int infoArrayLen = rawInfoArrayLen / 2;
-        int block;
-        int index;
-        final CharSequence[] retArray = new CharSequence[infoArrayLen];
-        for (int i = 0, j = 0; i < rawInfoArrayLen; i = i + 2, j++) {
-            block = rawInfoArray[i];
-            index = rawInfoArray[i + 1];
-            retArray[j] = index >= 0 ? mStringBlocks[block].get(index) : null;
+        synchronized (this) {
+            final int[] rawInfoArray = getArrayStringInfo(resId);
+            final int rawInfoArrayLen = rawInfoArray.length;
+            final int infoArrayLen = rawInfoArrayLen / 2;
+            int block;
+            int index;
+            final CharSequence[] retArray = new CharSequence[infoArrayLen];
+            for (int i = 0, j = 0; i < rawInfoArrayLen; i = i + 2, j++) {
+                block = rawInfoArray[i];
+                index = rawInfoArray[i + 1];
+                retArray[j] = index >= 0 ? mStringBlocks[block].get(index) : null;
+            }
+            return retArray;
         }
-        return retArray;
     }
 
     /**
@@ -320,8 +324,10 @@ public final class AssetManager implements AutoCloseable {
     }
 
     /*package*/ final CharSequence getPooledStringForCookie(int cookie, int id) {
-        // Cookies map to string blocks starting at 1.
-        return mStringBlocks[cookie - 1].get(id);
+        synchronized (this) {
+            // Cookies map to string blocks starting at 1.
+            return mStringBlocks[cookie - 1].get(id);
+        }
     }
 
     /**
-- 
2.11.0