Fix deadlock in SingleRequest.clear by using the Engine as the requestLock.

This occurs because CallResourceReady only acquires 2/3 of the necessary locks to perform its runnable (SingleRequest.requestLock & EngineJob.this), and is missing the Engine.this lock.

In this change, I changed SingleRequest.requestLock to always be Engine.this.

Note that change only affects uses of SingleRequest built through RequestBuilder. Users of the public API SingleRequest.obtain are unaffected by this change.

If this fix works as intended, we can consider providing a new SingleRequest.obtain API with the requestLock param removed and deprecating the current SingleRequest.obtain.

PiperOrigin-RevId: 817821083

Author: falhassen

instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java

@@ -53,7 +53,8 @@ public void thumbnail_onResourceReady_forPrimary_isComplete_whenRequestListenerI
     Glide.init(
         context,
         new GlideBuilder()
-            .setSourceExecutor(GlideExecutor.newSourceBuilder().setThreadCount(1).build()));
+            .setSourceExecutor(GlideExecutor.newSourceBuilder().setThreadCount(1).build())
+            .setFixSingleRequestClearDeadlock(true));
 
     AtomicBoolean isPrimaryRequestComplete = new AtomicBoolean(false);
     CountDownLatch countDownLatch = new CountDownLatch(1);

instrumentation/src/androidTest/java/com/bumptech/glide/RequestTest.java

@@ -50,7 +50,10 @@ public void setUp() {
     // Some emulators only have a single resize thread, so waiting on a latch will block them
     // forever.
     Glide.init(
-        context, new GlideBuilder().setSourceExecutor(GlideExecutor.newUnlimitedSourceExecutor()));
+        context,
+        new GlideBuilder()
+            .setSourceExecutor(GlideExecutor.newUnlimitedSourceExecutor())
+            .setFixSingleRequestClearDeadlock(true));
   }
 
   @Test

library/src/main/java/com/bumptech/glide/GlideBuilder.java

@@ -539,6 +539,19 @@ public GlideBuilder setDisableHardwareBitmapsOnO(boolean disableHardwareBitmapsO
     return this;
   }
 
+  /**
+   * Fix single request clear deadlock mentioned in <a
+   * href="https://github.com/bumptech/glide/issues/5617">#5617</a>.
+   *
+   * <p>This flag changes {@link com.bumptech.glide.request.SingleRequest} to lock on underlying
+   * {@link Engine} to avoid deadlocks in calling code that also requires a lock on the {@link
+   * Engine}.
+   */
+  public GlideBuilder setFixSingleRequestClearDeadlock(boolean isEnabled) {
+    glideExperimentsBuilder.update(new FixSingleRequestClearDeadlock(), isEnabled);
+    return this;
+  }
+
   void setRequestManagerFactory(@Nullable RequestManagerFactory factory) {
     this.requestManagerFactory = factory;
   }
@@ -653,4 +666,7 @@ public static final class OverrideGlideThreadPriority implements Experiment {}
 
   /** See {@link #setUseMediaStoreOpenFileApisIfPossible(boolean)}. */
   public static final class UseMediaStoreOpenFileApisIfPossible implements Experiment {}
+
+  /** See {@link #setFixSingleRequestClearDeadlock(boolean)}. */
+  public static final class FixSingleRequestClearDeadlock implements Experiment {}
 }

library/src/main/java/com/bumptech/glide/RequestBuilder.java

@@ -16,6 +16,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.RawRes;
+import com.bumptech.glide.GlideBuilder.FixSingleRequestClearDeadlock;
 import com.bumptech.glide.load.DataSource;
 import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.engine.DiskCacheStrategy;
@@ -68,6 +69,7 @@ public class RequestBuilder<TranscodeType> extends BaseRequestOptions<RequestBui
   private final Class<TranscodeType> transcodeClass;
   private final Glide glide;
   private final GlideContext glideContext;
+  private final GlideExperiments experiments;
 
   @NonNull
   @SuppressWarnings("unchecked")
@@ -99,6 +101,7 @@ protected RequestBuilder(
     this.context = context;
     this.transitionOptions = requestManager.getDefaultTransitionOptions(transcodeClass);
     this.glideContext = glide.getGlideContext();
+    this.experiments = glide.getGlideContext().getExperiments();
 
     initRequestListeners(requestManager.getDefaultRequestListeners());
     apply(requestManager.getDefaultRequestOptions());
@@ -1166,8 +1169,13 @@ private Request buildRequest(
       @Nullable RequestListener<TranscodeType> targetListener,
       BaseRequestOptions<?> requestOptions,
       Executor callbackExecutor) {
+    GlideExperiments experiments = glideContext.getExperiments();
+    Object requestLock =
+        glideContext.getExperiments().isEnabled(FixSingleRequestClearDeadlock.class)
+            ? glideContext.getEngine()
+            : new Object();
     return buildRequestRecursive(
-        /* requestLock= */ new Object(),
+        requestLock,
         target,
         targetListener,
         /* parentCoordinator= */ null,

library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java

@@ -23,6 +23,7 @@
 import android.graphics.drawable.Drawable;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import com.bumptech.glide.GlideBuilder.FixSingleRequestClearDeadlock;
 import com.bumptech.glide.GlideContext;
 import com.bumptech.glide.GlideExperiments;
 import com.bumptech.glide.Priority;
@@ -1071,6 +1072,8 @@ static final class SingleRequestBuilder {
     private final Map<Class<?>, Transformation<?>> transformations = new HashMap<>();
 
     SingleRequestBuilder() {
+      GlideExperiments glideExperiments = mock(GlideExperiments.class);
+      when(glideExperiments.isEnabled(FixSingleRequestClearDeadlock.class)).thenReturn(true);
       when(glideContext.getExperiments()).thenReturn(mock(GlideExperiments.class));
       when(requestCoordinator.getRoot()).thenReturn(requestCoordinator);
       when(requestCoordinator.canSetImage(any(Request.class))).thenReturn(true);