From eb4dd08ec132f83745b8b28fa7da58eb4478b5b9 Mon Sep 17 00:00:00 2001
From: Yang Ni <yangni@google.com>
Date: Thu, 24 Mar 2016 09:40:32 -0700
Subject: [PATCH] Added CloseGuard for BaseObj

Bug: 27719830

To turn on warnings, apps have to add to their Activity.onCreate() method
the following code.

        StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder()
                               .detectLeakedClosableObjects()
                               .penaltyLog()
                               .build());

For Slang generated ScriptC derived classes, we assume their
constructors won't throw exceptions after calling the ScriptC
constructor. In addition, ScriptIntrinsic derived classes do not seem
to throw exceptions in their constructors either. Therefore, we can
leave the guard.open() call in the Script constructor. This may be
only an approximation, but allows us to add CloseGuard for script
objects without making changes to slang.

Change-Id: I77ed45239a60b85af5c811dee6c124fb53da9060
---
 rs/java/android/renderscript/Allocation.java  |  2 ++
 rs/java/android/renderscript/BaseObj.java     | 13 +++++++++++--
 rs/java/android/renderscript/Element.java     |  2 ++
 rs/java/android/renderscript/FileA3D.java     |  1 +
 rs/java/android/renderscript/Font.java        |  1 +
 rs/java/android/renderscript/Mesh.java        |  1 +
 rs/java/android/renderscript/Program.java     |  1 +
 rs/java/android/renderscript/Sampler.java     |  1 +
 rs/java/android/renderscript/Script.java      | 15 +++++++++++++++
 rs/java/android/renderscript/ScriptGroup.java |  6 ++++++
 rs/java/android/renderscript/Type.java        |  1 +
 11 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/rs/java/android/renderscript/Allocation.java b/rs/java/android/renderscript/Allocation.java
index 6bcb5b6afc72f..37c1c7a59250d 100644
--- a/rs/java/android/renderscript/Allocation.java
+++ b/rs/java/android/renderscript/Allocation.java
@@ -372,6 +372,7 @@ public class Allocation extends BaseObj {
             Log.e(RenderScript.LOG_TAG, "Couldn't invoke registerNativeAllocation:" + e);
             throw new RSRuntimeException("Couldn't invoke registerNativeAllocation:" + e);
         }
