Proper error checking for setting cards
When shrinking the cards array while invalidates are still
in flight, it's possible to have an invalidate or texture
set land on a nonexistent card, causing an out-of-bounds
exception to be thrown, crashing the app. Frequently these
invalidates come from the user dragging the carousel to
another position, so it's not feasible to prevent
invalidates from being sent when we're about to clear the
cards array; nor is it feasible to empty any pending
invalidates, since Renderscript does not provide any
meaningful inspection of the event queue. Therefore, the
only real way we can address the problem is by detecting
these out-of-turn invalidates and handling the errors
cleanly. In practice, since these events are typically to
invalidate a nonexistent card, it's OK to just drop them,
since each such card is already invalid because it does
not exist.
This fix factors out card-get and card-set logic into
common methods, similar to card-set-or-create logic that's
been present for a long time.
Bug: 3381300
Change-Id: I037801de3b4c3bff514b1586fa16417e32c118df
diff --git a/carousel/java/com/android/ex/carousel/CarouselRS.java b/carousel/java/com/android/ex/carousel/CarouselRS.java
index 0d921a7..4467fb3 100644
--- a/carousel/java/com/android/ex/carousel/CarouselRS.java
+++ b/carousel/java/com/android/ex/carousel/CarouselRS.java
@@ -672,21 +672,39 @@
return allocation;
}
- private ScriptField_Card.Item getOrCreateCard(int n) {
+ private ScriptField_Card.Item getCard(int n) {
ScriptField_Card.Item item;
try {
item = mCards.get(n);
}
catch (ArrayIndexOutOfBoundsException e) {
+ if (DBG) Log.v(TAG, "getCard(): no item at index " + n);
item = null;
}
+ return item;
+ }
+
+ private ScriptField_Card.Item getOrCreateCard(int n) {
+ ScriptField_Card.Item item = getCard(n);
if (item == null) {
- if (DBG) Log.v(TAG, "getOrCreateItem(): no item at index " + n);
+ if (DBG) Log.v(TAG, "getOrCreateCard(): no item at index " + n + "; creating new");
item = new ScriptField_Card.Item();
}
return item;
}
+ private void setCard(int n, ScriptField_Card.Item item) {
+ try {
+ mCards.set(item, n, false); // This is primarily used for reference counting.
+ }
+ catch (ArrayIndexOutOfBoundsException e) {
+ // The specified index didn't exist. This can happen when a stale invalidate
+ // request outlived an array resize request. Something might be getting dropped,
+ // but there's not much we can do about this at this point to recover.
+ Log.w(TAG, "setCard(" + n + "): Texture " + n + " doesn't exist");
+ }
+ }
+
public void setTexture(int n, Bitmap bitmap)
{
if (n < 0) throw new IllegalArgumentException("Index cannot be negative");
@@ -701,7 +719,7 @@
item.texture = null;
}
}
- mCards.set(item, n, false); // This is primarily used for reference counting.
+ setCard(n, item);
mScript.invoke_setTexture(n, item.texture);
}
}
@@ -727,7 +745,7 @@
item.detailTexture = null;
}
}
- mCards.set(item, n, false); // This is primarily used for reference counting.
+ setCard(n, item);
mScript.invoke_setDetailTexture(n, offx, offy, loffx, loffy, item.detailTexture);
}
}
@@ -737,7 +755,7 @@
if (n < 0) throw new IllegalArgumentException("Index cannot be negative");
synchronized(this) {
- ScriptField_Card.Item item = mCards.get(n);
+ ScriptField_Card.Item item = getCard(n);
if (item == null) {
// This card was never created, so there's nothing to invalidate.
return;
@@ -749,7 +767,7 @@
item.texture.destroy();
item.texture = null;
}
- mCards.set(item, n, false); // This is primarily used for reference counting.
+ setCard(n, item);
mScript.invoke_invalidateTexture(n, eraseCurrent);
}
}
@@ -759,7 +777,7 @@
if (n < 0) throw new IllegalArgumentException("Index cannot be negative");
synchronized(this) {
- ScriptField_Card.Item item = mCards.get(n);
+ ScriptField_Card.Item item = getCard(n);
if (item == null) {
// This card was never created, so there's nothing to invalidate.
return;
@@ -771,7 +789,7 @@
item.detailTexture.destroy();
item.detailTexture = null;
}
- mCards.set(item, n, false); // This is primarily used for reference counting.
+ setCard(n, item);
mScript.invoke_invalidateDetailTexture(n, eraseCurrent);
}
}
@@ -792,7 +810,7 @@
item.geometry = null;
}
}
- mCards.set(item, n, false);
+ setCard(n, item);
mScript.invoke_setGeometry(n, item.geometry);
}
}
@@ -809,7 +827,7 @@
if (DBG) Log.v(TAG, "unloading matrix " + n);
item.matrix = null;
}
- mCards.set(item, n, false);
+ setCard(n, item);
mScript.invoke_setMatrix(n, item.matrix);
}
}