Merge "BaseBundle.java:: Clear underlying parcel when no longer referernced." (6ee787a8) · Commits · e / os / android_frameworks_base

core/java/android/os/BaseBundle.java

+46 −17

Original line number	Diff line number	Diff line
		@@ -29,6 +29,7 @@ import android.util.SparseArray;

		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.util.IndentingPrintWriter;
		import com.android.internal.util.Preconditions;

		import java.io.Serializable;
		import java.lang.ref.WeakReference;
		@@ -113,16 +114,20 @@ 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,
		* mParcelledData could be referenced elsewhere if mMap contains lazy values,
		* and bundle data is copied to another bundle using putAll or the copy constructors.
		*/
		boolean mOwnsLazyValues = true;

		/*
		/** Tracks how many lazy values are referenced in mMap */
		private int mLazyValues = 0;

		/**
		* 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.
		* Is non-null iff mMap contains lazy values.
		*/
		private WeakReference<Parcel> mWeakParcelledData = null;

		@@ -310,7 +315,8 @@ public class BaseBundle {
		synchronized (this) {
		final Parcel source = mParcelledData;
		if (source != null) {
		initializeFromParcelLocked(source, /recycleParcel=/ true, mParcelledByNative);
		Preconditions.checkState(mOwnsLazyValues);
		initializeFromParcelLocked(source, /ownsParcel/ true, mParcelledByNative);
		} else {
		if (DEBUG) {
		Log.d(TAG, "unparcel "
		@@ -401,11 +407,23 @@ public class BaseBundle {
		}
		}
		mMap.setValueAt(i, object);
		mLazyValues--;
		if (mOwnsLazyValues) {
		Preconditions.checkState(mLazyValues >= 0, "Lazy values ref count below 0");
		// No more lazy values in mMap, so we can recycle the parcel early rather than
		// waiting for the next GC run
		if (mLazyValues == 0) {
		Preconditions.checkState(mWeakParcelledData.get() != null,
		"Parcel recycled earlier than expected");
		recycleParcel(mWeakParcelledData.get());
		mWeakParcelledData = null;
		}
		}
		}
		return (clazz != null) ? clazz.cast(object) : (T) object;
		}

		private void initializeFromParcelLocked(@NonNull Parcel parcelledData, boolean recycleParcel,
		private void initializeFromParcelLocked(@NonNull Parcel parcelledData, boolean ownsParcel,
		boolean parcelledByNative) {
		if (isEmptyParcel(parcelledData)) {
		if (DEBUG) {
		@@ -437,9 +455,10 @@ public class BaseBundle {
		map.erase();
		map.ensureCapacity(count);
		}
		int numLazyValues = 0;
		try {
		recycleParcel &= parcelledData.readArrayMap(map, count, !parcelledByNative,
		/* lazy */ true, mClassLoader);
		numLazyValues = parcelledData.readArrayMap(map, count, !parcelledByNative,
		/* lazy */ ownsParcel, mClassLoader);
		} catch (BadParcelableException e) {
		if (sShouldDefuse) {
		Log.w(TAG, "Failed to parse Bundle, but defusing quietly", e);
		@@ -448,14 +467,19 @@ public class BaseBundle {
		throw e;
		}
		} finally {
		mMap = map;
		if (recycleParcel) {
		recycleParcel(parcelledData);
		mWeakParcelledData = null;
		if (ownsParcel) {
		if (numLazyValues == 0) {
		recycleParcel(parcelledData);
		} else {
		mWeakParcelledData = new WeakReference<>(parcelledData);
		}
		}

		mLazyValues = numLazyValues;
		mParcelledByNative = false;
		mMap = map;
		// Set field last as it is volatile
		mParcelledData = null;
		}
		if (DEBUG) {
		@@ -592,13 +616,17 @@ public class BaseBundle {

		/**
		* Removes all elements from the mapping of this Bundle.
		* Recycles the underlying parcel if it is still present.
		*/
		public void clear() {
		unparcel();
		if (mOwnsLazyValues && mWeakParcelledData != null) {
		recycleParcel(mWeakParcelledData.get());
		mWeakParcelledData = null;
		}

		mWeakParcelledData = null;
		mLazyValues = 0;
		mOwnsLazyValues = true;
		mMap.clear();
		}

		@@ -1844,8 +1872,8 @@ public class BaseBundle {
		// had been constructed with parcel or to make sure they trigger deserialization of the
		// bundle immediately; neither of which is obvious.
		synchronized (this) {
		initializeFromParcelLocked(parcel, /recycleParcel=/ false, isNativeBundle);
		unparcel(/* itemwise */ true);
		mOwnsLazyValues = false;
		initializeFromParcelLocked(parcel, /ownsParcel/ false, isNativeBundle);
		}
		return;
		}
		@@ -1862,6 +1890,7 @@ public class BaseBundle {
		+ ": " + length + " bundle bytes starting at " + offset);
		p.setDataPosition(0);

		mOwnsLazyValues = true;
		mParcelledByNative = isNativeBundle;
		mParcelledData = p;
		}

core/java/android/os/Parcel.java

+6 −7

Original line number	Diff line number	Diff line
		@@ -71,7 +71,6 @@ import java.util.Set;
		import java.util.function.BiFunction;
		import java.util.function.Function;
		import java.util.function.IntFunction;
		import java.util.function.Supplier;

		/**
		* Container for a message (data and object references) that can
		@@ -5242,20 +5241,20 @@ public final class Parcel {
		* Reads a map into {@code map}.
		*
		* @param sorted Whether the keys are sorted by their hashes, if so we use an optimized path.
		* @param lazy Whether to populate the map with lazy {@link Supplier} objects for
		* @param lazy Whether to populate the map with lazy {@link Function} objects for
		* length-prefixed values. See {@link Parcel#readLazyValue(ClassLoader)} for more
		* details.
		* @return whether the parcel can be recycled or not.
		* @return a count of the lazy values in the map
		* @hide
		*/
		boolean readArrayMap(ArrayMap<? super String, Object> map, int size, boolean sorted,
		int readArrayMap(ArrayMap<? super String, Object> map, int size, boolean sorted,
		boolean lazy, @Nullable ClassLoader loader) {
		boolean recycle = true;
		int lazyValues = 0;
		while (size > 0) {
		String key = readString();
		Object value = (lazy) ? readLazyValue(loader) : readValue(loader);
		if (value instanceof LazyValue) {
		recycle = false;
		lazyValues++;
		}
		if (sorted) {
		map.append(key, value);
		@@ -5267,7 +5266,7 @@ public final class Parcel {
		if (sorted) {
		map.validate();
		}
		return recycle;
		return lazyValues;
		}

		/**

core/tests/mockingcoretests/src/android/os/BundleRecyclingTest.java

+115 −31

Original line number	Diff line number	Diff line
		@@ -23,6 +23,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;

		import static org.junit.Assert.assertFalse;
		import static org.junit.Assert.assertNotNull;
		import static org.junit.Assert.assertNull;
		import static org.junit.Assert.assertTrue;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.times;
		@@ -34,7 +35,6 @@ import androidx.test.filters.SmallTest;

		import com.android.dx.mockito.inline.extended.StaticMockitoSession;

		import org.junit.Before;
		import org.junit.Test;
		import org.junit.runner.RunWith;
		import org.mockito.quality.Strictness;
		@@ -56,16 +56,11 @@ public class BundleRecyclingTest {
		private Parcel mParcelSpy;
		private Bundle mBundle;

		@Before
		public void setUp() throws Exception {
		setUpBundle(/* hasLazy */ true);
		}

		@Test
		public void bundleClear_whenUnparcelledWithoutLazy_recyclesParcelOnce() {
		setUpBundle(/* hasLazy */ false);
		setUpBundle(/* lazy / 0, / nonLazy */ 1);
		// Will unparcel and immediately recycle parcel
		assertNotNull(mBundle.getString("key"));
		assertNotNull(mBundle.getString("nonLazy0"));
		verify(mParcelSpy, times(1)).recycle();
		assertFalse(mBundle.isDefinitelyEmpty());

		@@ -77,6 +72,7 @@ public class BundleRecyclingTest {

		@Test
		public void bundleClear_whenParcelled_recyclesParcel() {
		setUpBundle(/* lazy */ 1);
		assertTrue(mBundle.isParcelled());
		verify(mParcelSpy, times(0)).recycle();

		@@ -89,24 +85,10 @@ public class BundleRecyclingTest {
		verify(mParcelSpy, times(1)).recycle();
		}

		@Test
		public void bundleClear_whenUnparcelledWithLazyValueUnwrapped_recyclesParcel() {
		// Will unparcel with a lazy value, and immediately unwrap the lazy value,
		// with no lazy values left at the end of getParcelable
		assertNotNull(mBundle.getParcelable("key", CustomParcelable.class));
		verify(mParcelSpy, times(0)).recycle();

		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();
		assertTrue(mBundle.isDefinitelyEmpty());

		// Should not recycle again
		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();
		}

		@Test
		public void bundleClear_whenUnparcelledWithLazy_recyclesParcel() {
		setUpBundle(/* lazy */ 1);

		// Will unparcel but keep the CustomParcelable lazy
		assertFalse(mBundle.isEmpty());
		verify(mParcelSpy, times(0)).recycle();
		@@ -122,6 +104,8 @@ public class BundleRecyclingTest {

		@Test
		public void bundleClear_whenClearedWithSharedParcel_doesNotRecycleParcel() {
		setUpBundle(/* lazy */ 1);

		Bundle copy = new Bundle();
		copy.putAll(mBundle);

		@@ -136,6 +120,8 @@ public class BundleRecyclingTest {

		@Test
		public void bundleClear_whenClearedWithCopiedParcel_doesNotRecycleParcel() {
		setUpBundle(/* lazy */ 1);

		// Will unparcel but keep the CustomParcelable lazy
		assertFalse(mBundle.isEmpty());

		@@ -151,7 +137,101 @@ public class BundleRecyclingTest {
		verify(mParcelSpy, never()).recycle();
		}

		private void setUpBundle(boolean hasLazy) {
		@Test
		public void bundleGet_whenUnparcelledWithLazyValueUnwrapped_recyclesParcel() {
		setUpBundle(/* lazy */ 1);

		// Will unparcel with a lazy value, and immediately unwrap the lazy value,
		// with no lazy values left at the end of getParcelable
		// Ref counting should immediately recycle
		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(1)).recycle();

		// Should not recycle again
		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();
		}

		@Test
		public void bundleGet_whenMultipleLazy_recyclesParcelWhenAllUnwrapped() {
		setUpBundle(/* lazy */ 2);

		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(0)).recycle();

		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(0)).recycle();

		assertNotNull(mBundle.getParcelable("lazy1", CustomParcelable.class));
		verify(mParcelSpy, times(1)).recycle();

		// Should not recycle again
		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();
		assertTrue(mBundle.isDefinitelyEmpty());
		}

		@Test
		public void bundleGet_whenLazyAndNonLazy_recyclesParcelWhenAllUnwrapped() {
		setUpBundle(/* lazy / 1, / nonLazy */ 1);

		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(1)).recycle();

		// Should not recycle again
		assertNotNull(mBundle.getString("nonLazy0"));
		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();
		}

		@Test
		public void bundleGet_whenLazyAndNonLazy_doesNotRecycleWhenOnlyNonLazyRetrieved() {
		setUpBundle(/* lazy / 1, / nonLazy */ 1);

		assertNotNull(mBundle.getString("nonLazy0"));
		verify(mParcelSpy, times(0)).recycle();

		assertNotNull(mBundle.getString("nonLazy0"));
		verify(mParcelSpy, times(0)).recycle();

		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(1)).recycle();
		}

		@Test
		public void bundleGet_withWithSharedParcel_doesNotRecycleParcel() {
		setUpBundle(/* lazy */ 1);

		Bundle copy = new Bundle();
		copy.putAll(mBundle);

		assertNotNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		mBundle.clear();

		assertNotNull(copy.getParcelable("lazy0", CustomParcelable.class));
		copy.clear();

		verify(mParcelSpy, never()).recycle();
		}

		@Test
		public void bundleGet_getAfterLazyCleared_doesNotRecycleAgain() {
		setUpBundle(/* lazy */ 1);
		mBundle.clear();
		verify(mParcelSpy, times(1)).recycle();

		assertNull(mBundle.getParcelable("lazy0", CustomParcelable.class));
		verify(mParcelSpy, times(1)).recycle();
		}

		private void setUpBundle(int lazy) {
		setUpBundle(lazy, /* nonLazy */ 0);
		}

		private void setUpBundle(int lazy, int nonLazy) {
		AtomicReference<Parcel> parcel = new AtomicReference<>();
		StaticMockitoSession session = mockitoSession()
		.strictness(Strictness.STRICT_STUBS)
		@@ -166,7 +246,7 @@ public class BundleRecyclingTest {

		Bundle bundle = new Bundle();
		bundle.setClassLoader(getClass().getClassLoader());
		Parcel p = createBundle(hasLazy);
		Parcel p = createBundle(lazy, nonLazy);
		bundle.readFromParcel(p);
		p.recycle();

		@@ -179,13 +259,17 @@ public class BundleRecyclingTest {
		/**
		* Create a test bundle, parcel it and return the parcel.
		*/
		private Parcel createBundle(boolean hasLazy) {
		private Parcel createBundle(int lazy, int nonLazy) {
		final Bundle source = new Bundle();
		if (hasLazy) {
		source.putParcelable("key", new CustomParcelable(13, "Tiramisu"));
		} else {
		source.putString("key", "tiramisu");

		for (int i = 0; i < nonLazy; i++) {
		source.putString("nonLazy" + i, "Tiramisu");
		}

		for (int i = 0; i < lazy; i++) {
		source.putParcelable("lazy" + i, new CustomParcelable(13, "Tiramisu"));
		}

		return getParcelledBundle(source);
		}