Merge "Use ReentrantReadWriteLock in the compat framework" into sc-dev

services/core/java/com/android/server/compat/CompatConfig.java

@@ -58,6 +58,8 @@ import java.io.PrintWriter;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.xml.datatype.DatatypeConfigurationException;
 
@@ -74,12 +76,14 @@ final class CompatConfig {
     private static final String STATIC_OVERRIDES_PRODUCT_DIR = "/product/etc/appcompat";
     private static final String OVERRIDES_FILE = "compat_framework_overrides.xml";
 
-    @GuardedBy("mChanges")
+    private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock();
+    @GuardedBy("mReadWriteLock")
     private final LongSparseArray<CompatChange> mChanges = new LongSparseArray<>();
 
     private final OverrideValidatorImpl mOverrideValidator;
     private final AndroidBuildClassifier mAndroidBuildClassifier;
     private Context mContext;
+    @GuardedBy("mOverridesFile")
     private File mOverridesFile;
 
     @VisibleForTesting
@@ -117,9 +121,12 @@ final class CompatConfig {
      * @param change the change to add
      */
     void addChange(CompatChange change) {
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             mChanges.put(change.getId(), change);
             invalidateCache();
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
     }
 
@@ -136,13 +143,16 @@ final class CompatConfig {
      */
     long[] getDisabledChanges(ApplicationInfo app) {
         LongArray disabled = new LongArray();
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 CompatChange c = mChanges.valueAt(i);
                 if (!c.isEnabled(app, mAndroidBuildClassifier)) {
                     disabled.add(c.getId());
                 }
             }
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
         // Note: we don't need to explicitly sort the array, as the behaviour of LongSparseArray
         // (mChanges) ensures it's already sorted.
@@ -156,12 +166,15 @@ final class CompatConfig {
      * @return the change ID, or {@code -1} if no change with that name exists
      */
     long lookupChangeId(String name) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 if (TextUtils.equals(mChanges.valueAt(i).getName(), name)) {
                     return mChanges.keyAt(i);
                 }
             }
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
         return -1;
     }
@@ -175,13 +188,16 @@ final class CompatConfig {
      * change ID is not known, as unknown changes are enabled by default.
      */
     boolean isChangeEnabled(long changeId, ApplicationInfo app) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c == null) {
                 // we know nothing about this change: default behaviour is enabled.
                 return true;
             }
             return c.isEnabled(app, mAndroidBuildClassifier);
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -194,13 +210,16 @@ final class CompatConfig {
      * {@code true} if the change ID is not known, as unknown changes are enabled by default.
      */
     boolean willChangeBeEnabled(long changeId, String packageName) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c == null) {
                 // we know nothing about this change: default behaviour is enabled.
                 return true;
             }
             return c.willBeEnabled(packageName);
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -251,7 +270,8 @@ final class CompatConfig {
                 mOverrideValidator.getOverrideAllowedState(changeId, packageName);
         allowedState.enforce(changeId, packageName);
         Long versionCode = getVersionCodeOrNull(packageName);
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c == null) {
                 alreadyKnown = false;
@@ -259,16 +279,21 @@ final class CompatConfig {
                 addChange(c);
             }
             c.addPackageOverride(packageName, overrides, allowedState, versionCode);
+            invalidateCache();
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
-        invalidateCache();
         return alreadyKnown;
     }
 
     /** Checks whether the change is known to the compat config. */
     boolean isKnownChangeId(long changeId) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             return c != null;
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -277,12 +302,15 @@ final class CompatConfig {
      * target SDK gated).
      */
     int maxTargetSdkForChangeIdOptIn(long changeId) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c != null && c.getEnableSinceTargetSdk() != -1) {
                 return c.getEnableSinceTargetSdk() - 1;
             }
             return -1;
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -290,9 +318,12 @@ final class CompatConfig {
      * Returns whether the change is marked as logging only.
      */
     boolean isLoggingOnly(long changeId) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             return c != null && c.getLoggingOnly();
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -300,9 +331,12 @@ final class CompatConfig {
      * Returns whether the change is marked as disabled.
      */
     boolean isDisabled(long changeId) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             return c != null && c.getDisabled();
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -310,9 +344,12 @@ final class CompatConfig {
      * Returns whether the change is overridable.
      */
     boolean isOverridable(long changeId) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             return c != null && c.getOverridable();
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -339,11 +376,14 @@ final class CompatConfig {
      */
     private boolean removeOverrideUnsafe(long changeId, String packageName) {
         Long versionCode = getVersionCodeOrNull(packageName);
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c != null) {
                 return removeOverrideUnsafe(c, packageName, versionCode);
             }
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
         return false;
     }
@@ -376,11 +416,14 @@ final class CompatConfig {
      */
     void removePackageOverrides(String packageName) {
         Long versionCode = getVersionCodeOrNull(packageName);
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 CompatChange change = mChanges.valueAt(i);
                 removeOverrideUnsafe(change, packageName, versionCode);
             }
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
         saveOverrides();
         invalidateCache();
@@ -408,7 +451,8 @@ final class CompatConfig {
     private long[] getAllowedChangesSinceTargetSdkForPackage(String packageName,
             int targetSdkVersion) {
         LongArray allowed = new LongArray();
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 CompatChange change = mChanges.valueAt(i);
                 if (change.getEnableSinceTargetSdk() != targetSdkVersion) {
@@ -421,6 +465,8 @@ final class CompatConfig {
                     allowed.add(change.getId());
                 }
             }
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
         return allowed.toArray();
     }
@@ -461,7 +507,8 @@ final class CompatConfig {
 
     boolean registerListener(long changeId, CompatChange.ChangeListener listener) {
         boolean alreadyKnown = true;
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             CompatChange c = mChanges.get(changeId);
             if (c == null) {
                 alreadyKnown = false;
@@ -469,6 +516,8 @@ final class CompatConfig {
                 addChange(c);
             }
             c.registerListener(listener);
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
         return alreadyKnown;
     }
@@ -488,8 +537,11 @@ final class CompatConfig {
 
     @VisibleForTesting
     void clearChanges() {
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             mChanges.clear();
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
     }
 
@@ -499,7 +551,8 @@ final class CompatConfig {
      * @param pw {@link PrintWriter} instance to which the information will be dumped
      */
     void dumpConfig(PrintWriter pw) {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             if (mChanges.size() == 0) {
                 pw.println("No compat overrides.");
                 return;
@@ -508,6 +561,8 @@ final class CompatConfig {
                 CompatChange c = mChanges.valueAt(i);
                 pw.println(c.toString());
             }
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -519,7 +574,8 @@ final class CompatConfig {
     CompatibilityChangeConfig getAppConfig(ApplicationInfo applicationInfo) {
         Set<Long> enabled = new HashSet<>();
         Set<Long> disabled = new HashSet<>();
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 CompatChange c = mChanges.valueAt(i);
                 if (c.isEnabled(applicationInfo, mAndroidBuildClassifier)) {
@@ -528,6 +584,8 @@ final class CompatConfig {
                     disabled.add(c.getId());
                 }
             }
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
         return new CompatibilityChangeConfig(new ChangeConfig(enabled, disabled));
     }
@@ -538,13 +596,16 @@ final class CompatConfig {
      * @return an array of {@link CompatibilityChangeInfo} with the current changes
      */
     CompatibilityChangeInfo[] dumpChanges() {
-        synchronized (mChanges) {
+        mReadWriteLock.readLock().lock();
+        try {
             CompatibilityChangeInfo[] changeInfos = new CompatibilityChangeInfo[mChanges.size()];
             for (int i = 0; i < mChanges.size(); ++i) {
                 CompatChange change = mChanges.valueAt(i);
                 changeInfos[i] = new CompatibilityChangeInfo(change);
             }
             return changeInfos;
+        } finally {
+            mReadWriteLock.readLock().unlock();
         }
     }
 
@@ -580,10 +641,13 @@ final class CompatConfig {
     @VisibleForTesting
     void initOverrides(File dynamicOverridesFile, File staticOverridesFile) {
         // Clear overrides from all changes before loading.
-        synchronized (mChanges) {
+        mReadWriteLock.writeLock().lock();
+        try {
             for (int i = 0; i < mChanges.size(); ++i) {
                 mChanges.valueAt(i).clearOverrides();
             }
+        } finally {
+            mReadWriteLock.writeLock().unlock();
         }
 
         loadOverrides(staticOverridesFile);
@@ -632,18 +696,21 @@ final class CompatConfig {
         if (mOverridesFile == null) {
             return;
         }
-        Overrides overrides = new Overrides();
-        synchronized (mChanges) {
-            List<ChangeOverrides> changeOverridesList = overrides.getChangeOverrides();
-            for (int idx = 0; idx < mChanges.size(); ++idx) {
-                CompatChange c = mChanges.valueAt(idx);
-                ChangeOverrides changeOverrides = c.saveOverrides();
-                if (changeOverrides != null) {
-                    changeOverridesList.add(changeOverrides);
-                }
-            }
-        }
         synchronized (mOverridesFile) {
+            Overrides overrides = new Overrides();
+            mReadWriteLock.readLock().lock();
+            try {
+                List<ChangeOverrides> changeOverridesList = overrides.getChangeOverrides();
+                for (int idx = 0; idx < mChanges.size(); ++idx) {
+                    CompatChange c = mChanges.valueAt(idx);
+                    ChangeOverrides changeOverrides = c.saveOverrides();
+                    if (changeOverrides != null) {
+                        changeOverridesList.add(changeOverrides);
+                    }
+                }
+            } finally {
+                mReadWriteLock.readLock().unlock();
+            }
             // Create the file if it doesn't already exist
             try {
                 mOverridesFile.createNewFile();
@@ -673,8 +740,9 @@ final class CompatConfig {
      */
     void recheckOverrides(String packageName) {
         Long versionCode = getVersionCodeOrNull(packageName);
-        synchronized (mChanges) {
+        boolean shouldInvalidateCache = false;
-            boolean shouldInvalidateCache = false;
+        mReadWriteLock.readLock().lock();
+        try {
             for (int idx = 0; idx < mChanges.size(); ++idx) {
                 CompatChange c = mChanges.valueAt(idx);
                 if (!c.hasPackageOverride(packageName)) {
@@ -685,9 +753,11 @@ final class CompatConfig {
                                 packageName);
                 shouldInvalidateCache |= c.recheckOverride(packageName, allowedState, versionCode);
             }
-            if (shouldInvalidateCache) {
+        } finally {
-                invalidateCache();
+            mReadWriteLock.readLock().unlock();
-            }
+        }
+        if (shouldInvalidateCache) {
+            invalidateCache();
         }
     }