From 01f5a3d3dc0e78cc83b9e390b331a65dd5c1a9f3 Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Thu, 7 Sep 2017 19:11:32 -0700
Subject: [PATCH] Key off of the safe key in writeLock in DiskLruCacheWrapper.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 .../engine/cache/DiskCacheWriteLocker.java    | 19 +++++++++----------
 .../engine/cache/DiskLruCacheWrapper.java     |  6 +++---
 2 files changed, 12 insertions(+), 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
index 13318a8fc..d11f0f02a 100644
--- 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,16 +18,16 @@ import java.util.concurrent.locks.ReentrantLock;
  *  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++;
     }
@@ -36,24 +35,24 @@ final class DiskCacheWriteLocker {
     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
index c0e6f5269..8cd104d02 100644
--- 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 class DiskLruCacheWrapper implements DiskCache {
   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 class DiskLruCacheWrapper implements DiskCache {
         }
       }
     } finally {
-      writeLocker.release(key);
+      writeLocker.release(safeKey);
     }
   }
 
-- 
GitLab