Key off of the safe key in writeLock in DiskLruCacheWrapper.

The Key we’re given may have a different implementation for equals and hashCode than it does for updateDiskCacheKey. As a result, two different Key objects that are not equal to each other may produce the same disk cache key. To avoid simultaneous puts for the same disk cache key we need to lock on the disk cache key, not the original key object.
bumptech · Sep 8, 2017 · 01f5a3d · 01f5a3d
1 parent 03f5bd4
commit 01f5a3d
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 13 deletions.
diff --git a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskCacheWriteLocker.java b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskCacheWriteLocker.java
@@ -1,6 +1,5 @@
 package com.bumptech.glide.load.engine.cache;
 
-import com.bumptech.glide.load.Key;
 import com.bumptech.glide.util.Preconditions;
 import com.bumptech.glide.util.Synthetic;
 import java.util.ArrayDeque;
@@ -19,41 +18,41 @@
  *  0, the lock can safely be removed from the map. </p>
  */
 final class DiskCacheWriteLocker {
-  private final Map<Key, WriteLock> locks = new HashMap<>();
+  private final Map<String, WriteLock> locks = new HashMap<>();
   private final WriteLockPool writeLockPool = new WriteLockPool();
 
-  void acquire(Key key) {
+  void acquire(String safeKey) {
     WriteLock writeLock;
     synchronized (this) {
-      writeLock = locks.get(key);
+      writeLock = locks.get(safeKey);
       if (writeLock == null) {
         writeLock = writeLockPool.obtain();
-        locks.put(key, writeLock);
+        locks.put(safeKey, writeLock);
       }
       writeLock.interestedThreads++;
     }
 
     writeLock.lock.lock();
   }
 
-  void release(Key key) {
+  void release(String safeKey) {
     WriteLock writeLock;
     synchronized (this) {
-      writeLock = Preconditions.checkNotNull(locks.get(key));
+      writeLock = Preconditions.checkNotNull(locks.get(safeKey));
       if (writeLock.interestedThreads < 1) {
         throw new IllegalStateException("Cannot release a lock that is not held"
-            + ", key: " + key
+            + ", safeKey: " + safeKey
             + ", interestedThreads: " + writeLock.interestedThreads);
       }
 
       writeLock.interestedThreads--;
       if (writeLock.interestedThreads == 0) {
-        WriteLock removed = locks.remove(key);
+        WriteLock removed = locks.remove(safeKey);
         if (!removed.equals(writeLock)) {
           throw new IllegalStateException("Removed the wrong lock"
               + ", expected to remove: " + writeLock
               + ", but actually removed: " + removed
-              + ", key: " + key);
+              + ", safeKey: " + safeKey);
         }
         writeLockPool.offer(removed);
       }

diff --git a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java
@@ -88,9 +88,9 @@ public File get(Key key) {
   public void put(Key key, Writer writer) {
     // We want to make sure that puts block so that data is available when put completes. We may
     // actually not write any data if we find that data is written by the time we acquire the lock.
-    writeLocker.acquire(key);
+    String safeKey = safeKeyGenerator.getSafeKey(key);
+    writeLocker.acquire(safeKey);
     try {
-      String safeKey = safeKeyGenerator.getSafeKey(key);
       if (Log.isLoggable(TAG, Log.VERBOSE)) {
         Log.v(TAG, "Put: Obtained: " + safeKey + " for for Key: " + key);
       }
@@ -121,7 +121,7 @@ public void put(Key key, Writer writer) {
         }
       }
     } finally {
-      writeLocker.release(key);
+      writeLocker.release(safeKey);
     }
   }