Proof of concept for investigation of "Cannot recycle a resource whil…

…e it is still acquired"
bumptech · Oct 12, 2023 · 70be029 · 70be029
1 parent b1c6076
commit 70be029
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 3 deletions.
diff --git a/library/src/main/java/com/bumptech/glide/load/engine/ResourceRecycler.java b/library/src/main/java/com/bumptech/glide/load/engine/ResourceRecycler.java
@@ -16,7 +16,7 @@ synchronized void recycle(Resource<?> resource, boolean forceNextFrame) {
       // If a resource has sub-resources, releasing a sub resource can cause it's parent to be
       // synchronously evicted which leads to a recycle loop when the parent releases it's children.
       // Posting breaks this loop.
-      handler.obtainMessage(ResourceRecyclerCallback.RECYCLE_RESOURCE, resource).sendToTarget();
+      handler.obtainMessage(ResourceRecyclerCallback.RECYCLE_RESOURCE, new RecycleTask(resource, new Throwable("Async stack trace"))).sendToTarget();
     } else {
       isRecycling = true;
       resource.recycle();
@@ -33,11 +33,37 @@ private static final class ResourceRecyclerCallback implements Handler.Callback
     @Override
     public boolean handleMessage(Message message) {
       if (message.what == RECYCLE_RESOURCE) {
-        Resource<?> resource = (Resource<?>) message.obj;
-        resource.recycle();
+        RecycleTask task = (RecycleTask) message.obj;
+        try {
+          Resource<?> resource = task.resource;
+          resource.recycle();
+        } catch (RuntimeException | Error ex) {
+          task.rethrow(ex);
+        }
         return true;
       }
       return false;
     }
   }
+
+  private static final class RecycleTask {
+    final Resource<?> resource;
+    final Throwable stacktrace;
+
+    RecycleTask(Resource<?> resource, Throwable stacktrace) {
+      this.resource = resource;
+      this.stacktrace = stacktrace;
+    }
+
+    void rethrow(Throwable original) {
+      Throwable rootCause = original;
+      while (rootCause.getCause() != null) {
+        rootCause = rootCause.getCause();
+      }
+      rootCause.initCause(stacktrace);
+      if (original instanceof Error) throw (Error) original;
+      if (original instanceof RuntimeException) throw (RuntimeException) original;
+      throw new IllegalStateException("Unknown exception: " + original, original);
+    }
+  }
 }
diff --git a/library/test/src/test/java/com/bumptech/glide/load/engine/ResourceRecyclerTest.java b/library/test/src/test/java/com/bumptech/glide/load/engine/ResourceRecyclerTest.java
@@ -2,11 +2,19 @@
 
 import static com.bumptech.glide.RobolectricConstants.ROBOLECTRIC_SDK;
 import static com.bumptech.glide.tests.Util.mockResource;
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
 import android.os.Looper;
+import com.google.common.truth.Correspondence;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -71,4 +79,49 @@ public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
 
     verify(child).recycle();
   }
+
+  @Test
+  public void recycle_withChild_asyncTraceable() {
+    final Resource<?> child = mockResource();
+    Throwable someCause = new Throwable("Some simulated cause.");
+    IllegalStateException testEx = new IllegalStateException("Simulated error for test.", someCause);
+    doThrow(testEx).when(child).recycle();
+    Resource<?> parent = mockResource();
+    class ChildRecycler implements Answer<Void> {
+      @Override
+      public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
+        recycler.recycle(child, /* forceNextFrame= */ false);
+        return null;
+      }
+    }
+    doAnswer(new ChildRecycler()).when(parent).recycle();
+
+    Shadows.shadowOf(Looper.getMainLooper()).pause();
+    recycler.recycle(parent, /* forceNextFrame= */ false);
+    IllegalStateException ex = assertThrows(
+        IllegalStateException.class,
+        () -> Shadows.shadowOf(Looper.getMainLooper()).runOneTask()
+    );
+    assertSame("Original exception is thrown", testEx, ex);
+    assertSame("Cause is kept", someCause, ex.getCause());
+    assertThat(fullStackOf(ex))
+        .comparingElementsUsing(className())
+        .contains(ChildRecycler.class.getName());
+  }
+
+  private static Correspondence<StackTraceElement, String> className() {
+    return Correspondence.from(
+        (actual, expected) -> actual.getClassName().equals(expected),
+        "StackTraceElement.className"
+    );
+  }
+
+  private static List<StackTraceElement> fullStackOf(Throwable t) {
+    List<StackTraceElement> stack = new ArrayList<>();
+    do {
+      stack.addAll(Arrays.asList(t.getStackTrace()));
+      t = t.getCause();
+    } while (t != null);
+    return stack;
+  }
 }