Skip to content

Commit

Permalink
Avoid deadlock between SingleRequest and RequestFutureTarget.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sjudd authored and glide-copybara-robot committed Aug 5, 2019
1 parent b2a46ef commit 01ea6a5
Showing 1 changed file with 35 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down

0 comments on commit 01ea6a5

Please sign in to comment.