diff --git a/android/guava/src/com/google/common/cache/LocalCache.java b/android/guava/src/com/google/common/cache/LocalCache.java index 6fb3945df7ee..f0a7699e0700 100644 --- a/android/guava/src/com/google/common/cache/LocalCache.java +++ b/android/guava/src/com/google/common/cache/LocalCache.java @@ -2180,7 +2180,12 @@ V lockedGetOrLoad(K key, int hash, CacheLoader loader) throws Exec if (createNewEntry) { try { - return loadSync(key, hash, loadingValueReference, loader); + // Synchronizes on the entry to allow failing fast when a recursive load is + // detected. This may be circumvented when an entry is copied, but will fail fast most + // of the time. + synchronized (e) { + return loadSync(key, hash, loadingValueReference, loader); + } } finally { statsCounter.recordMisses(1); } @@ -2196,22 +2201,7 @@ V waitForLoadingValue(ReferenceEntry e, K key, ValueReference valueR throw new AssertionError(); } - // As of this writing, the only prod ValueReference implementation for which isLoading() is - // true is LoadingValueReference. (Note, however, that not all LoadingValueReference instances - // have isLoading()==true: LoadingValueReference has a subclass, ComputingValueReference, for - // which isLoading() is false!) However, that might change, and we already have a *test* - // implementation for which it doesn't hold. So we check instanceof to be safe. - if (valueReference instanceof LoadingValueReference) { - // We check whether the thread that is loading the entry is our current thread, which would - // mean that we are both loading and waiting for the entry. In this case, we fail fast - // instead of deadlocking. - checkState( - ((LoadingValueReference) valueReference).getLoadingThread() - != Thread.currentThread(), - "Recursive load of: %s", - key); - } - + checkState(!Thread.holdsLock(e), "Recursive load of: %s", key); // don't consider expiration as we're concurrent with loading try { V value = valueReference.waitForValue(); @@ -3437,8 +3427,6 @@ static class LoadingValueReference implements ValueReference { final SettableFuture futureValue = SettableFuture.create(); final Stopwatch stopwatch = Stopwatch.createUnstarted(); - final Thread loadingThread; - public LoadingValueReference() { this(LocalCache.unset()); } @@ -3450,7 +3438,6 @@ public LoadingValueReference() { */ public LoadingValueReference(ValueReference oldValue) { this.oldValue = oldValue; - this.loadingThread = Thread.currentThread(); } @Override @@ -3554,10 +3541,6 @@ public ValueReference copyFor( ReferenceQueue queue, @CheckForNull V value, ReferenceEntry entry) { return this; } - - Thread getLoadingThread() { - return this.loadingThread; - } } // Queues diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index dd3590357ed0..a38543dbb7b6 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -2184,7 +2184,12 @@ V lockedGetOrLoad(K key, int hash, CacheLoader loader) throws Exec if (createNewEntry) { try { - return loadSync(key, hash, loadingValueReference, loader); + // Synchronizes on the entry to allow failing fast when a recursive load is + // detected. This may be circumvented when an entry is copied, but will fail fast most + // of the time. + synchronized (e) { + return loadSync(key, hash, loadingValueReference, loader); + } } finally { statsCounter.recordMisses(1); } @@ -2200,22 +2205,7 @@ V waitForLoadingValue(ReferenceEntry e, K key, ValueReference valueR throw new AssertionError(); } - // As of this writing, the only prod ValueReference implementation for which isLoading() is - // true is LoadingValueReference. (Note, however, that not all LoadingValueReference instances - // have isLoading()==true: LoadingValueReference has a subclass, ComputingValueReference, for - // which isLoading() is false!) However, that might change, and we already have a *test* - // implementation for which it doesn't hold. So we check instanceof to be safe. - if (valueReference instanceof LoadingValueReference) { - // We check whether the thread that is loading the entry is our current thread, which would - // mean that we are both loading and waiting for the entry. In this case, we fail fast - // instead of deadlocking. - checkState( - ((LoadingValueReference) valueReference).getLoadingThread() - != Thread.currentThread(), - "Recursive load of: %s", - key); - } - + checkState(!Thread.holdsLock(e), "Recursive load of: %s", key); // don't consider expiration as we're concurrent with loading try { V value = valueReference.waitForValue(); @@ -3527,15 +3517,12 @@ static class LoadingValueReference implements ValueReference { final SettableFuture futureValue = SettableFuture.create(); final Stopwatch stopwatch = Stopwatch.createUnstarted(); - final Thread loadingThread; - public LoadingValueReference() { this(null); } public LoadingValueReference(@CheckForNull ValueReference oldValue) { this.oldValue = (oldValue == null) ? LocalCache.unset() : oldValue; - this.loadingThread = Thread.currentThread(); } @Override @@ -3660,10 +3647,6 @@ public ValueReference copyFor( ReferenceQueue queue, @CheckForNull V value, ReferenceEntry entry) { return this; } - - Thread getLoadingThread() { - return this.loadingThread; - } } static class ComputingValueReference extends LoadingValueReference {