Merge "binder_rs: Make ParcelableHolder thread-safe"
diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs
index 3e75d1b..bccfd2d 100644
--- a/libs/binder/rust/src/parcel/parcelable_holder.rs
+++ b/libs/binder/rust/src/parcel/parcelable_holder.rs
@@ -16,13 +16,12 @@
use crate::binder::Stability;
use crate::error::{Result, StatusCode};
-use crate::parcel::{Parcel, Parcelable};
+use crate::parcel::{OwnedParcel, Parcel, Parcelable};
use crate::{impl_deserialize_for_parcelable, impl_serialize_for_parcelable};
-use downcast_rs::{impl_downcast, Downcast};
+use downcast_rs::{impl_downcast, DowncastSync};
use std::any::Any;
-use std::cell::RefCell;
-use std::rc::Rc;
+use std::sync::{Arc, Mutex};
/// Metadata that `ParcelableHolder` needs for all parcelables.
///
@@ -40,18 +39,18 @@
}
}
-trait AnyParcelable: Downcast + Parcelable + std::fmt::Debug {}
-impl_downcast!(AnyParcelable);
-impl<T> AnyParcelable for T where T: Downcast + Parcelable + std::fmt::Debug {}
+trait AnyParcelable: DowncastSync + Parcelable + std::fmt::Debug {}
+impl_downcast!(sync AnyParcelable);
+impl<T> AnyParcelable for T where T: DowncastSync + Parcelable + std::fmt::Debug {}
#[derive(Debug, Clone)]
enum ParcelableHolderData {
Empty,
Parcelable {
- parcelable: Rc<dyn AnyParcelable>,
+ parcelable: Arc<dyn AnyParcelable>,
name: String,
},
- Parcel(Parcel),
+ Parcel(OwnedParcel),
}
impl Default for ParcelableHolderData {
@@ -67,15 +66,15 @@
/// `ParcelableHolder` is currently not thread-safe (neither
/// `Send` nor `Sync`), mainly because it internally contains
/// a `Parcel` which in turn is not thread-safe.
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default)]
pub struct ParcelableHolder {
- // This is a `RefCell` because of `get_parcelable`
+ // This is a `Mutex` because of `get_parcelable`
// which takes `&self` for consistency with C++.
// We could make `get_parcelable` take a `&mut self`
- // and get rid of the `RefCell` here for a performance
+ // and get rid of the `Mutex` here for a performance
// improvement, but then callers would require a mutable
// `ParcelableHolder` even for that getter method.
- data: RefCell<ParcelableHolderData>,
+ data: Mutex<ParcelableHolderData>,
stability: Stability,
}
@@ -83,7 +82,7 @@
/// Construct a new `ParcelableHolder` with the given stability.
pub fn new(stability: Stability) -> Self {
Self {
- data: RefCell::new(ParcelableHolderData::Empty),
+ data: Mutex::new(ParcelableHolderData::Empty),
stability,
}
}
@@ -93,20 +92,20 @@
/// Note that this method does not reset the stability,
/// only the contents.
pub fn reset(&mut self) {
- *self.data.get_mut() = ParcelableHolderData::Empty;
+ *self.data.get_mut().unwrap() = ParcelableHolderData::Empty;
// We could also clear stability here, but C++ doesn't
}
/// Set the parcelable contained in this `ParcelableHolder`.
- pub fn set_parcelable<T>(&mut self, p: Rc<T>) -> Result<()>
+ pub fn set_parcelable<T>(&mut self, p: Arc<T>) -> Result<()>
where
- T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug,
+ T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug + Send + Sync,
{
if self.stability > p.get_stability() {
return Err(StatusCode::BAD_VALUE);
}
- *self.data.get_mut() = ParcelableHolderData::Parcelable {
+ *self.data.get_mut().unwrap() = ParcelableHolderData::Parcelable {
parcelable: p,
name: T::get_descriptor().into(),
};
@@ -127,12 +126,12 @@
/// * `Ok(None)` if the holder is empty or the descriptor does not match
/// * `Ok(Some(_))` if the object holds a parcelable of type `T`
/// with the correct descriptor
- pub fn get_parcelable<T>(&self) -> Result<Option<Rc<T>>>
+ pub fn get_parcelable<T>(&self) -> Result<Option<Arc<T>>>
where
- T: Any + Parcelable + ParcelableMetadata + Default + std::fmt::Debug,
+ T: Any + Parcelable + ParcelableMetadata + Default + std::fmt::Debug + Send + Sync,
{
let parcelable_desc = T::get_descriptor();
- let mut data = self.data.borrow_mut();
+ let mut data = self.data.lock().unwrap();
match *data {
ParcelableHolderData::Empty => Ok(None),
ParcelableHolderData::Parcelable {
@@ -143,12 +142,13 @@
return Err(StatusCode::BAD_VALUE);
}
- match Rc::clone(parcelable).downcast_rc::<T>() {
+ match Arc::clone(parcelable).downcast_arc::<T>() {
Err(_) => Err(StatusCode::BAD_VALUE),
Ok(x) => Ok(Some(x)),
}
}
- ParcelableHolderData::Parcel(ref parcel) => {
+ ParcelableHolderData::Parcel(ref mut parcel) => {
+ let parcel = parcel.borrowed();
unsafe {
// Safety: 0 should always be a valid position.
parcel.set_data_position(0)?;
@@ -160,10 +160,10 @@
}
let mut parcelable = T::default();
- parcelable.read_from_parcel(parcel)?;
+ parcelable.read_from_parcel(&parcel)?;
- let parcelable = Rc::new(parcelable);
- let result = Rc::clone(&parcelable);
+ let parcelable = Arc::new(parcelable);
+ let result = Arc::clone(&parcelable);
*data = ParcelableHolderData::Parcelable { parcelable, name };
Ok(Some(result))
@@ -184,7 +184,8 @@
fn write_to_parcel(&self, parcel: &mut Parcel) -> Result<()> {
parcel.write(&self.stability)?;
- match *self.data.borrow() {
+ let mut data = self.data.lock().unwrap();
+ match *data {
ParcelableHolderData::Empty => parcel.write(&0i32),
ParcelableHolderData::Parcelable {
ref parcelable,
@@ -212,9 +213,10 @@
Ok(())
}
- ParcelableHolderData::Parcel(ref p) => {
+ ParcelableHolderData::Parcel(ref mut p) => {
+ let p = p.borrowed();
parcel.write(&p.get_data_size())?;
- parcel.append_all_from(p)
+ parcel.append_all_from(&p)
}
}
}
@@ -229,7 +231,7 @@
return Err(StatusCode::BAD_VALUE);
}
if data_size == 0 {
- *self.data.get_mut() = ParcelableHolderData::Empty;
+ *self.data.get_mut().unwrap() = ParcelableHolderData::Empty;
return Ok(());
}
@@ -240,9 +242,11 @@
.checked_add(data_size)
.ok_or(StatusCode::BAD_VALUE)?;
- let mut new_parcel = Parcel::new();
- new_parcel.append_from(parcel, data_start, data_size)?;
- *self.data.get_mut() = ParcelableHolderData::Parcel(new_parcel);
+ let mut new_parcel = OwnedParcel::new();
+ new_parcel
+ .borrowed()
+ .append_from(parcel, data_start, data_size)?;
+ *self.data.get_mut().unwrap() = ParcelableHolderData::Parcel(new_parcel);
unsafe {
// Safety: `append_from` checks if `data_size` overflows
