Merge "binder_rs: Make ParcelableHolder thread-safe" (5f8b5952) · Commits · e / os / platform_frameworks_native

libs/binder/rust/src/parcel/parcelable_holder.rs

+37 −33

Original line number	Original line	Diff line number	Diff line
	@@ -16,13 +16,12 @@

	use crate::binder::Stability;		use crate::binder::Stability;
	use crate::error::{Result, StatusCode};		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 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::any::Any;
	use std::cell::RefCell;		use std::sync::{Arc, Mutex};
	use std::rc::Rc;

	/// Metadata that `ParcelableHolder` needs for all parcelables.		/// Metadata that `ParcelableHolder` needs for all parcelables.
	///		///
	@@ -40,18 +39,18 @@ pub trait ParcelableMetadata {
	}		}
	}		}

	trait AnyParcelable: Downcast + Parcelable + std::fmt::Debug {}		trait AnyParcelable: DowncastSync + Parcelable + std::fmt::Debug {}
	impl_downcast!(AnyParcelable);		impl_downcast!(sync AnyParcelable);
	impl<T> AnyParcelable for T where T: Downcast + Parcelable + std::fmt::Debug {}		impl<T> AnyParcelable for T where T: DowncastSync + Parcelable + std::fmt::Debug {}

	#[derive(Debug, Clone)]		#[derive(Debug, Clone)]
	enum ParcelableHolderData {		enum ParcelableHolderData {
	Empty,		Empty,
	Parcelable {		Parcelable {
	parcelable: Rc<dyn AnyParcelable>,		parcelable: Arc<dyn AnyParcelable>,
	name: String,		name: String,
	},		},
	Parcel(Parcel),		Parcel(OwnedParcel),
	}		}

	impl Default for ParcelableHolderData {		impl Default for ParcelableHolderData {
	@@ -67,15 +66,15 @@ impl Default for ParcelableHolderData {
	/// `ParcelableHolder` is currently not thread-safe (neither		/// `ParcelableHolder` is currently not thread-safe (neither
	/// `Send` nor `Sync`), mainly because it internally contains		/// `Send` nor `Sync`), mainly because it internally contains
	/// a `Parcel` which in turn is not thread-safe.		/// a `Parcel` which in turn is not thread-safe.
	#[derive(Debug, Default, Clone)]		#[derive(Debug, Default)]
	pub struct ParcelableHolder {		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++.		// which takes `&self` for consistency with C++.
	// We could make `get_parcelable` take a `&mut self`		// 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		// improvement, but then callers would require a mutable
	// `ParcelableHolder` even for that getter method.		// `ParcelableHolder` even for that getter method.
	data: RefCell<ParcelableHolderData>,		data: Mutex<ParcelableHolderData>,
	stability: Stability,		stability: Stability,
	}		}

	@@ -83,7 +82,7 @@ impl ParcelableHolder {
	/// Construct a new `ParcelableHolder` with the given stability.		/// Construct a new `ParcelableHolder` with the given stability.
	pub fn new(stability: Stability) -> Self {		pub fn new(stability: Stability) -> Self {
	Self {		Self {
	data: RefCell::new(ParcelableHolderData::Empty),		data: Mutex::new(ParcelableHolderData::Empty),
	stability,		stability,
	}		}
	}		}
	@@ -93,20 +92,20 @@ impl ParcelableHolder {
	/// Note that this method does not reset the stability,		/// Note that this method does not reset the stability,
	/// only the contents.		/// only the contents.
	pub fn reset(&mut self) {		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		// We could also clear stability here, but C++ doesn't
	}		}

	/// Set the parcelable contained in this `ParcelableHolder`.		/// 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		where
	T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug,		T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug + Send + Sync,
	{		{
	if self.stability > p.get_stability() {		if self.stability > p.get_stability() {
	return Err(StatusCode::BAD_VALUE);		return Err(StatusCode::BAD_VALUE);
	}		}

	*self.data.get_mut() = ParcelableHolderData::Parcelable {		*self.data.get_mut().unwrap() = ParcelableHolderData::Parcelable {
	parcelable: p,		parcelable: p,
	name: T::get_descriptor().into(),		name: T::get_descriptor().into(),
	};		};
	@@ -127,12 +126,12 @@ impl ParcelableHolder {
	/// * `Ok(None)` if the holder is empty or the descriptor does not match		/// * `Ok(None)` if the holder is empty or the descriptor does not match
	/// * `Ok(Some(_))` if the object holds a parcelable of type `T`		/// * `Ok(Some(_))` if the object holds a parcelable of type `T`
	/// with the correct descriptor		/// 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		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 parcelable_desc = T::get_descriptor();
	let mut data = self.data.borrow_mut();		let mut data = self.data.lock().unwrap();
	match *data {		match *data {
	ParcelableHolderData::Empty => Ok(None),		ParcelableHolderData::Empty => Ok(None),
	ParcelableHolderData::Parcelable {		ParcelableHolderData::Parcelable {
	@@ -143,12 +142,13 @@ impl ParcelableHolder {
	return Err(StatusCode::BAD_VALUE);		return Err(StatusCode::BAD_VALUE);
	}		}

	match Rc::clone(parcelable).downcast_rc::<T>() {		match Arc::clone(parcelable).downcast_arc::<T>() {
	Err(_) => Err(StatusCode::BAD_VALUE),		Err(_) => Err(StatusCode::BAD_VALUE),
	Ok(x) => Ok(Some(x)),		Ok(x) => Ok(Some(x)),
	}		}
	}		}
	ParcelableHolderData::Parcel(ref parcel) => {		ParcelableHolderData::Parcel(ref mut parcel) => {
			let parcel = parcel.borrowed();
	unsafe {		unsafe {
	// Safety: 0 should always be a valid position.		// Safety: 0 should always be a valid position.
	parcel.set_data_position(0)?;		parcel.set_data_position(0)?;
	@@ -160,10 +160,10 @@ impl ParcelableHolder {
	}		}

	let mut parcelable = T::default();		let mut parcelable = T::default();
	parcelable.read_from_parcel(parcel)?;		parcelable.read_from_parcel(&parcel)?;

	let parcelable = Rc::new(parcelable);		let parcelable = Arc::new(parcelable);
	let result = Rc::clone(&parcelable);		let result = Arc::clone(&parcelable);
	*data = ParcelableHolderData::Parcelable { parcelable, name };		*data = ParcelableHolderData::Parcelable { parcelable, name };

	Ok(Some(result))		Ok(Some(result))
	@@ -184,7 +184,8 @@ impl Parcelable for ParcelableHolder {
	fn write_to_parcel(&self, parcel: &mut Parcel) -> Result<()> {		fn write_to_parcel(&self, parcel: &mut Parcel) -> Result<()> {
	parcel.write(&self.stability)?;		parcel.write(&self.stability)?;

	match *self.data.borrow() {		let mut data = self.data.lock().unwrap();
			match *data {
	ParcelableHolderData::Empty => parcel.write(&0i32),		ParcelableHolderData::Empty => parcel.write(&0i32),
	ParcelableHolderData::Parcelable {		ParcelableHolderData::Parcelable {
	ref parcelable,		ref parcelable,
	@@ -212,9 +213,10 @@ impl Parcelable for ParcelableHolder {

	Ok(())		Ok(())
	}		}
	ParcelableHolderData::Parcel(ref p) => {		ParcelableHolderData::Parcel(ref mut p) => {
			let p = p.borrowed();
	parcel.write(&p.get_data_size())?;		parcel.write(&p.get_data_size())?;
	parcel.append_all_from(p)		parcel.append_all_from(&p)
	}		}
	}		}
	}		}
	@@ -229,7 +231,7 @@ impl Parcelable for ParcelableHolder {
	return Err(StatusCode::BAD_VALUE);		return Err(StatusCode::BAD_VALUE);
	}		}
	if data_size == 0 {		if data_size == 0 {
	*self.data.get_mut() = ParcelableHolderData::Empty;		*self.data.get_mut().unwrap() = ParcelableHolderData::Empty;
	return Ok(());		return Ok(());
	}		}

	@@ -240,9 +242,11 @@ impl Parcelable for ParcelableHolder {
	.checked_add(data_size)		.checked_add(data_size)
	.ok_or(StatusCode::BAD_VALUE)?;		.ok_or(StatusCode::BAD_VALUE)?;

	let mut new_parcel = Parcel::new();		let mut new_parcel = OwnedParcel::new();
	new_parcel.append_from(parcel, data_start, data_size)?;		new_parcel
	*self.data.get_mut() = ParcelableHolderData::Parcel(new_parcel);		.borrowed()
			.append_from(parcel, data_start, data_size)?;
			*self.data.get_mut().unwrap() = ParcelableHolderData::Parcel(new_parcel);

	unsafe {		unsafe {
	// Safety: `append_from` checks if `data_size` overflows		// Safety: `append_from` checks if `data_size` overflows