Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 22723ffa authored by Johan Redestig's avatar Johan Redestig Committed by Adam Lesinski
Browse files

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
parent 8716ef94

core/java/android/content/res/AssetManager.java

+29 −23
Original line number Diff line number Diff line
@@ -222,6 +222,7 @@ public final class AssetManager implements AutoCloseable { 
     */

    final boolean getResourceValue(@AnyRes int resId, int densityDpi, @NonNull TypedValue outValue,

            boolean resolveRefs) {

        synchronized (this) {

            final int block = loadResourceValue(resId, (short) densityDpi, outValue, resolveRefs);

            if (block < 0) {

                return false;
@@ -236,6 +237,7 @@ public final class AssetManager implements AutoCloseable { 
            }

            return true;

        }

    }



    /**

     * Retrieve the text array associated with a particular resource
@@ -244,6 +246,7 @@ public final class AssetManager implements AutoCloseable { 
     * @param resId the resource id of the string array

     */

    final CharSequence[] getResourceTextArray(@ArrayRes int resId) {

        synchronized (this) {

            final int[] rawInfoArray = getArrayStringInfo(resId);

            final int rawInfoArrayLen = rawInfoArray.length;

            final int infoArrayLen = rawInfoArrayLen / 2;
@@ -257,6 +260,7 @@ public final class AssetManager implements AutoCloseable { 
            }

            return retArray;

        }

    }



    /**

     * Populates {@code outValue} with the data associated with a particular
@@ -320,9 +324,11 @@ public final class AssetManager implements AutoCloseable { 
    }



    /*package*/ final CharSequence getPooledStringForCookie(int cookie, int id) {

        synchronized (this) {

            // Cookies map to string blocks starting at 1.

            return mStringBlocks[cookie - 1].get(id);

        }

    }



    /**

     * Open an asset using ACCESS_STREAMING mode.  This provides access to