Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit feba6cad authored by Alice Ryhl's avatar Alice Ryhl
Browse files

rust: split transact into prepare and submit

The async Rust binder interface will want to prepare and build the
Parcel on one thread, and submit the transaction from another thread.
Doing this will reduce the amount of `: 'static` bounds necessary in the
async binder interface.

Such a workflow where `AIBinder_prepareTransaction` and
`AIBinder_transact` are called on two different threads is not possible
with the current API.

Test: m
Change-Id: Iaf13b070e69faf8658a202c6e05f0a2aeaa4a904
parent f76dca25
Loading
Loading
Loading
Loading
+29 −3
Original line number Original line Diff line number Diff line
@@ -152,20 +152,46 @@ pub trait IBinderInternal: IBinder {
    /// available.
    /// available.
    fn get_extension(&mut self) -> Result<Option<SpIBinder>>;
    fn get_extension(&mut self) -> Result<Option<SpIBinder>>;


    /// Create a Parcel that can be used with `submit_transact`.
    fn prepare_transact(&self) -> Result<Parcel>;

    /// Perform a generic operation with the object.
    /// Perform a generic operation with the object.
    ///
    ///
    /// The provided [`Parcel`] must have been created by a call to
    /// `prepare_transact` on the same binder.
    ///
    /// # Arguments
    ///
    /// * `code` - Transaction code for the operation.
    /// * `data` - [`Parcel`] with input data.
    /// * `flags` - Transaction flags, e.g. marking the transaction as
    ///   asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY)).
    fn submit_transact(
        &self,
        code: TransactionCode,
        data: Parcel,
        flags: TransactionFlags,
    ) -> Result<Parcel>;

    /// Perform a generic operation with the object. This is a convenience
    /// method that internally calls `prepare_transact` followed by
    /// `submit_transact.
    ///
    /// # Arguments
    /// # Arguments
    /// * `code` - Transaction code for the operation
    /// * `code` - Transaction code for the operation
    /// * `data` - [`Parcel`] with input data
    /// * `reply` - Optional [`Parcel`] for reply data
    /// * `flags` - Transaction flags, e.g. marking the transaction as
    /// * `flags` - Transaction flags, e.g. marking the transaction as
    ///   asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY))
    ///   asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY))
    /// * `input_callback` A callback for building the `Parcel`.
    fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
    fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
        &self,
        &self,
        code: TransactionCode,
        code: TransactionCode,
        flags: TransactionFlags,
        flags: TransactionFlags,
        input_callback: F,
        input_callback: F,
    ) -> Result<Parcel>;
    ) -> Result<Parcel> {
        let mut parcel = self.prepare_transact()?;
        input_callback(&mut parcel)?;
        self.submit_transact(code, parcel, flags)
    }
}
}


/// Interface of binder local or remote objects.
/// Interface of binder local or remote objects.
+15 −0
Original line number Original line Diff line number Diff line
@@ -25,6 +25,7 @@ use std::cell::RefCell;
use std::convert::TryInto;
use std::convert::TryInto;
use std::mem::ManuallyDrop;
use std::mem::ManuallyDrop;
use std::ptr;
use std::ptr;
use std::fmt;


mod file_descriptor;
mod file_descriptor;
mod parcelable;
mod parcelable;
@@ -96,6 +97,13 @@ impl Parcel {
        let _ = ManuallyDrop::new(self);
        let _ = ManuallyDrop::new(self);
        ptr
        ptr
    }
    }

    pub(crate) fn is_owned(&self) -> bool {
        match *self {
            Self::Owned(_) => true,
            Self::Borrowed(_) => false,
        }
    }
}
}


// Data serialization methods
// Data serialization methods
@@ -412,6 +420,13 @@ impl Drop for Parcel {
    }
    }
}
}


impl fmt::Debug for Parcel {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.debug_struct("Parcel")
            .finish()
    }
}

