From 01ea6a544d6159ed986e4f8628a4d962a7aa520d Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Mon, 5 Aug 2019 10:57:26 -0700
Subject: [PATCH] Avoid deadlock between SingleRequest and RequestFutureTarget.

Clearing the Request object outside of RequestFutureTarget's lock should prevent the RequestFutureTarget -> Request path, which will eliminate the deadlock. Since a new Request object is created for each subsequent load, there isn't any risk that clearing the Request outside of the lock will end up clearing a new or restarted request.

PiperOrigin-RevId: 261721324
---
 .../glide/request/RequestFutureTarget.java    | 46 ++++++++++++++-----
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java b/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java
index dcdb60d8b5..c8216f9df1 100644
--- a/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java
+++ b/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java
@@ -1,6 +1,7 @@
 package com.bumptech.glide.request;
 
 import android.graphics.drawable.Drawable;
+import androidx.annotation.GuardedBy;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
@@ -57,12 +58,26 @@ public class RequestFutureTarget<R> implements FutureTarget<R>, RequestListener<
   private final boolean assertBackgroundThread;
   private final Waiter waiter;
 
-  @Nullable private R resource;
-  @Nullable private Request request;
+  @GuardedBy("this")
+  @Nullable
+  private R resource;
+
+  @GuardedBy("this")
+  @Nullable
+  private Request request;
+
+  @GuardedBy("this")
   private boolean isCancelled;
+
+  @GuardedBy("this")
   private boolean resultReceived;
+
+  @GuardedBy("this")
   private boolean loadFailed;
-  @Nullable private GlideException exception;
+
+  @GuardedBy("this")
+  @Nullable
+  private GlideException exception;
 
   /** Constructor for a RequestFutureTarget. Should not be used directly. */
   public RequestFutureTarget(int width, int height) {
@@ -77,15 +92,24 @@ public RequestFutureTarget(int width, int height) {
   }
 
   @Override
-  public synchronized boolean cancel(boolean mayInterruptIfRunning) {
-    if (isDone()) {
-      return false;
+  public boolean cancel(boolean mayInterruptIfRunning) {
+    Request toClear = null;
+    synchronized (this) {
+      if (isDone()) {
+        return false;
+      }
+
+      isCancelled = true;
+      waiter.notifyAll(this);
+      if (mayInterruptIfRunning) {
+        toClear = request;
+        request = null;
+      }
     }
-    isCancelled = true;
-    waiter.notifyAll(this);
-    if (mayInterruptIfRunning && request != null) {
-      request.clear();
-      request = null;
+
+    // Avoid deadlock by clearing outside of the lock (b/138335419)
+    if (toClear != null) {
+      toClear.clear();
     }
     return true;
   }