Fixed BaseObj finalizer and destroy()

Bug: 28242626
Bug: 27972184
Bug: 27973681

This is resolving issues in ScriptGroup (V1) again.
In ScriptGroup.destroy(), we also need to consider the old API where
mClosures is not initialized.

Also cleaned up the finalizer for ScriptGroup and Allocation:
Since BaseObj.finalize() calls BaseObj.helpDestroy(), instead of
BaseObj.destroy(), there is no possibility that the finalizers of
child objects may race their parents finalizers. Note that
helpDestroy() does not try to recurse on child objects.

Change-Id: I9dbb2b60f8478f656f8a418c2b5fc8d6848aeef0

2 files changed, 5 insertions, 15 deletions

diff --git a/rs/java/android/renderscript/Allocation.java b/rs/java/android/renderscript/Allocation.java
index 04ea8e5a663f..c0ea1324602b 100644
--- a/rs/java/android/renderscript/Allocation.java
+++ b/rs/java/android/renderscript/Allocation.java
@@ -392,8 +392,6 @@ public class Allocation extends BaseObj {
 
     protected void finalize() throws Throwable {
         RenderScript.registerNativeFree.invoke(RenderScript.sRuntime, mSize);
-        // Set mType null to avoid double-destroying it in case its finalizer races ahead
-        mType = null;
         super.finalize();
     }
 
@@ -2615,7 +2613,6 @@ public class Allocation extends BaseObj {
 
         if (mType != null && mOwningType) {
             mType.destroy();
-            mType = null;
         }
 
         super.destroy();
diff --git a/rs/java/android/renderscript/ScriptGroup.java b/rs/java/android/renderscript/ScriptGroup.java
index 0d10c6d9dc50..e0bdbfcdfed5 100644
--- a/rs/java/android/renderscript/ScriptGroup.java
+++ b/rs/java/android/renderscript/ScriptGroup.java
@@ -1066,19 +1066,12 @@ public final class ScriptGroup extends BaseObj {
      */
     public void destroy() {
         super.destroy();
-        for(Closure c : mClosures) {
-            c.destroy();
-        }
-    }
-
-    protected void finalize() throws Throwable {
-        // Clear out the list mClosures to avoid double-destroying the closures,
-        // in case their finalizers race ahead.
+        // ScriptGroup created using the old Builder class does not
+        // initialize the field mClosures
         if (mClosures != null) {
-            // ScriptGroup created using the old Builder class does not
-            // initialize the field mClosures
-            mClosures.clear();
+            for (Closure c : mClosures) {
+                c.destroy();
+            }
         }
-        super.finalize();
     }
 }