Skip to content

Commit 70be029

Browse files
committed
Proof of concept for investigation of "Cannot recycle a resource while it is still acquired"
1 parent b1c6076 commit 70be029

File tree

2 files changed

+82
-3
lines changed

2 files changed

+82
-3
lines changed

library/src/main/java/com/bumptech/glide/load/engine/ResourceRecycler.java

+29-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ synchronized void recycle(Resource<?> resource, boolean forceNextFrame) {
1616
// If a resource has sub-resources, releasing a sub resource can cause it's parent to be
1717
// synchronously evicted which leads to a recycle loop when the parent releases it's children.
1818
// Posting breaks this loop.
19-
handler.obtainMessage(ResourceRecyclerCallback.RECYCLE_RESOURCE, resource).sendToTarget();
19+
handler.obtainMessage(ResourceRecyclerCallback.RECYCLE_RESOURCE, new RecycleTask(resource, new Throwable("Async stack trace"))).sendToTarget();
2020
} else {
2121
isRecycling = true;
2222
resource.recycle();
@@ -33,11 +33,37 @@ private static final class ResourceRecyclerCallback implements Handler.Callback
3333
@Override
3434
public boolean handleMessage(Message message) {
3535
if (message.what == RECYCLE_RESOURCE) {
36-
Resource<?> resource = (Resource<?>) message.obj;
37-
resource.recycle();
36+
RecycleTask task = (RecycleTask) message.obj;
37+
try {
38+
Resource<?> resource = task.resource;
39+
resource.recycle();
40+
} catch (RuntimeException | Error ex) {
41+
task.rethrow(ex);
42+
}
3843
return true;
3944
}
4045
return false;
4146
}
4247
}
48+
49+
private static final class RecycleTask {
50+
final Resource<?> resource;
51+
final Throwable stacktrace;
52+
53+
RecycleTask(Resource<?> resource, Throwable stacktrace) {
54+
this.resource = resource;
55+
this.stacktrace = stacktrace;
56+
}
57+
58+
void rethrow(Throwable original) {
59+
Throwable rootCause = original;
60+
while (rootCause.getCause() != null) {
61+
rootCause = rootCause.getCause();
62+
}
63+
rootCause.initCause(stacktrace);
64+
if (original instanceof Error) throw (Error) original;
65+
if (original instanceof RuntimeException) throw (RuntimeException) original;
66+
throw new IllegalStateException("Unknown exception: " + original, original);
67+
}
68+
}
4369
}

library/test/src/test/java/com/bumptech/glide/load/engine/ResourceRecyclerTest.java

+53
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,19 @@
22

33
import static com.bumptech.glide.RobolectricConstants.ROBOLECTRIC_SDK;
44
import static com.bumptech.glide.tests.Util.mockResource;
5+
import static com.google.common.truth.Truth.assertThat;
6+
import static org.junit.Assert.assertSame;
7+
import static org.junit.Assert.assertThrows;
58
import static org.mockito.Mockito.doAnswer;
9+
import static org.mockito.Mockito.doThrow;
610
import static org.mockito.Mockito.never;
711
import static org.mockito.Mockito.verify;
812

913
import android.os.Looper;
14+
import com.google.common.truth.Correspondence;
15+
import java.util.ArrayList;
16+
import java.util.Arrays;
17+
import java.util.List;
1018
import org.junit.Before;
1119
import org.junit.Test;
1220
import org.junit.runner.RunWith;
@@ -71,4 +79,49 @@ public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
7179

7280
verify(child).recycle();
7381
}
82+
83+
@Test
84+
public void recycle_withChild_asyncTraceable() {
85+
final Resource<?> child = mockResource();
86+
Throwable someCause = new Throwable("Some simulated cause.");
87+
IllegalStateException testEx = new IllegalStateException("Simulated error for test.", someCause);
88+
doThrow(testEx).when(child).recycle();
89+
Resource<?> parent = mockResource();
90+
class ChildRecycler implements Answer<Void> {
91+
@Override
92+
public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
93+
recycler.recycle(child, /* forceNextFrame= */ false);
94+
return null;
95+
}
96+
}
97+
doAnswer(new ChildRecycler()).when(parent).recycle();
98+
99+
Shadows.shadowOf(Looper.getMainLooper()).pause();
100+
recycler.recycle(parent, /* forceNextFrame= */ false);
101+
IllegalStateException ex = assertThrows(
102+
IllegalStateException.class,
103+
() -> Shadows.shadowOf(Looper.getMainLooper()).runOneTask()
104+
);
105+
assertSame("Original exception is thrown", testEx, ex);
106+
assertSame("Cause is kept", someCause, ex.getCause());
107+
assertThat(fullStackOf(ex))
108+
.comparingElementsUsing(className())
109+
.contains(ChildRecycler.class.getName());
110+
}
111+
112+
private static Correspondence<StackTraceElement, String> className() {
113+
return Correspondence.from(
114+
(actual, expected) -> actual.getClassName().equals(expected),
115+
"StackTraceElement.className"
116+
);
117+
}
118+
119+
private static List<StackTraceElement> fullStackOf(Throwable t) {
120+
List<StackTraceElement> stack = new ArrayList<>();
121+
do {
122+
stack.addAll(Arrays.asList(t.getStackTrace()));
123+
t = t.getCause();
124+
} while (t != null);
125+
return stack;
126+
}
74127
}

0 commit comments

Comments
 (0)