+        guard.open("destroy");
     }
 
     Allocation(long id, RenderScript rs, Type t, int usage, MipmapControl mips) {
@@ -1907,6 +1908,7 @@ public class Allocation extends BaseObj {
             if (type.getID(rs) == 0) {
                 throw new RSInvalidStateException("Bad Type");
             }
+            // TODO: What if there is an exception after this? The native allocation would leak.
             long id = rs.nAllocationCreateTyped(type.getID(rs), mips.mID, usage, 0);
             if (id == 0) {
                 throw new RSRuntimeException("Allocation creation failed.");
diff --git a/rs/java/android/renderscript/BaseObj.java b/rs/java/android/renderscript/BaseObj.java
index 1372ab79e2642..f95af1673730e 100644
--- a/rs/java/android/renderscript/BaseObj.java
+++ b/rs/java/android/renderscript/BaseObj.java
@@ -16,6 +16,7 @@
 
 package android.renderscript;
 
+import dalvik.system.CloseGuard;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
@@ -69,6 +70,7 @@ public class BaseObj {
     }
 
     private long mID;
+    final CloseGuard guard = CloseGuard.get();
     private boolean mDestroyed;
     private String mName;
     RenderScript mRS;
@@ -119,6 +121,7 @@ public class BaseObj {
         }
 
         if (shouldDestroy) {
+            guard.close();
             // must include nObjDestroy in the critical section
             ReentrantReadWriteLock.ReadLock rlock = mRS.mRWLock.readLock();
             rlock.lock();
@@ -133,8 +136,14 @@ public class BaseObj {
     }
 
     protected void finalize() throws Throwable {
-        helpDestroy();
-        super.finalize();
+        try {
+            if (guard != null) {
+                guard.warnIfOpen();
+            }
+            helpDestroy();
+        } finally {
+            super.finalize();
+        }
     }
 
     /**
diff --git a/rs/java/android/renderscript/Element.java b/rs/java/android/renderscript/Element.java
index 6efb6d6a5735e..50226acc77b4f 100644
--- a/rs/java/android/renderscript/Element.java
+++ b/rs/java/android/renderscript/Element.java
@@ -808,6 +808,7 @@ public class Element extends BaseObj {
             mSize += mElements[ct].mSize * mArraySizes[ct];
         }
         updateVisibleSubElements();
+        guard.open("destroy");
     }
 
     Element(long id, RenderScript rs, DataType dt, DataKind dk, boolean norm, int size) {
@@ -827,6 +828,7 @@ public class Element extends BaseObj {
         mKind = dk;
         mNormalized = norm;
         mVectorSize = size;
+        guard.open("destroy");
     }
 
     Element(long id, RenderScript rs) {
diff --git a/rs/java/android/renderscript/FileA3D.java b/rs/java/android/renderscript/FileA3D.java
index 9d8f1624a0519..278d309c74f7c 100644
--- a/rs/java/android/renderscript/FileA3D.java
+++ b/rs/java/android/renderscript/FileA3D.java
@@ -170,6 +170,7 @@ public class FileA3D extends BaseObj {
     FileA3D(long id, RenderScript rs, InputStream stream) {
         super(id, rs);
         mInputStream = stream;
+        guard.open("destroy");
     }
 
     private void initEntries() {
diff --git a/rs/java/android/renderscript/Font.java b/rs/java/android/renderscript/Font.java
index 4318b9d4d21cc..d5ca31e934187 100644
--- a/rs/java/android/renderscript/Font.java
+++ b/rs/java/android/renderscript/Font.java
@@ -150,6 +150,7 @@ public class Font extends BaseObj {
 
     Font(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/Mesh.java b/rs/java/android/renderscript/Mesh.java
index 13c8e1c91052c..9e4f90573ae98 100644
--- a/rs/java/android/renderscript/Mesh.java
+++ b/rs/java/android/renderscript/Mesh.java
@@ -91,6 +91,7 @@ public class Mesh extends BaseObj {
 
     Mesh(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/Program.java b/rs/java/android/renderscript/Program.java
index 3eb9b7590f45f..772021c7815c4 100644
--- a/rs/java/android/renderscript/Program.java
+++ b/rs/java/android/renderscript/Program.java
@@ -76,6 +76,7 @@ public class Program extends BaseObj {
 
     Program(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/Sampler.java b/rs/java/android/renderscript/Sampler.java
index a4edbb50ac109..5c4bae99ba8fd 100644
--- a/rs/java/android/renderscript/Sampler.java
+++ b/rs/java/android/renderscript/Sampler.java
@@ -51,6 +51,7 @@ public class Sampler extends BaseObj {
 
     Sampler(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/Script.java b/rs/java/android/renderscript/Script.java
index 84f980dc24cde..5b009a6c44fba 100644
--- a/rs/java/android/renderscript/Script.java
+++ b/rs/java/android/renderscript/Script.java
@@ -41,6 +41,7 @@ public class Script extends BaseObj {
             mScript = s;
             mSlot = slot;
             mSig = sig;
+            guard.open("destroy");
         }
     }
 
@@ -118,6 +119,7 @@ public class Script extends BaseObj {
             super(id, rs);
             mScript = s;
             mSlot = slot;
+            guard.open("destroy");
         }
     }
 
@@ -358,6 +360,19 @@ public class Script extends BaseObj {
         super(id, rs);
 
         mInIdsBuffer = new long[1];
+
+        /* The constructors for the derived classes (including ScriptIntrinsic
+         * derived classes and ScriptC derived classes generated by Slang
+         * reflection) seem to be simple enough, so we just put the guard.open()
+         * call here, rather than in the end of the constructor for the derived
+         * class. This, of course, assumes the derived constructor would not
+         * throw any exception after calling this constructor.
+         *
+         * If new derived classes are added with more complicated constructors
+         * that throw exceptions, this call has to be (duplicated and) moved
+         * to the end of each derived class constructor.
+         */
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/ScriptGroup.java b/rs/java/android/renderscript/ScriptGroup.java
index 9357c3bb0428d..219f16b91baf7 100644
--- a/rs/java/android/renderscript/ScriptGroup.java
+++ b/rs/java/android/renderscript/ScriptGroup.java
@@ -148,6 +148,8 @@ public final class ScriptGroup extends BaseObj {
                                         fieldIDs, values, sizes, depClosures, depFieldIDs);
 
             setID(id);
+
+            guard.open("destroy");
         }
 
         Closure(RenderScript rs, Script.InvokeID invokeID,
@@ -181,6 +183,8 @@ public final class ScriptGroup extends BaseObj {
                                               values, sizes);
 
             setID(id);
+
+            guard.open("destroy");
         }
 
         private void retrieveValueAndDependenceInfo(RenderScript rs,
@@ -382,6 +386,7 @@ public final class ScriptGroup extends BaseObj {
 
     ScriptGroup(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     ScriptGroup(RenderScript rs, String name, List<Closure> closures,
@@ -398,6 +403,7 @@ public final class ScriptGroup extends BaseObj {
         }
         long id = rs.nScriptGroup2Create(name, RenderScript.getCachePath(), closureIDs);
         setID(id);
+        guard.open("destroy");
     }
 
     /**
diff --git a/rs/java/android/renderscript/Type.java b/rs/java/android/renderscript/Type.java
index dc2378596d002..9252898781f46 100644
--- a/rs/java/android/renderscript/Type.java
+++ b/rs/java/android/renderscript/Type.java
@@ -227,6 +227,7 @@ public class Type extends BaseObj {
 
     Type(long id, RenderScript rs) {
         super(id, rs);
+        guard.open("destroy");
     }
 
     @Override