#[cfg(test)]
#[cfg(test)]
impl Parcel {
impl Parcel {
    /// Create a new parcel tied to a bogus binder. TESTING ONLY!
    /// Create a new parcel tied to a bogus binder. TESTING ONLY!
+17 −13
Original line number Original line Diff line number Diff line
@@ -233,13 +233,7 @@ impl Drop for SpIBinder {
}
}


impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
    /// Perform a binder transaction
    fn prepare_transact(&self) -> Result<Parcel> {
    fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
        &self,
        code: TransactionCode,
        flags: TransactionFlags,
        input_callback: F,
    ) -> Result<Parcel> {
        let mut input = ptr::null_mut();
        let mut input = ptr::null_mut();
        let status = unsafe {
        let status = unsafe {
            // Safety: `SpIBinder` guarantees that `self` always contains a
            // Safety: `SpIBinder` guarantees that `self` always contains a
@@ -252,15 +246,25 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
            // pointer, or null.
            // pointer, or null.
            sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input)
            sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input)
        };
        };

        status_result(status)?;
        status_result(status)?;
        let mut input = unsafe {

        unsafe {
            // Safety: At this point, `input` is either a valid, owned `AParcel`
            // Safety: At this point, `input` is either a valid, owned `AParcel`
            // pointer, or null. `Parcel::owned` safely handles both cases,
            // pointer, or null. `Parcel::owned` safely handles both cases,
            // taking ownership of the parcel.
            // taking ownership of the parcel.
            Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL)?
            Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL)
        };
        }
        input_callback(&mut input)?;
    }

    fn submit_transact(
        &self,
        code: TransactionCode,
        data: Parcel,
        flags: TransactionFlags,
    ) -> Result<Parcel> {
        let mut reply = ptr::null_mut();
        let mut reply = ptr::null_mut();
        assert!(data.is_owned());
        let status = unsafe {
        let status = unsafe {
            // Safety: `SpIBinder` guarantees that `self` always contains a
            // Safety: `SpIBinder` guarantees that `self` always contains a
            // valid pointer to an `AIBinder`. Although `IBinder::transact` is
            // valid pointer to an `AIBinder`. Although `IBinder::transact` is
@@ -275,13 +279,13 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
            // only providing `on_transact` with an immutable reference to
            // only providing `on_transact` with an immutable reference to
            // `self`.
            // `self`.
            //
            //
            // This call takes ownership of the `input` parcel pointer, and
            // This call takes ownership of the `data` parcel pointer, and
            // passes ownership of the `reply` out parameter to its caller. It
            // passes ownership of the `reply` out parameter to its caller. It
            // does not affect ownership of the `binder` parameter.
            // does not affect ownership of the `binder` parameter.
            sys::AIBinder_transact(
            sys::AIBinder_transact(
                self.as_native() as *mut sys::AIBinder,
                self.as_native() as *mut sys::AIBinder,
                code,
                code,
                &mut input.into_raw(),
                &mut data.into_raw(),
                &mut reply,
                &mut reply,
                flags,
                flags,
            )
            )
+23 −0
Original line number Original line Diff line number Diff line
@@ -637,4 +637,27 @@ mod tests {
        assert!(!(service1 > service1));
        assert!(!(service1 > service1));
        assert_eq!(service1 < service2, !(service2 < service1));
        assert_eq!(service1 < service2, !(service2 < service1));
    }
    }

    #[test]
    fn binder_parcel_mixup() {
        let service1 = BnTest::new_binder(
            TestService::new("testing_service1"),
            BinderFeatures::default(),
        );
        let service2 = BnTest::new_binder(
            TestService::new("testing_service2"),
            BinderFeatures::default(),
        );

        let service1 = service1.as_binder();
        let service2 = service2.as_binder();

        let parcel = service1.prepare_transact().unwrap();
        let res = service2.submit_transact(super::TestTransactionCode::Test as binder::TransactionCode, parcel, 0);

        match res {
            Ok(_) => panic!("submit_transact should fail"),
            Err(err) => assert_eq!(err, binder::StatusCode::BAD_VALUE),
        }
    }
}
}