From 2685de9862657b0e0ac3593aa3f351e1b42dc01e Mon Sep 17 00:00:00 2001
From: Hani Kazmi <hanikazmi@google.com>
Date: Wed, 8 Jun 2022 13:53:42 +0000
Subject: [PATCH] BaseBundle.java: Recycle underlying parcel when bundle is
 cleared.

Lazy Bundles, (aosp/1787847), introduced a change in behavior where a Parcel
created as part of initializing a Bundle is dependent on the next ART GC run to be
recycled, causing a short term memory-leak.

To land this in T, we are making the change targetted and allowing
consumers to opt into the parcel being immediately cleared by calling
.clear() on the bundle.

As part of the unparcel() in clear(), mParcelledData is set to null, and
mMap may or may not still contain references through lazy values,
depending on if the lazy valyes have been unmarshalled.  As
such, we keep a weak reference to mParcelledData we can use to recycle it.

The mParcelledData reference could have been copied to other bundles in
a few operations:
new Bundle(Bundle o)
bundle.deepCopy()
bundle.putAll()

In this case we can not recycle the parcel yet as other bundles may
still require it. If so, we will skip the recycle and rely on the later GC pass

Bug: 233216232
Test: Reproduced linked bug on-device
Test: atest android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest
Change-Id: Ic26eceaa1c11da67866af0963f760423d41d54bc
---
 core/java/android/os/BaseBundle.java | 26 +++++++++++++++++++++++++-
 core/java/android/os/Bundle.java     |  2 ++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java
index e5dab0539a8e6..0418a4bb9f806 100644
--- a/core/java/android/os/BaseBundle.java
+++ b/core/java/android/os/BaseBundle.java
@@ -31,6 +31,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.IndentingPrintWriter;
 
 import java.io.Serializable;
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Set;
 import java.util.function.BiFunction;
@@ -102,7 +103,7 @@ public class BaseBundle {
     /*
      * If mParcelledData is non-null, then mMap will be null and the
      * data are stored as a Parcel containing a Bundle.  When the data
-     * are unparcelled, mParcelledData willbe set to null.
+     * are unparcelled, mParcelledData will be set to null.
      */
     @UnsupportedAppUsage
     volatile Parcel mParcelledData = null;
@@ -112,6 +113,19 @@ public class BaseBundle {
      */
     private boolean mParcelledByNative;
 
+    /*
+     * Flag indicating if mParcelledData is only referenced in this bundle.
+     * mParcelledData could be referenced by other bundles if mMap contains lazy values,
+     * and bundle data is copied to another bundle using putAll or the copy constructors.
+     */
+    boolean mOwnsLazyValues = true;
+
+    /*
+     * As mParcelledData is set to null when it is unparcelled, we keep a weak reference to
+     * it to aid in recycling it. Do not use this reference otherwise.
+     */
+    private WeakReference<Parcel> mWeakParcelledData = null;
+
     /**
      * The ClassLoader used when unparcelling data from mParcelledData.
      */
@@ -200,6 +214,9 @@ public class BaseBundle {
             mClassLoader = from.mClassLoader;
 
             if (from.mMap != null) {
+                mOwnsLazyValues = false;
+                from.mOwnsLazyValues = false;
+
                 if (!deep) {
                     mMap = new ArrayMap<>(from.mMap);
                 } else {
@@ -434,6 +451,9 @@ public class BaseBundle {
             mMap = map;
             if (recycleParcel) {
                 recycleParcel(parcelledData);
+                mWeakParcelledData = null;
+            } else {
+                mWeakParcelledData = new WeakReference<>(parcelledData);
             }
             mParcelledByNative = false;
             mParcelledData = null;
@@ -575,6 +595,10 @@ public class BaseBundle {
      */
     public void clear() {
         unparcel();
+        if (mOwnsLazyValues && mWeakParcelledData != null) {
+            recycleParcel(mWeakParcelledData.get());
+            mWeakParcelledData = null;
+        }
         mMap.clear();
     }
 
diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java
index cf28c1639fac2..7e355d95cdb33 100644
--- a/core/java/android/os/Bundle.java
+++ b/core/java/android/os/Bundle.java
@@ -301,6 +301,8 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
     public void putAll(Bundle bundle) {
         unparcel();
         bundle.unparcel();
+        mOwnsLazyValues = false;
+        bundle.mOwnsLazyValues = false;
         mMap.putAll(bundle.mMap);
 
         // FD state is now known if and only if both bundles already knew