Synchronize in LazyValue.get()
When we use Bundle's copy constructor we perform a shallow copy of the
underlying map, so lazy values are the same across the original and the
clone. So, if we're accessing the same item backed by a lazy value in 2
different threads, each one accessing a different bundle (the original
and the clone), both would have to synchronize on a common lock.
Although Bundle is not thread-safe, we should keep the expectation that
the client only needs to properly synchronize access to one bundle, not
more. To achieve this, we synchronize on the original parcel held by
LazyValue in get(), this removes the need for the above.
Note that after we produce the lazy values and put them in the map, we
don't have a reference to mParcelledData anymore inside the Bundle, the
only objects holding a reference to that parcel are the lazy values.
Also fixed some stuff in LazyValue:
* Fixed condition inside LazyValue that used mObject == null as
signal for "not deserialized" state since mObject can be null, so
switched to mSource != null.
* Made mSource volatile and adjusted operations to always check it
before, ensuring visibility of itself and mObject via happens-before.
This works because after checking mSource, if that's null, mObject is
guaranteed to be visible and valid, if it's not null, we can use it
since we're in the serialized state and this works even if another
thread deserializes in between as long as we hold on to a reference of
mSource in a local because the parcel is still valid.
* Remove the recycling of mValueParcel, otherwise we'd need locking to
ensure whatever was using it was still using a valid object. Decided
to just remove recycling for now since the plan is to remove
mValueParcel entirely when we add comparison and hasFileDescriptor()
methods that operate on a range to Parcel.
Bug: 195622897
Test: atest -d android.os.cts.ParcelTest android.os.cts.BundleTest
android.os.BundleTest android.os.ParcelTest
Test: More tests coming
Change-Id: I501c91f505950338b23e7837e55f89b2f63ff93a
2 files changed