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

Commit f6ea6733 authored by Kweku Adams's avatar Kweku Adams
Browse files

Address static cache access issues.

ArrayMap and ArraySet are not thread safe, so callers are expected to
lock around their instances before calling any methods. However, if a
caller fails to lock, there can be races that end up corrupting the
cache pool (despite having the class level lock held). A corrupted pool
causes issues for other ArrayMap/Set instances, even if those instances
are properly used under locks. There's no way to guard against cache
pool corruption without making ArrayMap/Set fully thread-safe, changing
the pool structure, or appropriately locking around each ArrayMap/Set
instance.

For now, we make ArraySet throw ConcurrentModificationException on a best
effort basis to provide better awareness when an ArraySet is accessed
concurrently without any locks and try to detect when the cache is
corrupted.

Bug: 62282384
Bug: 73549921
Bug: 122969275
Bug: 139015193
Bug: 139401479
Test: atest CtsUtilTestCases:ArrayMapTest
Test: atest CtsUtilTestCases:ArraySetTest
Test: atest FrameworksCoreTests:ArrayMapTest
Test: atest FrameworksCoreTests:ArraySetTest
Change-Id: Icd76630b25afd9f3627239301ffa5c37da25ea18
parent 8cfff7b8
Loading
Loading
Loading
Loading
+58 −20
Original line number Original line Diff line number Diff line
@@ -48,6 +48,8 @@ import java.util.Set;
 * you have no control over this shrinking -- if you set a capacity and then remove an
 * you have no control over this shrinking -- if you set a capacity and then remove an
 * item, it may reduce the capacity to better match the current size.  In the future an
 * item, it may reduce the capacity to better match the current size.  In the future an
 * explicit call to set the capacity should turn off this aggressive shrinking behavior.</p>
 * explicit call to set the capacity should turn off this aggressive shrinking behavior.</p>
 *
 * <p>This structure is <b>NOT</b> thread-safe.</p>
 */
 */
