Added locking for FilteringPipeline and ImageFilterRS. Gets rid of leaked threads.

Bug: 8373600
Bug: 8363001
Bug: 8344345
Bug: 8264659

This CL fixes several things:
- FilteringPipeline no longer leaks a handler thread every time photoeditor is reopened.
- ImageFilterRS no longer leaks an RS context and thread every time photoeditor is reopened.
- ImageFilterRS now uses locking.
- ImageFilterRS no longer leaks Bitmap references or static renderscript allocations.

Change-Id: I79ebde1a8ba8ad689791c2af8db0c3c907e06399

4 files changed, 90 insertions, 74 deletions

diff --git a/src/com/android/gallery3d/filtershow/FilterShowActivity.java b/src/com/android/gallery3d/filtershow/FilterShowActivity.java
index 93bb02483..37b2cd9da 100644
--- a/src/com/android/gallery3d/filtershow/FilterShowActivity.java
+++ b/src/com/android/gallery3d/filtershow/FilterShowActivity.java
@@ -574,6 +574,7 @@ public class FilterShowActivity extends Activity implements OnItemClickListener,
         // TODO:  Using singletons is a bad design choice for many of these
         // due static reference leaks and in general.  Please refactor.
         MasterImage.reset();
+        ImageFilterRS.destroyRenderScriptContext();
         FilteringPipeline.reset();
         ImageFilter.resetStatics();
         FiltersManager.reset();
