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
Loading
Please register or sign in to comment