Skip to content

Commit 01f5a3d

Browse files
committed
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.
1 parent 03f5bd4 commit 01f5a3d

File tree

2 files changed

+12
-13
lines changed

2 files changed

+12
-13
lines changed

library/src/main/java/com/bumptech/glide/load/engine/cache/DiskCacheWriteLocker.java

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.bumptech.glide.load.engine.cache;
22

3-
import com.bumptech.glide.load.Key;
43
import com.bumptech.glide.util.Preconditions;
54
import com.bumptech.glide.util.Synthetic;
65
import java.util.ArrayDeque;
@@ -19,41 +18,41 @@
1918
* 0, the lock can safely be removed from the map. </p>
2019
*/
2120
final class DiskCacheWriteLocker {
22-
private final Map<Key, WriteLock> locks = new HashMap<>();
21+
private final Map<String, WriteLock> locks = new HashMap<>();
2322
private final WriteLockPool writeLockPool = new WriteLockPool();
2423

25-
void acquire(Key key) {
24+
void acquire(String safeKey) {
2625
WriteLock writeLock;
2726
synchronized (this) {
28-
writeLock = locks.get(key);
27+
writeLock = locks.get(safeKey);
2928
if (writeLock == null) {
3029
writeLock = writeLockPool.obtain();
31-
locks.put(key, writeLock);
30+
locks.put(safeKey, writeLock);
3231
}
3332
writeLock.interestedThreads++;
3433
}
3534

3635
writeLock.lock.lock();
3736
}
3837

39-
void release(Key key) {
38+
void release(String safeKey) {
4039
WriteLock writeLock;
4140
synchronized (this) {
42-
writeLock = Preconditions.checkNotNull(locks.get(key));
41+
writeLock = Preconditions.checkNotNull(locks.get(safeKey));
4342
if (writeLock.interestedThreads < 1) {
4443
throw new IllegalStateException("Cannot release a lock that is not held"
45-
+ ", key: " + key
44+
+ ", safeKey: " + safeKey
4645
+ ", interestedThreads: " + writeLock.interestedThreads);
4746
}
4847

4948
writeLock.interestedThreads--;
5049
if (writeLock.interestedThreads == 0) {
51-
WriteLock removed = locks.remove(key);
50+
WriteLock removed = locks.remove(safeKey);
5251
if (!removed.equals(writeLock)) {
5352
throw new IllegalStateException("Removed the wrong lock"
5453
+ ", expected to remove: " + writeLock
5554
+ ", but actually removed: " + removed
56-
+ ", key: " + key);
55+
+ ", safeKey: " + safeKey);
5756
}
5857
writeLockPool.offer(removed);
5958
}

library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ public File get(Key key) {
8888
public void put(Key key, Writer writer) {
8989
// We want to make sure that puts block so that data is available when put completes. We may
9090
// actually not write any data if we find that data is written by the time we acquire the lock.
91-
writeLocker.acquire(key);
91+
String safeKey = safeKeyGenerator.getSafeKey(key);
92+
writeLocker.acquire(safeKey);
9293
try {
93-
String safeKey = safeKeyGenerator.getSafeKey(key);
9494
if (Log.isLoggable(TAG, Log.VERBOSE)) {
9595
Log.v(TAG, "Put: Obtained: " + safeKey + " for for Key: " + key);
9696
}
@@ -121,7 +121,7 @@ public void put(Key key, Writer writer) {
121121
}
122122
}
123123
} finally {
124-
writeLocker.release(key);
124+
writeLocker.release(safeKey);
125125
}
126126
}
127127

0 commit comments

Comments
 (0)