public final class ArrayMap<K, V> implements Map<K, V> {
public final class ArrayMap<K, V> implements Map<K, V> {
    private static final boolean DEBUG = false;
    private static final boolean DEBUG = false;
@@ -103,15 +105,21 @@ public final class ArrayMap<K, V> implements Map<K, V> {
    static Object[] mTwiceBaseCache;
    static Object[] mTwiceBaseCache;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    static int mTwiceBaseCacheSize;
    static int mTwiceBaseCacheSize;
    /**
     * Separate locks for each cache since each can be accessed independently of the other without
     * risk of a deadlock.
     */
    private static final Object sBaseCacheLock = new Object();
    private static final Object sTwiceBaseCacheLock = new Object();


    final boolean mIdentityHashCode;
    private final boolean mIdentityHashCode;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use public key/value API.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use public key/value API.
    int[] mHashes;
    int[] mHashes;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Storage is an implementation detail. Use public key/value API.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Storage is an implementation detail. Use public key/value API.
    Object[] mArray;
    Object[] mArray;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Use size()
    @UnsupportedAppUsage(maxTargetSdk = 28) // Use size()
    int mSize;
    int mSize;
    MapCollections<K, V> mCollections;
    private MapCollections<K, V> mCollections;


    private static int binarySearchHashes(int[] hashes, int N, int hash) {
    private static int binarySearchHashes(int[] hashes, int N, int hash) {
        try {
        try {
@@ -209,32 +217,58 @@ public final class ArrayMap<K, V> implements Map<K, V> {
            throw new UnsupportedOperationException("ArrayMap is immutable");
            throw new UnsupportedOperationException("ArrayMap is immutable");
        }
        }
        if (size == (BASE_SIZE*2)) {
        if (size == (BASE_SIZE*2)) {
            synchronized (ArrayMap.class) {
            synchronized (sTwiceBaseCacheLock) {
                if (mTwiceBaseCache != null) {
                if (mTwiceBaseCache != null) {
                    final Object[] array = mTwiceBaseCache;
                    final Object[] array = mTwiceBaseCache;
                    mArray = array;
                    mArray = array;
                    try {
                        mTwiceBaseCache = (Object[]) array[0];
                        mTwiceBaseCache = (Object[]) array[0];
                        mHashes = (int[]) array[1];
                        mHashes = (int[]) array[1];
                        if (mHashes != null) {
                            array[0] = array[1] = null;
                            array[0] = array[1] = null;
                            mTwiceBaseCacheSize--;
                            mTwiceBaseCacheSize--;
                    if (DEBUG) Log.d(TAG, "Retrieving 2x cache " + mHashes
                            if (DEBUG) {
                                Log.d(TAG, "Retrieving 2x cache " + mHashes
                                        + " now have " + mTwiceBaseCacheSize + " entries");
                                        + " now have " + mTwiceBaseCacheSize + " entries");
                            }
                            return;
                            return;
                        }
                        }
                    } catch (ClassCastException e) {
                    }
                    // Whoops!  Someone trampled the array (probably due to not protecting
                    // their access with a lock).  Our cache is corrupt; report and give up.
                    Slog.wtf(TAG, "Found corrupt ArrayMap cache: [0]=" + array[0]
                            + " [1]=" + array[1]);
                    mTwiceBaseCache = null;
                    mTwiceBaseCacheSize = 0;
                }
            }
            }
        } else if (size == BASE_SIZE) {
        } else if (size == BASE_SIZE) {
            synchronized (ArrayMap.class) {
            synchronized (sBaseCacheLock) {
                if (mBaseCache != null) {
                if (mBaseCache != null) {
                    final Object[] array = mBaseCache;
                    final Object[] array = mBaseCache;
                    mArray = array;
                    mArray = array;
                    try {
                        mBaseCache = (Object[]) array[0];
                        mBaseCache = (Object[]) array[0];
                        mHashes = (int[]) array[1];
                        mHashes = (int[]) array[1];
                        if (mHashes != null) {
                            array[0] = array[1] = null;
                            array[0] = array[1] = null;
                            mBaseCacheSize--;
                            mBaseCacheSize--;
                    if (DEBUG) Log.d(TAG, "Retrieving 1x cache " + mHashes
                            if (DEBUG) {
                                Log.d(TAG, "Retrieving 1x cache " + mHashes
                                        + " now have " + mBaseCacheSize + " entries");
                                        + " now have " + mBaseCacheSize + " entries");
                            }
                            return;
                            return;
                        }
                        }
                    } catch (ClassCastException e) {
                    }
                    // Whoops!  Someone trampled the array (probably due to not protecting
                    // their access with a lock).  Our cache is corrupt; report and give up.
                    Slog.wtf(TAG, "Found corrupt ArrayMap cache: [0]=" + array[0]
                            + " [1]=" + array[1]);
                    mBaseCache = null;
                    mBaseCacheSize = 0;
                }
            }
            }
        }
        }


@@ -242,10 +276,14 @@ public final class ArrayMap<K, V> implements Map<K, V> {
        mArray = new Object[size<<1];
        mArray = new Object[size<<1];
    }
    }


    /**
     * Make sure <b>NOT</b> to call this method with arrays that can still be modified. In other
     * words, don't pass mHashes or mArray in directly.
     */
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    private static void freeArrays(final int[] hashes, final Object[] array, final int size) {
    private static void freeArrays(final int[] hashes, final Object[] array, final int size) {
        if (hashes.length == (BASE_SIZE*2)) {
        if (hashes.length == (BASE_SIZE*2)) {
            synchronized (ArrayMap.class) {
            synchronized (sTwiceBaseCacheLock) {
                if (mTwiceBaseCacheSize < CACHE_SIZE) {
                if (mTwiceBaseCacheSize < CACHE_SIZE) {
                    array[0] = mTwiceBaseCache;
                    array[0] = mTwiceBaseCache;
                    array[1] = hashes;
                    array[1] = hashes;
@@ -259,7 +297,7 @@ public final class ArrayMap<K, V> implements Map<K, V> {
                }
                }
            }
            }
        } else if (hashes.length == BASE_SIZE) {
        } else if (hashes.length == BASE_SIZE) {
            synchronized (ArrayMap.class) {
            synchronized (sBaseCacheLock) {
                if (mBaseCacheSize < CACHE_SIZE) {
                if (mBaseCacheSize < CACHE_SIZE) {
                    array[0] = mBaseCache;
                    array[0] = mBaseCache;
                    array[1] = hashes;
                    array[1] = hashes;
+101 −45
Original line number Original line Diff line number Diff line
@@ -23,6 +23,7 @@ import libcore.util.EmptyArray;


import java.lang.reflect.Array;
import java.lang.reflect.Array;
import java.util.Collection;
import java.util.Collection;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
import java.util.Iterator;
import java.util.Map;
import java.util.Map;
import java.util.Set;
import java.util.Set;
@@ -46,6 +47,8 @@ import java.util.function.Predicate;
 * you have no control over this shrinking -- if you set a capacity and then remove an
 * you have no control over this shrinking -- if you set a capacity and then remove an
 * item, it may reduce the capacity to better match the current size.  In the future an
 * item, it may reduce the capacity to better match the current size.  In the future an
 * explicit call to set the capacity should turn off this aggressive shrinking behavior.</p>
 * explicit call to set the capacity should turn off this aggressive shrinking behavior.</p>
 *
 * <p>This structure is <b>NOT</b> thread-safe.</p>
 */
 */
public final class ArraySet<E> implements Collection<E>, Set<E> {
public final class ArraySet<E> implements Collection<E>, Set<E> {
    private static final boolean DEBUG = false;
    private static final boolean DEBUG = false;
@@ -72,15 +75,30 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
    static int sBaseCacheSize;
    static int sBaseCacheSize;
    static Object[] sTwiceBaseCache;
    static Object[] sTwiceBaseCache;
    static int sTwiceBaseCacheSize;
    static int sTwiceBaseCacheSize;
    /**
     * Separate locks for each cache since each can be accessed independently of the other without
     * risk of a deadlock.
     */
    private static final Object sBaseCacheLock = new Object();
    private static final Object sTwiceBaseCacheLock = new Object();


    final boolean mIdentityHashCode;
    private final boolean mIdentityHashCode;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use public API.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use public API.
    int[] mHashes;
    int[] mHashes;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Storage is an implementation detail. Use public API.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Storage is an implementation detail. Use public API.
    Object[] mArray;
    Object[] mArray;
    @UnsupportedAppUsage(maxTargetSdk = 28) // Use size()
    @UnsupportedAppUsage(maxTargetSdk = 28) // Use size()
    int mSize;
    int mSize;
    MapCollections<E, E> mCollections;
    private MapCollections<E, E> mCollections;

    private int binarySearch(int[] hashes, int hash) {
        try {
            return ContainerHelpers.binarySearch(hashes, mSize, hash);
        } catch (ArrayIndexOutOfBoundsException e) {
            throw new ConcurrentModificationException();
        }
    }



    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use indexOfKey(Object).
    @UnsupportedAppUsage(maxTargetSdk = 28) // Hashes are an implementation detail. Use indexOfKey(Object).
    private int indexOf(Object key, int hash) {
    private int indexOf(Object key, int hash) {
@@ -91,7 +109,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            return ~0;
            return ~0;
        }
        }


        int index = ContainerHelpers.binarySearch(mHashes, N, hash);
        int index = binarySearch(mHashes, hash);


        // If the hash code wasn't found, then we have no entry for this key.
        // If the hash code wasn't found, then we have no entry for this key.
        if (index < 0) {
        if (index < 0) {
@@ -130,7 +148,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            return ~0;
            return ~0;
        }
        }


        int index = ContainerHelpers.binarySearch(mHashes, N, 0);
        int index = binarySearch(mHashes, 0);


        // If the hash code wasn't found, then we have no entry for this key.
        // If the hash code wasn't found, then we have no entry for this key.
        if (index < 0) {
        if (index < 0) {
@@ -163,13 +181,14 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    private void allocArrays(final int size) {
    private void allocArrays(final int size) {
        if (size == (BASE_SIZE * 2)) {
        if (size == (BASE_SIZE * 2)) {
            synchronized (ArraySet.class) {
            synchronized (sTwiceBaseCacheLock) {
                if (sTwiceBaseCache != null) {
                if (sTwiceBaseCache != null) {
                    final Object[] array = sTwiceBaseCache;
                    final Object[] array = sTwiceBaseCache;
                    try {
                    try {
                        mArray = array;
                        mArray = array;
                        sTwiceBaseCache = (Object[]) array[0];
                        sTwiceBaseCache = (Object[]) array[0];
                        mHashes = (int[]) array[1];
                        mHashes = (int[]) array[1];
                        if (mHashes != null) {
                            array[0] = array[1] = null;
                            array[0] = array[1] = null;
                            sTwiceBaseCacheSize--;
                            sTwiceBaseCacheSize--;
                            if (DEBUG) {
                            if (DEBUG) {
@@ -177,6 +196,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
                                        + sTwiceBaseCacheSize + " entries");
                                        + sTwiceBaseCacheSize + " entries");
                            }
                            }
                            return;
                            return;
                        }
                    } catch (ClassCastException e) {
                    } catch (ClassCastException e) {
                    }
                    }
                    // Whoops!  Someone trampled the array (probably due to not protecting
                    // Whoops!  Someone trampled the array (probably due to not protecting
@@ -188,20 +208,22 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
                }
                }
            }
            }
        } else if (size == BASE_SIZE) {
        } else if (size == BASE_SIZE) {
            synchronized (ArraySet.class) {
            synchronized (sBaseCacheLock) {
                if (sBaseCache != null) {
                if (sBaseCache != null) {
                    final Object[] array = sBaseCache;
                    final Object[] array = sBaseCache;
                    try {
                    try {
                        mArray = array;
                        mArray = array;
                        sBaseCache = (Object[]) array[0];
                        sBaseCache = (Object[]) array[0];
                        mHashes = (int[]) array[1];
                        mHashes = (int[]) array[1];
                        if (mHashes != null) {
                            array[0] = array[1] = null;
                            array[0] = array[1] = null;
                            sBaseCacheSize--;
                            sBaseCacheSize--;
                            if (DEBUG) {
                            if (DEBUG) {
                            Log.d(TAG, "Retrieving 1x cache " + mHashes + " now have " + sBaseCacheSize
                                Log.d(TAG, "Retrieving 1x cache " + mHashes + " now have "
                                    + " entries");
                                        + sBaseCacheSize + " entries");
                            }
                            }
                            return;
                            return;
                        }
                    } catch (ClassCastException e) {
                    } catch (ClassCastException e) {
                    }
                    }
                    // Whoops!  Someone trampled the array (probably due to not protecting
                    // Whoops!  Someone trampled the array (probably due to not protecting
@@ -218,10 +240,14 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
        mArray = new Object[size];
        mArray = new Object[size];
    }
    }


    /**
     * Make sure <b>NOT</b> to call this method with arrays that can still be modified. In other
     * words, don't pass mHashes or mArray in directly.
     */
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    @UnsupportedAppUsage(maxTargetSdk = 28) // Allocations are an implementation detail.
    private static void freeArrays(final int[] hashes, final Object[] array, final int size) {
    private static void freeArrays(final int[] hashes, final Object[] array, final int size) {
        if (hashes.length == (BASE_SIZE * 2)) {
        if (hashes.length == (BASE_SIZE * 2)) {
            synchronized (ArraySet.class) {
            synchronized (sTwiceBaseCacheLock) {
                if (sTwiceBaseCacheSize < CACHE_SIZE) {
                if (sTwiceBaseCacheSize < CACHE_SIZE) {
                    array[0] = sTwiceBaseCache;
                    array[0] = sTwiceBaseCache;
                    array[1] = hashes;
                    array[1] = hashes;
@@ -237,7 +263,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
                }
                }
            }
            }
        } else if (hashes.length == BASE_SIZE) {
        } else if (hashes.length == BASE_SIZE) {
            synchronized (ArraySet.class) {
            synchronized (sBaseCacheLock) {
                if (sBaseCacheSize < CACHE_SIZE) {
                if (sBaseCacheSize < CACHE_SIZE) {
                    array[0] = sBaseCache;
                    array[0] = sBaseCache;
                    array[1] = hashes;
                    array[1] = hashes;
@@ -308,10 +334,16 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
    @Override
    @Override
    public void clear() {
    public void clear() {
        if (mSize != 0) {
        if (mSize != 0) {
            freeArrays(mHashes, mArray, mSize);
            final int[] ohashes = mHashes;
            final Object[] oarray = mArray;
            final int osize = mSize;
            mHashes = EmptyArray.INT;
            mHashes = EmptyArray.INT;
            mArray = EmptyArray.OBJECT;
            mArray = EmptyArray.OBJECT;
            mSize = 0;
            mSize = 0;
            freeArrays(ohashes, oarray, osize);
        }
        if (mSize != 0) {
            throw new ConcurrentModificationException();
        }
        }
    }
    }


@@ -320,6 +352,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
     * items.
     * items.
     */
     */
    public void ensureCapacity(int minimumCapacity) {
    public void ensureCapacity(int minimumCapacity) {
        final int oSize = mSize;
        if (mHashes.length < minimumCapacity) {
        if (mHashes.length < minimumCapacity) {
            final int[] ohashes = mHashes;
            final int[] ohashes = mHashes;
            final Object[] oarray = mArray;
            final Object[] oarray = mArray;
@@ -330,6 +363,9 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            }
            }
            freeArrays(ohashes, oarray, mSize);
            freeArrays(ohashes, oarray, mSize);
        }
        }
        if (mSize != oSize) {
            throw new ConcurrentModificationException();
        }
    }
    }


    /**
    /**
@@ -400,11 +436,10 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
     *
     *
     * @param value the object to add.
     * @param value the object to add.
     * @return {@code true} if this set is modified, {@code false} otherwise.
     * @return {@code true} if this set is modified, {@code false} otherwise.
     * @throws ClassCastException
     *             when the class of the object is inappropriate for this set.
     */
     */
    @Override
    @Override
    public boolean add(E value) {
    public boolean add(E value) {
        final int oSize = mSize;
        final int hash;
        final int hash;
        int index;
        int index;
        if (value == null) {
        if (value == null) {
@@ -419,9 +454,9 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
        }
        }


        index = ~index;
        index = ~index;
        if (mSize >= mHashes.length) {
        if (oSize >= mHashes.length) {
            final int n = mSize >= (BASE_SIZE * 2) ? (mSize + (mSize >> 1))
            final int n = oSize >= (BASE_SIZE * 2) ? (oSize + (oSize >> 1))
                    : (mSize >= BASE_SIZE ? (BASE_SIZE * 2) : BASE_SIZE);
                    : (oSize >= BASE_SIZE ? (BASE_SIZE * 2) : BASE_SIZE);


            if (DEBUG) Log.d(TAG, "add: grow from " + mHashes.length + " to " + n);
            if (DEBUG) Log.d(TAG, "add: grow from " + mHashes.length + " to " + n);


@@ -429,21 +464,29 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            final Object[] oarray = mArray;
            final Object[] oarray = mArray;
            allocArrays(n);
            allocArrays(n);


            if (oSize != mSize) {
                throw new ConcurrentModificationException();
            }

            if (mHashes.length > 0) {
            if (mHashes.length > 0) {
                if (DEBUG) Log.d(TAG, "add: copy 0-" + mSize + " to 0");
                if (DEBUG) Log.d(TAG, "add: copy 0-" + oSize + " to 0");
                System.arraycopy(ohashes, 0, mHashes, 0, ohashes.length);
                System.arraycopy(ohashes, 0, mHashes, 0, ohashes.length);
                System.arraycopy(oarray, 0, mArray, 0, oarray.length);
                System.arraycopy(oarray, 0, mArray, 0, oarray.length);
            }
            }


            freeArrays(ohashes, oarray, mSize);
            freeArrays(ohashes, oarray, oSize);
        }
        }


        if (index < mSize) {
        if (index < oSize) {
            if (DEBUG) {
            if (DEBUG) {
                Log.d(TAG, "add: move " + index + "-" + (mSize - index) + " to " + (index + 1));
                Log.d(TAG, "add: move " + index + "-" + (oSize - index) + " to " + (index + 1));
            }
            }
            System.arraycopy(mHashes, index, mHashes, index + 1, mSize - index);
            System.arraycopy(mHashes, index, mHashes, index + 1, oSize - index);
            System.arraycopy(mArray, index, mArray, index + 1, mSize - index);
            System.arraycopy(mArray, index, mArray, index + 1, oSize - index);
        }

        if (oSize != mSize || index >= mHashes.length) {
            throw new ConcurrentModificationException();
        }
        }


        mHashes[index] = hash;
        mHashes[index] = hash;
@@ -458,6 +501,7 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
     * @hide
     * @hide
     */
     */
    public void append(E value) {
    public void append(E value) {
        final int oSize = mSize;
        final int index = mSize;
        final int index = mSize;
        final int hash = value == null ? 0
        final int hash = value == null ? 0
                : (mIdentityHashCode ? System.identityHashCode(value) : value.hashCode());
                : (mIdentityHashCode ? System.identityHashCode(value) : value.hashCode());
@@ -476,6 +520,11 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            add(value);
            add(value);
            return;
            return;
        }
        }

        if (oSize != mSize) {
            throw new ConcurrentModificationException();
        }

        mSize = index + 1;
        mSize = index + 1;
        mHashes[index] = hash;
        mHashes[index] = hash;
        mArray[index] = value;
        mArray[index] = value;
@@ -492,6 +541,9 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            if (N > 0) {
            if (N > 0) {
                System.arraycopy(array.mHashes, 0, mHashes, 0, N);
                System.arraycopy(array.mHashes, 0, mHashes, 0, N);
                System.arraycopy(array.mArray, 0, mArray, 0, N);
                System.arraycopy(array.mArray, 0, mArray, 0, N);
                if (0 != mSize) {
                    throw new ConcurrentModificationException();
                }
                mSize = N;
                mSize = N;
            }
            }
        } else {
        } else {
@@ -549,12 +601,14 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
            // Check if exception should be thrown outside of the critical path.
            // Check if exception should be thrown outside of the critical path.
            throw new ArrayIndexOutOfBoundsException(index);
            throw new ArrayIndexOutOfBoundsException(index);
        }
        }
        final int oSize = mSize;
        final Object old = mArray[index];
        final Object old = mArray[index];
        if (mSize <= 1) {
        if (oSize <= 1) {
            // Now empty.
            // Now empty.
            if (DEBUG) Log.d(TAG, "remove: shrink from " + mHashes.length + " to 0");
            if (DEBUG) Log.d(TAG, "remove: shrink from " + mHashes.length + " to 0");
            clear();
            clear();
        } else {
        } else {
            final int nSize = oSize - 1;
            if (shouldShrink()) {
            if (shouldShrink()) {
                // Shrunk enough to reduce size of arrays.
                // Shrunk enough to reduce size of arrays.
                final int n = getNewShrunkenSize();
                final int n = getNewShrunkenSize();
@@ -565,31 +619,33 @@ public final class ArraySet<E> implements Collection<E>, Set<E> {
                final Object[] oarray = mArray;
                final Object[] oarray = mArray;
                allocArrays(n);
                allocArrays(n);


                mSize--;
                if (index > 0) {
                if (index > 0) {
                    if (DEBUG) Log.d(TAG, "remove: copy from 0-" + index + " to 0");
                    if (DEBUG) Log.d(TAG, "remove: copy from 0-" + index + " to 0");
                    System.arraycopy(ohashes, 0, mHashes, 0, index);
                    System.arraycopy(ohashes, 0, mHashes, 0, index);
                    System.arraycopy(oarray, 0, mArray, 0, index);
                    System.arraycopy(oarray, 0, mArray, 0, index);
                }
                }
                if (index < mSize) {
                if (index < nSize) {
                    if (DEBUG) {
                    if (DEBUG) {
                        Log.d(TAG, "remove: copy from " + (index + 1) + "-" + mSize
                        Log.d(TAG, "remove: copy from " + (index + 1) + "-" + nSize
                                + " to " + index);
                                + " to " + index);
                    }
                    }
                    System.arraycopy(ohashes, index + 1, mHashes, index, mSize - index);
                    System.arraycopy(ohashes, index + 1, mHashes, index, nSize - index);
                    System.arraycopy(oarray, index + 1, mArray, index, mSize - index);
                    System.arraycopy(oarray, index + 1, mArray, index, nSize - index);
                }
                }
            } else {
            } else {
                mSize--;
                if (index < nSize) {
                if (index < mSize) {
                    if (DEBUG) {
                    if (DEBUG) {
                        Log.d(TAG, "remove: move " + (index + 1) + "-" + mSize + " to " + index);
                        Log.d(TAG, "remove: move " + (index + 1) + "-" + nSize + " to " + index);
                    }
                    System.arraycopy(mHashes, index + 1, mHashes, index, nSize - index);
                    System.arraycopy(mArray, index + 1, mArray, index, nSize - index);
                }
                }
                    System.arraycopy(mHashes, index + 1, mHashes, index, mSize - index);
                mArray[nSize] = null;
                    System.arraycopy(mArray, index + 1, mArray, index, mSize - index);
            }
            }
                mArray[mSize] = null;
            if (oSize != mSize) {
                throw new ConcurrentModificationException();
            }
            }
            mSize = nSize;
        }
        }
        return (E) old;
        return (E) old;
    }
    }
+113 −0
Original line number Original line Diff line number Diff line
/*
 * Copyright (C) 2019 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package android.util;

import androidx.test.filters.LargeTest;

import junit.framework.TestCase;

import org.junit.After;
import org.junit.Test;

import java.util.ConcurrentModificationException;

/**
 * Unit tests for ArraySet that don't belong in CTS.
 */
@LargeTest
public class ArraySetTest extends TestCase {
    private static final String TAG = "ArraySetTest";
    ArraySet<String> mSet = new ArraySet<>();

    @After
    public void tearDown() {
        mSet = null;
    }

    /**
     * Attempt to generate a ConcurrentModificationException in ArraySet.
     * <p>
     * ArraySet is explicitly documented to be non-thread-safe, yet it's easy to accidentally screw
     * this up; ArraySet should (in the spirit of the core Java collection types) make an effort to
     * catch this and throw ConcurrentModificationException instead of crashing somewhere in its
     * internals.
     */
    @Test
    public void testConcurrentModificationException() throws Exception {
        final int testDurMs = 10_000;
        System.out.println("Starting ArraySet concurrency test");
        new Thread(() -> {
            int i = 0;
            while (mSet != null) {
                try {
                    mSet.add(String.format("key %d", i++));
                } catch (ArrayIndexOutOfBoundsException e) {
                    Log.e(TAG, "concurrent modification uncaught, causing indexing failure", e);
                    fail("Concurrent modification uncaught, causing indexing failure: " + e);
                } catch (ClassCastException e) {
                    Log.e(TAG, "concurrent modification uncaught, causing cache corruption", e);
                    fail("Concurrent modification uncaught, causing cache corruption: " + e);
                } catch (ConcurrentModificationException e) {
                    System.out.println("[successfully caught CME at put #" + i
                            + " size=" + (mSet == null ? "??" : String.valueOf(mSet.size())) + "]");
                    if (i % 200 == 0) {
                        System.out.print(".");
                    }
                }
            }
        }).start();
        for (int i = 0; i < (testDurMs / 100); i++) {
            try {
                if (mSet.size() % 4 == 0) {
                    mSet.clear();
                }
                System.out.print("X");
            } catch (ArrayIndexOutOfBoundsException e) {
                Log.e(TAG, "concurrent modification uncaught, causing indexing failure", e);
                fail("Concurrent modification uncaught, causing indexing failure: " + e);
            } catch (ClassCastException e) {
                Log.e(TAG, "concurrent modification uncaught, causing cache corruption", e);
                fail("Concurrent modification uncaught, causing cache corruption: " + e);
            } catch (ConcurrentModificationException e) {
                System.out.println(
                        "[successfully caught CME at clear #" + i + " size=" + mSet.size() + "]");
            }
        }
    }

    /**
     * Check to make sure the same operations behave as expected in a single thread.
     */
    @Test
    public void testNonConcurrentAccesses() throws Exception {
        for (int i = 0; i < 100000; i++) {
            try {
                mSet.add(String.format("key %d", i++));
                if (i % 200 == 0) {
                    System.out.print(".");
                }
                if (i % 500 == 0) {
                    mSet.clear();
                    System.out.print("X");
                }
            } catch (ConcurrentModificationException e) {
                Log.e(TAG, "concurrent modification caught on single thread", e);
                fail();
            }
        }
    }
}