OSDN Git Service

Use local locks in SharedPreferencesImpl.
authorPhilip P. Moltmann <moltmann@google.com>
Thu, 15 Dec 2016 22:23:51 +0000 (14:23 -0800)
committerPhilip P. Moltmann <moltmann@google.com>
Thu, 15 Dec 2016 22:44:50 +0000 (14:44 -0800)
Fixes: 33430093
Test: SharedPreferences CTS tests
Change-Id: Ia2ac48a0273608d5be6a227dc6669cea9d44bc1d

core/java/android/app/SharedPreferencesImpl.java

index 5943433..023b4f3 100644 (file)
@@ -54,23 +54,34 @@ import libcore.io.IoUtils;
 final class SharedPreferencesImpl implements SharedPreferences {
     private static final String TAG = "SharedPreferencesImpl";
     private static final boolean DEBUG = false;
+    private static final Object CONTENT = new Object();
 
     // Lock ordering rules:
-    //  - acquire SharedPreferencesImpl.this before EditorImpl.this
-    //  - acquire mWritingToDiskLock before EditorImpl.this
+    //  - acquire SharedPreferencesImpl.mLock before EditorImpl.mLock
+    //  - acquire mWritingToDiskLock before EditorImpl.mLock
 
     private final File mFile;
     private final File mBackupFile;
     private final int mMode;
+    private final Object mLock = new Object();
+    private final Object mWritingToDiskLock = new Object();
 
-    private Map<String, Object> mMap;     // guarded by 'this'
-    private int mDiskWritesInFlight = 0;  // guarded by 'this'
-    private boolean mLoaded = false;      // guarded by 'this'
-    private long mStatTimestamp;          // guarded by 'this'
-    private long mStatSize;               // guarded by 'this'
+    @GuardedBy("mLock")
+    private Map<String, Object> mMap;
 
-    private final Object mWritingToDiskLock = new Object();
-    private static final Object mContent = new Object();
+    @GuardedBy("mLock")
+    private int mDiskWritesInFlight = 0;
+
+    @GuardedBy("mLock")
+    private boolean mLoaded = false;
+
+    @GuardedBy("mLock")
+    private long mStatTimestamp;
+
+    @GuardedBy("mLock")
+    private long mStatSize;
+
+    @GuardedBy("mLock")
     private final WeakHashMap<OnSharedPreferenceChangeListener, Object> mListeners =
             new WeakHashMap<OnSharedPreferenceChangeListener, Object>();
 
@@ -92,7 +103,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
     }
 
     private void startLoadFromDisk() {
-        synchronized (this) {
+        synchronized (mLock) {
             mLoaded = false;
         }
         new Thread("SharedPreferencesImpl-load") {
@@ -103,7 +114,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
     }
 
     private void loadFromDisk() {
-        synchronized (SharedPreferencesImpl.this) {
+        synchronized (mLock) {
             if (mLoaded) {
                 return;
             }
@@ -138,7 +149,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
             /* ignore */
         }
 
-        synchronized (SharedPreferencesImpl.this) {
+        synchronized (mLock) {
             mLoaded = true;
             if (map != null) {
                 mMap = map;
@@ -147,7 +158,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
             } else {
                 mMap = new HashMap<>();
             }
-            notifyAll();
+            mLock.notifyAll();
         }
     }
 
@@ -156,7 +167,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
     }
 
     void startReloadIfChangedUnexpectedly() {
-        synchronized (this) {
+        synchronized (mLock) {
             // TODO: wait for any pending writes to disk?
             if (!hasFileChangedUnexpectedly()) {
                 return;
@@ -168,7 +179,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
     // Has the file changed out from under us?  i.e. writes that
     // we didn't instigate.
     private boolean hasFileChangedUnexpectedly() {
-        synchronized (this) {
+        synchronized (mLock) {
             if (mDiskWritesInFlight > 0) {
                 // If we know we caused it, it's not unexpected.
                 if (DEBUG) Log.d(TAG, "disk write in flight, not unexpected.");
@@ -188,19 +199,19 @@ final class SharedPreferencesImpl implements SharedPreferences {
             return true;
         }
 
-        synchronized (this) {
+        synchronized (mLock) {
             return mStatTimestamp != stat.st_mtime || mStatSize != stat.st_size;
         }
     }
 
     public void registerOnSharedPreferenceChangeListener(OnSharedPreferenceChangeListener listener) {
-        synchronized(this) {
-            mListeners.put(listener, mContent);
+        synchronized(mLock) {
+            mListeners.put(listener, CONTENT);
         }
     }
 
     public void unregisterOnSharedPreferenceChangeListener(OnSharedPreferenceChangeListener listener) {
-        synchronized(this) {
+        synchronized(mLock) {
             mListeners.remove(listener);
         }
     }
@@ -214,14 +225,14 @@ final class SharedPreferencesImpl implements SharedPreferences {
         }
         while (!mLoaded) {
             try {
-                wait();
+                mLock.wait();
             } catch (InterruptedException unused) {
             }
         }
     }
 
     public Map<String, ?> getAll() {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             //noinspection unchecked
             return new HashMap<String, Object>(mMap);
@@ -230,7 +241,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
 
     @Nullable
     public String getString(String key, @Nullable String defValue) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             String v = (String)mMap.get(key);
             return v != null ? v : defValue;
@@ -239,7 +250,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
 
     @Nullable
     public Set<String> getStringSet(String key, @Nullable Set<String> defValues) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             Set<String> v = (Set<String>) mMap.get(key);
             return v != null ? v : defValues;
@@ -247,28 +258,28 @@ final class SharedPreferencesImpl implements SharedPreferences {
     }
 
     public int getInt(String key, int defValue) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             Integer v = (Integer)mMap.get(key);
             return v != null ? v : defValue;
         }
     }
     public long getLong(String key, long defValue) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             Long v = (Long)mMap.get(key);
             return v != null ? v : defValue;
         }
     }
     public float getFloat(String key, float defValue) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             Float v = (Float)mMap.get(key);
             return v != null ? v : defValue;
         }
     }
     public boolean getBoolean(String key, boolean defValue) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             Boolean v = (Boolean)mMap.get(key);
             return v != null ? v : defValue;
@@ -276,7 +287,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
     }
 
     public boolean contains(String key) {
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
             return mMap.containsKey(key);
         }
@@ -290,7 +301,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
         //      context.getSharedPreferences(..).edit().putString(..).apply()
         //
         // ... all without blocking.
-        synchronized (this) {
+        synchronized (mLock) {
             awaitLoadedLocked();
         }
 
@@ -299,70 +310,86 @@ final class SharedPreferencesImpl implements SharedPreferences {
 
     // Return value from EditorImpl#commitToMemory()
     private static class MemoryCommitResult {
-        public long memoryStateGeneration;
-        public List<String> keysModified;  // may be null
-        public Set<OnSharedPreferenceChangeListener> listeners;  // may be null
-        public Map<?, ?> mapToWriteToDisk;
-        public final CountDownLatch writtenToDiskLatch = new CountDownLatch(1);
-        public volatile boolean writeToDiskResult = false;
-
-        public void setDiskWriteResult(boolean result) {
+        final long memoryStateGeneration;
+        @Nullable final List<String> keysModified;
+        @Nullable final Set<OnSharedPreferenceChangeListener> listeners;
+        final Map<String, Object> mapToWriteToDisk;
+        final CountDownLatch writtenToDiskLatch = new CountDownLatch(1);
+
+        @GuardedBy("mWritingToDiskLock")
+        volatile boolean writeToDiskResult = false;
+
+        private MemoryCommitResult(long memoryStateGeneration, @Nullable List<String> keysModified,
+                @Nullable Set<OnSharedPreferenceChangeListener> listeners,
+                Map<String, Object> mapToWriteToDisk) {
+            this.memoryStateGeneration = memoryStateGeneration;
+            this.keysModified = keysModified;
+            this.listeners = listeners;
+            this.mapToWriteToDisk = mapToWriteToDisk;
+        }
+
+        void setDiskWriteResult(boolean result) {
             writeToDiskResult = result;
             writtenToDiskLatch.countDown();
         }
     }
 
     public final class EditorImpl implements Editor {
+        private final Object mLock = new Object();
+
+        @GuardedBy("mLock")
         private final Map<String, Object> mModified = Maps.newHashMap();
+
+        @GuardedBy("mLock")
         private boolean mClear = false;
 
         public Editor putString(String key, @Nullable String value) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, value);
                 return this;
             }
         }
         public Editor putStringSet(String key, @Nullable Set<String> values) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key,
                         (values == null) ? null : new HashSet<String>(values));
                 return this;
             }
         }
         public Editor putInt(String key, int value) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, value);
                 return this;
             }
         }
         public Editor putLong(String key, long value) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, value);
                 return this;
             }
         }
         public Editor putFloat(String key, float value) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, value);
                 return this;
             }
         }
         public Editor putBoolean(String key, boolean value) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, value);
                 return this;
             }
         }
 
         public Editor remove(String key) {
-            synchronized (this) {
+            synchronized (mLock) {
                 mModified.put(key, this);
                 return this;
             }
         }
 
         public Editor clear() {
-            synchronized (this) {
+            synchronized (mLock) {
                 mClear = true;
                 return this;
             }
@@ -399,8 +426,12 @@ final class SharedPreferencesImpl implements SharedPreferences {
 
         // Returns true if any changes were made
         private MemoryCommitResult commitToMemory() {
-            MemoryCommitResult mcr = new MemoryCommitResult();
-            synchronized (SharedPreferencesImpl.this) {
+            long memoryStateGeneration;
+            List<String> keysModified = null;
+            Set<OnSharedPreferenceChangeListener> listeners = null;
+            Map<String, Object> mapToWriteToDisk;
+
+            synchronized (SharedPreferencesImpl.this.mLock) {
                 // We optimistically don't make a deep copy until
                 // a memory commit comes in when we're already
                 // writing to disk.
@@ -411,17 +442,16 @@ final class SharedPreferencesImpl implements SharedPreferences {
                     // noinspection unchecked
                     mMap = new HashMap<String, Object>(mMap);
                 }
-                mcr.mapToWriteToDisk = mMap;
+                mapToWriteToDisk = mMap;
                 mDiskWritesInFlight++;
 
                 boolean hasListeners = mListeners.size() > 0;
                 if (hasListeners) {
-                    mcr.keysModified = new ArrayList<String>();
-                    mcr.listeners =
-                            new HashSet<OnSharedPreferenceChangeListener>(mListeners.keySet());
+                    keysModified = new ArrayList<String>();
+                    listeners = new HashSet<OnSharedPreferenceChangeListener>(mListeners.keySet());
                 }
 
-                synchronized (this) {
+                synchronized (mLock) {
                     boolean changesMade = false;
 
                     if (mClear) {
@@ -455,7 +485,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
 
                         changesMade = true;
                         if (hasListeners) {
-                            mcr.keysModified.add(k);
+                            keysModified.add(k);
                         }
                     }
 
@@ -465,10 +495,11 @@ final class SharedPreferencesImpl implements SharedPreferences {
                         mCurrentMemoryStateGeneration++;
                     }
 
-                    mcr.memoryStateGeneration = mCurrentMemoryStateGeneration;
+                    memoryStateGeneration = mCurrentMemoryStateGeneration;
                 }
             }
-            return mcr;
+            return new MemoryCommitResult(memoryStateGeneration, keysModified, listeners,
+                    mapToWriteToDisk);
         }
 
         public boolean commit() {
@@ -534,7 +565,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
                     synchronized (mWritingToDiskLock) {
                         writeToFile(mcr, isFromSyncCommit);
                     }
-                    synchronized (SharedPreferencesImpl.this) {
+                    synchronized (mLock) {
                         mDiskWritesInFlight--;
                     }
                     if (postWriteRunnable != null) {
@@ -547,7 +578,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
         // the current thread.
         if (isFromSyncCommit) {
             boolean wasEmpty = false;
-            synchronized (SharedPreferencesImpl.this) {
+            synchronized (mLock) {
                 wasEmpty = mDiskWritesInFlight == 1;
             }
             if (wasEmpty) {
@@ -597,7 +628,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
                 if (isFromSyncCommit) {
                     needsWrite = true;
                 } else {
-                    synchronized (this) {
+                    synchronized (mLock) {
                         // No need to persist intermediate states. Just wait for the latest state to
                         // be persisted.
                         if (mCurrentMemoryStateGeneration == mcr.memoryStateGeneration) {
@@ -647,7 +678,7 @@ final class SharedPreferencesImpl implements SharedPreferences {
             ContextImpl.setFilePermissionsFromMode(mFile.getPath(), mMode, 0);
             try {
                 final StructStat stat = Os.stat(mFile.getPath());
-                synchronized (this) {
+                synchronized (mLock) {
                     mStatTimestamp = stat.st_mtime;
                     mStatSize = stat.st_size;
                 }