From 38f3678fe9ee5e050af1587a525dadc40f526a51 Mon Sep 17 00:00:00 2001
From: Chris Craik <ccraik@google.com>
Date: Thu, 23 Apr 2015 14:20:58 -0700
Subject: Fix race in bitmap decode vs release.

Now, if the decode is still running at destroy time, have
the decode runnable/thread manage the cleanup.

Change-Id: Ifcebf9f00417275229087d6c36989519d5f73aed
---
 .../support/rastermill/FrameSequenceDrawable.java  | 59 ++++++++++++++--------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java b/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
index af25c82..2228b1d 100644
--- a/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
+++ b/framesequence/src/android/support/rastermill/FrameSequenceDrawable.java
@@ -161,13 +161,26 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
             int lastFrame = nextFrame - 2;
             long invalidateTimeMs = mFrameSequenceState.getFrame(nextFrame, bitmap, lastFrame);
 
+            boolean schedule = false;
+            Bitmap bitmapToRelease = null;
             synchronized (mLock) {
-                if (mNextFrameToDecode < 0 || mState != STATE_DECODING) return;
-                mNextSwap = invalidateTimeMs + mLastSwap;
-
-                mState = STATE_WAITING_TO_SWAP;
+                if (mDestroyed) {
+                    bitmapToRelease = mBackBitmap;
+                    mBackBitmap = null;
+                } else if (mNextFrameToDecode >= 0 && mState == STATE_DECODING) {
+                    schedule = true;
+                    mNextSwap = invalidateTimeMs + mLastSwap;
+                    mState = STATE_WAITING_TO_SWAP;
+                }
+            }
+            if (schedule) {
+                scheduleSelf(FrameSequenceDrawable.this, mNextSwap);
+            }
+            if (bitmapToRelease != null) {
+                // destroy the bitmap here, since there's no safe way to get back to
+                // drawable thread - drawable is likely detached, so schedule is noop.
+                mBitmapProvider.releaseBitmap(bitmapToRelease);
             }
-            scheduleSelf(FrameSequenceDrawable.this, mNextSwap);
         }
     };
 
@@ -238,30 +251,31 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
      * If no BitmapProvider is attached to the drawable, recycle() is called on the Bitmaps.
      */
     public void destroy() {
-        destroy(mBitmapProvider);
-    }
-
-    private void destroy(BitmapProvider bitmapProvider) {
-        if (bitmapProvider == null) {
+        if (mBitmapProvider == null) {
             throw new IllegalStateException("BitmapProvider must be non-null");
         }
 
         Bitmap bitmapToReleaseA;
-        Bitmap bitmapToReleaseB;
+        Bitmap bitmapToReleaseB = null;
         synchronized (mLock) {
             checkDestroyedLocked();
 
             bitmapToReleaseA = mFrontBitmap;
-            bitmapToReleaseB = mBackBitmap;
-
             mFrontBitmap = null;
-            mBackBitmap = null;
+
+            if (mState != STATE_DECODING) {
+                bitmapToReleaseB = mBackBitmap;
+                mBackBitmap = null;
+            }
+
             mDestroyed = true;
         }
 
         // For simplicity and safety, we don't destroy the state object here
-        bitmapProvider.releaseBitmap(bitmapToReleaseA);
-        bitmapProvider.releaseBitmap(bitmapToReleaseB);
+        mBitmapProvider.releaseBitmap(bitmapToReleaseA);
+        if (bitmapToReleaseB != null) {
+            mBitmapProvider.releaseBitmap(bitmapToReleaseB);
+        }
     }
 
     @Override
@@ -322,12 +336,17 @@ public class FrameSequenceDrawable extends Drawable implements Animatable, Runna
 
     @Override
     public void run() {
-        // set ready to swap
+        // set ready to swap as necessary
+        boolean invalidate = false;
         synchronized (mLock) {
-            if (mState != STATE_WAITING_TO_SWAP || mNextFrameToDecode < 0) return;
-            mState = STATE_READY_TO_SWAP;
+            if (mNextFrameToDecode >= 0 && mState == STATE_WAITING_TO_SWAP) {
+                mState = STATE_READY_TO_SWAP;
+                invalidate = true;
+            }
+        }
+        if (invalidate) {
+            invalidateSelf();
         }
-        invalidateSelf();
     }
 
     @Override
-- 
cgit v1.2.3