Merge "Refactor Bundle for thread-safety of Bundle.hasFileDescriptors()" am: f5cc73d25f am: adcfd2f940 am: f509e2d9b7 am: 7ceb94a294 am: 7e83fa172f

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1878339

Change-Id: Idefb8a2b014d08da48ae9fb30368c718546020cd
This commit is contained in:
Bernardo Rufino
2021-11-05 21:24:16 +00:00
committed by Automerger Merge Worker
3 changed files with 75 additions and 63 deletions

View File

@@ -103,7 +103,7 @@ public class BaseBundle {
* are unparcelled, mParcelledData willbe set to null.
*/
@UnsupportedAppUsage
Parcel mParcelledData = null;
volatile Parcel mParcelledData = null;
/**
* Whether {@link #mParcelledData} was generated by native code or not.
@@ -182,13 +182,56 @@ public class BaseBundle {
* @param b a Bundle to be copied.
*/
BaseBundle(BaseBundle b) {
copyInternal(b, false);
this(b, /* deep */ false);
}
/**
* Special constructor that does not initialize the bundle.
* Constructs a {@link BaseBundle} containing a copy of {@code from}.
*
* @param from The bundle to be copied.
* @param deep Whether is a deep or shallow copy.
*
* @hide
*/
BaseBundle(boolean doInit) {
BaseBundle(BaseBundle from, boolean deep) {
synchronized (from) {
mClassLoader = from.mClassLoader;
if (from.mMap != null) {
if (!deep) {
mMap = new ArrayMap<>(from.mMap);
} else {
final ArrayMap<String, Object> fromMap = from.mMap;
final int n = fromMap.size();
mMap = new ArrayMap<>(n);
for (int i = 0; i < n; i++) {
mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
}
}
} else {
mMap = null;
}
final Parcel parcelledData;
if (from.mParcelledData != null) {
if (from.isEmptyParcel()) {
parcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
mParcelledByNative = false;
} else {
parcelledData = Parcel.obtain();
parcelledData.appendFrom(from.mParcelledData, 0,
from.mParcelledData.dataSize());
parcelledData.setDataPosition(0);
mParcelledByNative = from.mParcelledByNative;
}
} else {
parcelledData = null;
mParcelledByNative = false;
}
// Keep as last statement to ensure visibility of other fields
mParcelledData = parcelledData;
}
}
/**
@@ -323,8 +366,8 @@ public class BaseBundle {
} else {
mMap.erase();
}
mParcelledData = null;
mParcelledByNative = false;
mParcelledData = null;
return;
}
@@ -358,8 +401,8 @@ public class BaseBundle {
if (recycleParcel) {
recycleParcel(parcelledData);
}
mParcelledData = null;
mParcelledByNative = false;
mParcelledData = null;
}
if (DEBUG) {
Log.d(TAG, "unparcel " + Integer.toHexString(System.identityHashCode(this))
@@ -501,44 +544,7 @@ public class BaseBundle {
mMap.clear();
}
void copyInternal(BaseBundle from, boolean deep) {
synchronized (from) {
if (from.mParcelledData != null) {
if (from.isEmptyParcel()) {
mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
mParcelledByNative = false;
} else {
mParcelledData = Parcel.obtain();
mParcelledData.appendFrom(from.mParcelledData, 0,
from.mParcelledData.dataSize());
mParcelledData.setDataPosition(0);
mParcelledByNative = from.mParcelledByNative;
}
} else {
mParcelledData = null;
mParcelledByNative = false;
}
if (from.mMap != null) {
if (!deep) {
mMap = new ArrayMap<>(from.mMap);
} else {
final ArrayMap<String, Object> fromMap = from.mMap;
final int N = fromMap.size();
mMap = new ArrayMap<>(N);
for (int i = 0; i < N; i++) {
mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
}
}
} else {
mMap = null;
}
mClassLoader = from.mClassLoader;
}
}
Object deepCopyValue(Object value) {
private Object deepCopyValue(Object value) {
if (value == null) {
return null;
}
@@ -570,7 +576,7 @@ public class BaseBundle {
return value;
}
ArrayList deepcopyArrayList(ArrayList from) {
private ArrayList deepcopyArrayList(ArrayList from) {
final int N = from.size();
ArrayList out = new ArrayList(N);
for (int i=0; i<N; i++) {
@@ -1717,9 +1723,9 @@ public class BaseBundle {
if (length < 0) {
throw new RuntimeException("Bad length in parcel: " + length);
} else if (length == 0) {
mParcelledByNative = false;
// Empty Bundle or end of data.
mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
mParcelledByNative = false;
return;
} else if (length % 4 != 0) {
throw new IllegalStateException("Bundle length is not aligned by 4: " + length);
@@ -1757,8 +1763,8 @@ public class BaseBundle {
+ ": " + length + " bundle bytes starting at " + offset);
p.setDataPosition(0);
mParcelledData = p;
mParcelledByNative = isNativeBundle;
mParcelledData = p;
}
/** {@hide} */

View File

@@ -101,6 +101,18 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
maybePrefillHasFds();
}
/**
* Constructs a {@link Bundle} containing a copy of {@code from}.
*
* @param from The bundle to be copied.
* @param deep Whether is a deep or shallow copy.
*
* @hide
*/
Bundle(Bundle from, boolean deep) {
super(from, deep);
}
/**
* If {@link #mParcelledData} is not null, copy the HAS FDS bit from it because it's fast.
* Otherwise (if {@link #mParcelledData} is already null), leave {@link #FLAG_HAS_FDS_KNOWN}
@@ -166,13 +178,6 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
mFlags = FLAG_HAS_FDS_KNOWN | FLAG_ALLOW_FDS;
}
/**
* Constructs a Bundle without initializing it.
*/
Bundle(boolean doInit) {
super(doInit);
}
/**
* Make a Bundle for a single key/value pair.
*
@@ -260,9 +265,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
* are referenced as-is and not copied in any way.
*/
public Bundle deepCopy() {
Bundle b = new Bundle(false);
b.copyInternal(this, true);
return b;
return new Bundle(this, /* deep */ true);
}
/**

View File

@@ -156,10 +156,15 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
}
/**
* Constructs a PersistableBundle without initializing it.
* Constructs a {@link PersistableBundle} containing a copy of {@code from}.
*
* @param from The bundle to be copied.
* @param deep Whether is a deep or shallow copy.
*
* @hide
*/
PersistableBundle(boolean doInit) {
super(doInit);
PersistableBundle(PersistableBundle from, boolean deep) {
super(from, deep);
}
/**
@@ -190,9 +195,7 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
* are referenced as-is and not copied in any way.
*/
public PersistableBundle deepCopy() {
PersistableBundle b = new PersistableBundle(false);
b.copyInternal(this, true);
return b;
return new PersistableBundle(this, /* deep */ true);
}
/**