diff --git a/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java b/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java
index 4aeb580e6..d7e9a62a7 100644
--- a/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java
+++ b/src/com/android/gallery3d/filtershow/cache/FilteringPipeline.java
@@ -30,7 +30,7 @@ import com.android.gallery3d.filtershow.presets.ImagePreset;
 
 public class FilteringPipeline implements Handler.Callback {
 
-    private static FilteringPipeline sPipeline;
+    private static volatile FilteringPipeline sPipeline = null;
     private static final String LOGTAG = "FilteringPipeline";
     private ImagePreset mPreviousGeometryPreset = null;
     private ImagePreset mPreviousFiltersPreset = null;
@@ -120,7 +120,7 @@ public class FilteringPipeline implements Handler.Callback {
         mProcessingHandler = new Handler(mHandlerThread.getLooper(), this);
     }
 
-    public static FilteringPipeline getPipeline() {
+    public synchronized static FilteringPipeline getPipeline() {
         if (sPipeline == null) {
             sPipeline = new FilteringPipeline();
         }
@@ -173,8 +173,6 @@ public class FilteringPipeline implements Handler.Callback {
                 Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT);
 
         mPreviousGeometry = new GeometryMetadata(geometry);
-
-        FiltersManager.getManager().resetBitmapsRS();
         return true;
     }
 
@@ -259,17 +257,19 @@ public class FilteringPipeline implements Handler.Callback {
         preset.setupEnvironment();
 
         if (request.getType() == RenderingRequest.PARTIAL_RENDERING) {
-            bitmap = MasterImage.getImage().getImageLoader().getScaleOneImageForPreset(null, preset,
+            ImageLoader loader = MasterImage.getImage().getImageLoader();
+            if (loader == null) {
+                Log.w(LOGTAG, "loader not yet setup, cannot handle: " + getType(request));
+                return;
+            }
+            bitmap = loader.getScaleOneImageForPreset(null, preset,
                     request.getBounds(), request.getDestination(), false);
             if (bitmap == null) {
+                Log.w(LOGTAG, "could not get bitmap for: " + getType(request));
                 return;
             }
         }
 
-        if (request.getType() == RenderingRequest.FILTERS_RENDERING) {
-            FiltersManager.getManager().resetBitmapsRS();
-        }
-
         if (request.getType() != RenderingRequest.ICON_RENDERING
                 && request.getType() != RenderingRequest.PARTIAL_RENDERING) {
             updateOriginalAllocation(preset);
@@ -296,9 +296,6 @@ public class FilteringPipeline implements Handler.Callback {
             FiltersManager.getManager().freeFilterResources(preset);
         }
 
-        if (request.getType() == RenderingRequest.FILTERS_RENDERING) {
-            FiltersManager.getManager().resetBitmapsRS();
-        }
     }
 
     private void compute(TripleBufferBitmap buffer, ImagePreset preset, int type) {
@@ -356,7 +353,8 @@ public class FilteringPipeline implements Handler.Callback {
         return mPreviewScaleFactor;
     }
 
-    public static void reset() {
+    public static synchronized void reset() {
+        sPipeline.mHandlerThread.quit();
         sPipeline = null;
     }
 }
diff --git a/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java b/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java
index 215d5d438..f6c3bdd89 100644
--- a/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java
+++ b/src/com/android/gallery3d/filtershow/filters/BaseFiltersManager.java
@@ -61,16 +61,6 @@ public abstract class BaseFiltersManager {
         return null;
     }
 
-    public void resetBitmapsRS() {
-        for (Class c : mFilters.keySet()) {
-            ImageFilter filter = mFilters.get(c);
-            if (filter instanceof ImageFilterRS) {
-                ImageFilterRS filterRS = (ImageFilterRS) filter;
-                filterRS.resetBitmap();
-            }
-        }
-    }
-
     public void freeFilterResources(ImagePreset preset) {
         if (preset == null) {
             return;
diff --git a/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java b/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java
index 978bc8bd1..595aa9b30 100644
--- a/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java
+++ b/src/com/android/gallery3d/filtershow/filters/ImageFilterRS.java
@@ -21,72 +21,76 @@ import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.support.v8.renderscript.*;
 import android.util.Log;
+import android.content.res.Resources;
 import com.android.gallery3d.R;
 
 public abstract class ImageFilterRS extends ImageFilter {
-    private final String LOGTAG = "ImageFilterRS";
+    private static final String LOGTAG = "ImageFilterRS";
 
-    private static RenderScript mRS = null;
-    protected static Allocation mInPixelsAllocation;
-    protected static Allocation mOutPixelsAllocation;
-    private static android.content.res.Resources mResources = null;
-    private static Bitmap sOldBitmap = null;
-    private Bitmap mOldBitmap = null;
+    protected static volatile Allocation mInPixelsAllocation;
+    protected static volatile Allocation mOutPixelsAllocation;
 
-    private boolean mResourcesLoaded = false;
+    private static volatile RenderScript sRS = null;
+    private static volatile int sWidth = 0;
+    private static volatile int sHeight = 0;
 
-    private final Bitmap.Config mBitmapConfig = Bitmap.Config.ARGB_8888;
+    private static volatile Resources sResources = null;
+    private boolean mResourcesLoaded = false;
 
-    public void resetBitmap() {
-        mOldBitmap = null;
-    }
+    private static final Bitmap.Config BITMAP_CONFIG = Bitmap.Config.ARGB_8888;
 
+    // This must be used inside block synchronized on ImageFilterRS class object
     public void prepare(Bitmap bitmap, float scaleFactor, int quality) {
-        if (sOldBitmap == null
-                || (bitmap.getWidth() != sOldBitmap.getWidth())
-                || (bitmap.getHeight() != sOldBitmap.getHeight())) {
-            if (mInPixelsAllocation != null) {
-                mInPixelsAllocation.destroy();
-                mInPixelsAllocation = null;
+        if (mOutPixelsAllocation == null || mInPixelsAllocation == null ||
+                bitmap.getWidth() != sWidth || bitmap.getHeight() != sHeight) {
+            destroyPixelAllocations();
+            Bitmap bitmapBuffer = bitmap;
+            if (bitmap.getConfig() == null || bitmap.getConfig() != BITMAP_CONFIG) {
+                bitmapBuffer = bitmap.copy(BITMAP_CONFIG, true);
             }
-            if (mOutPixelsAllocation != null) {
-                mOutPixelsAllocation.destroy();
-                mOutPixelsAllocation = null;
-            }
-            Bitmap bitmapBuffer = bitmap.copy(mBitmapConfig, true);
-            mOutPixelsAllocation = Allocation.createFromBitmap(mRS, bitmapBuffer,
+            mOutPixelsAllocation = Allocation.createFromBitmap(sRS, bitmapBuffer,
                     Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT);
-            mInPixelsAllocation = Allocation.createTyped(mRS,
+            mInPixelsAllocation = Allocation.createTyped(sRS,
                     mOutPixelsAllocation.getType());
-            sOldBitmap = bitmap;
         }
         mInPixelsAllocation.copyFrom(bitmap);
-        if (mOldBitmap != sOldBitmap || !isResourcesLoaded()) {
+        if (bitmap.getWidth() != sWidth
+            || bitmap.getHeight() != sHeight || !isResourcesLoaded()) {
             freeResources();
-            createFilter(mResources, scaleFactor, quality);
-            mOldBitmap = sOldBitmap;
+            createFilter(sResources, scaleFactor, quality);
+            sWidth = bitmap.getWidth();
+            sHeight = bitmap.getHeight();
             setResourcesLoaded(true);
         }
     }
 
+    // This must be used inside block synchronized on ImageFilterRS class object
     abstract public void createFilter(android.content.res.Resources res,
             float scaleFactor, int quality);
 
+    // This must be used inside block synchronized on ImageFilterRS class object
     abstract public void runFilter();
 
+    // This must be used inside block synchronized on ImageFilterRS class object
     public void update(Bitmap bitmap) {
         mOutPixelsAllocation.copyTo(bitmap);
     }
 
     @Override
     public Bitmap apply(Bitmap bitmap, float scaleFactor, int quality) {
-        if (bitmap == null) {
+        if (bitmap == null || bitmap.getWidth() == 0 || bitmap.getHeight() == 0) {
             return bitmap;
         }
         try {
-            prepare(bitmap, scaleFactor, quality);
-            runFilter();
-            update(bitmap);
+            synchronized(ImageFilterRS.class) {
+                if (sRS == null)  {
+                    Log.w(LOGTAG, "Cannot apply before calling setRenderScriptContext");
+                    return bitmap;
+                }
+                prepare(bitmap, scaleFactor, quality);
+                runFilter();
+                update(bitmap);
+            }
         } catch (android.renderscript.RSIllegalArgumentException e) {
             Log.e(LOGTAG, "Illegal argument? " + e);
         } catch (android.renderscript.RSRuntimeException e) {
@@ -100,15 +104,21 @@ public abstract class ImageFilterRS extends ImageFilter {
         return bitmap;
     }
 
-    public static RenderScript getRenderScriptContext() {
-        return mRS;
+    public static synchronized RenderScript getRenderScriptContext() {
+        return sRS;
     }
 
-    public static void setRenderScriptContext(Activity context) {
-        if (mRS == null) {
-            mRS = RenderScript.create(context);
+    public static synchronized void setRenderScriptContext(Activity context) {
+        if( sRS != null) {
+            Log.w(LOGTAG, "A prior RS context exists when calling setRenderScriptContext");
+            destroyRenderScriptContext();
         }
-        mResources = context.getResources();
+        sRS = RenderScript.create(context);
+        sResources = context.getResources();
+        destroyPixelAllocations();
+    }
+
+    private static synchronized void destroyPixelAllocations() {
         if (mInPixelsAllocation != null) {
             mInPixelsAllocation.destroy();
             mInPixelsAllocation = null;
@@ -117,36 +127,49 @@ public abstract class ImageFilterRS extends ImageFilter {
             mOutPixelsAllocation.destroy();
             mOutPixelsAllocation = null;
         }
-        sOldBitmap = null;
+        sWidth = 0;
+        sHeight = 0;
+    }
+
+    public static synchronized void destroyRenderScriptContext() {
+        destroyPixelAllocations();
+        sRS.destroy();
+        sRS = null;
+        sResources = null;
     }
 
-    private Allocation convertBitmap(Bitmap bitmap) {
-        return Allocation.createFromBitmap(mRS, bitmap,
+    private static synchronized Allocation convertBitmap(Bitmap bitmap) {
+        return Allocation.createFromBitmap(sRS, bitmap,
                 Allocation.MipmapControl.MIPMAP_NONE, Allocation.USAGE_SCRIPT);
     }
 
-    private Allocation convertRGBAtoA(Bitmap bitmap) {
-        Type.Builder tb_a8 = new Type.Builder(mRS, Element.U8(mRS));
-        ScriptC_grey greyConvert = new ScriptC_grey(mRS, mResources, R.raw.grey);
+    private static synchronized Allocation convertRGBAtoA(Bitmap bitmap) {
+        Type.Builder tb_a8 = new Type.Builder(sRS, Element.U8(sRS));
+        ScriptC_grey greyConvert = new ScriptC_grey(sRS,
+                sRS.getApplicationContext().getResources(), R.raw.grey);
 
         Allocation bitmapTemp = convertBitmap(bitmap);
-        if (bitmapTemp.getType().getElement().isCompatible(Element.U8(mRS))) {
+        if (bitmapTemp.getType().getElement().isCompatible(Element.U8(sRS))) {
             return bitmapTemp;
         }
 
         tb_a8.setX(bitmapTemp.getType().getX());
         tb_a8.setY(bitmapTemp.getType().getY());
-        Allocation bitmapAlloc = Allocation.createTyped(mRS, tb_a8.create());
+        Allocation bitmapAlloc = Allocation.createTyped(sRS, tb_a8.create());
         greyConvert.forEach_RGBAtoA(bitmapTemp, bitmapAlloc);
 
         return bitmapAlloc;
     }
 
     public Allocation loadResourceAlpha(int resource) {
+        Resources res = null;
+        synchronized(ImageFilterRS.class) {
+            res = sRS.getApplicationContext().getResources();
+        }
         final BitmapFactory.Options options = new BitmapFactory.Options();
         options.inPreferredConfig = Bitmap.Config.ALPHA_8;
         Bitmap bitmap = BitmapFactory.decodeResource(
-                mRS.getApplicationContext().getResources(),
+                res,
                 resource, options);
         Allocation ret = convertRGBAtoA(bitmap);
         bitmap.recycle();
@@ -154,21 +177,25 @@ public abstract class ImageFilterRS extends ImageFilter {
     }
 
     public Allocation loadResource(int resource) {
+        Resources res = null;
+        synchronized(ImageFilterRS.class) {
+            res = sRS.getApplicationContext().getResources();
+        }
         final BitmapFactory.Options options = new BitmapFactory.Options();
         options.inPreferredConfig = Bitmap.Config.ARGB_8888;
         Bitmap bitmap = BitmapFactory.decodeResource(
-                mRS.getApplicationContext().getResources(),
+                res,
                 resource, options);
         Allocation ret = convertBitmap(bitmap);
         bitmap.recycle();
         return ret;
     }
 
-    public boolean isResourcesLoaded() {
+    private boolean isResourcesLoaded() {
         return mResourcesLoaded;
     }
 
-    public void setResourcesLoaded(boolean resourcesLoaded) {
+    private void setResourcesLoaded(boolean resourcesLoaded) {
         mResourcesLoaded = resourcesLoaded;
     }