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

Commit 4449d4e5 authored by Devin Moore's avatar Devin Moore
Browse files

Change callback handling from Arc to Box 

There is only ever one ref count for this callback object, and it's
always completely dropped when the Accessor or AccessorProvider objects
are dropped, so Box is a better type.

Test: atest rustBinderTest # has the *callback_destruction tests
Bug: 358427181
Change-Id: Ifc4b17a589b27f232c20c7e8a82d2b23a717fda8
parent a3867c73

libs/binder/rust/src/system_only.rs

+13 −13
Original line number Original line Diff line number Diff line
@@ -23,7 +23,7 @@ use std::ffi::{c_void, CStr, CString}; 
use std::os::raw::c_char;

use std::os::raw::c_char;





use libc::{sockaddr, sockaddr_un, sockaddr_vm, socklen_t};

use libc::{sockaddr, sockaddr_un, sockaddr_vm, socklen_t};

use std::sync::Arc;

use std::boxed::Box;

use std::{mem, ptr};

use std::{mem, ptr};





/// Rust wrapper around ABinderRpc_Accessor objects for RPC binder service management.

/// Rust wrapper around ABinderRpc_Accessor objects for RPC binder service management.
@@ -65,7 +65,7 @@ impl Accessor { 
    where

    where

        F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static,

        F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static,

    {

    {

        let callback: *mut c_void = Arc::into_raw(Arc::new(callback)) as *mut c_void;

        let callback: *mut c_void = Box::into_raw(Box::new(callback)) as *mut c_void;

        let inst = CString::new(instance).unwrap();

        let inst = CString::new(instance).unwrap();





        // Safety: The function pointer is a valid connection_info callback.

        // Safety: The function pointer is a valid connection_info callback.
@@ -149,7 +149,7 @@ impl Accessor { 
    ///   the string within isize::MAX from the pointer. The memory must not be mutated for

    ///   the string within isize::MAX from the pointer. The memory must not be mutated for

    ///   the duration of this function  call and must be valid for reads from the pointer

    ///   the duration of this function  call and must be valid for reads from the pointer

    ///   to the null terminator.

    ///   to the null terminator.

    /// - The `cookie` parameter must be the cookie for an `Arc<F>` and

    /// - The `cookie` parameter must be the cookie for a `Box<F>` and

    ///   the caller must hold a ref-count to it.

    ///   the caller must hold a ref-count to it.

    unsafe extern "C" fn connection_info<F>(

    unsafe extern "C" fn connection_info<F>(

        instance: *const c_char,

        instance: *const c_char,
@@ -162,7 +162,7 @@ impl Accessor { 
            log::error!("Cookie({cookie:p}) or instance({instance:p}) is null!");

            log::error!("Cookie({cookie:p}) or instance({instance:p}) is null!");

            return ptr::null_mut();

            return ptr::null_mut();

        }

        }

        // Safety: The caller promises that `cookie` is for an Arc<F>.

        // Safety: The caller promises that `cookie` is for a Box<F>.

        let callback = unsafe { (cookie as *const F).as_ref().unwrap() };

        let callback = unsafe { (cookie as *const F).as_ref().unwrap() };





        // Safety: The caller in libbinder_ndk will have already verified this is a valid

        // Safety: The caller in libbinder_ndk will have already verified this is a valid
@@ -207,19 +207,19 @@ impl Accessor { 
        }

        }

    }

    }





    /// Callback that decrements the ref-count.

    /// Callback that drops the `Box<F>`.

    /// This is invoked from C++ when a binder is unlinked.

    /// This is invoked from C++ when a binder is unlinked.

    ///

    ///

    /// # Safety

    /// # Safety

    ///

    ///

    /// - The `cookie` parameter must be the cookie for an `Arc<F>` and

    /// - The `cookie` parameter must be the cookie for a `Box<F>` and

    ///   the owner must give up a ref-count to it.

    ///   the owner must give up a ref-count to it.

    unsafe extern "C" fn cookie_decr_refcount<F>(cookie: *mut c_void)

    unsafe extern "C" fn cookie_decr_refcount<F>(cookie: *mut c_void)

    where

    where

        F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static,

        F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static,

    {

    {

        // Safety: The caller promises that `cookie` is for an Arc<F>.

        // Safety: The caller promises that `cookie` is for a Box<F>.

        unsafe { Arc::decrement_strong_count(cookie as *const F) };

        unsafe { std::mem::drop(Box::from_raw(cookie as *mut F)) };

    }

    }

}

}
@@ -296,7 +296,7 @@ impl AccessorProvider { 
    where

    where

        F: Fn(&str) -> Option<Accessor> + Send + Sync + 'static,

        F: Fn(&str) -> Option<Accessor> + Send + Sync + 'static,

    {

    {

        let callback: *mut c_void = Arc::into_raw(Arc::new(provider)) as *mut c_void;

        let callback: *mut c_void = Box::into_raw(Box::new(provider)) as *mut c_void;

        let c_str_instances: Vec<CString> =

        let c_str_instances: Vec<CString> =

            instances.iter().map(|s| CString::new(s.as_bytes()).unwrap()).collect();

            instances.iter().map(|s| CString::new(s.as_bytes()).unwrap()).collect();

        let mut c_instances: Vec<*const c_char> =

        let mut c_instances: Vec<*const c_char> =
@@ -346,7 +346,7 @@ impl AccessorProvider { 
            log::error!("Cookie({cookie:p}) or instance({instance:p}) is null!");

            log::error!("Cookie({cookie:p}) or instance({instance:p}) is null!");

            return ptr::null_mut();

            return ptr::null_mut();

        }

        }

        // Safety: The caller promises that `cookie` is for an Arc<F>.

        // Safety: The caller promises that `cookie` is for a Box<F>.

        let callback = unsafe { (cookie as *const F).as_ref().unwrap() };

        let callback = unsafe { (cookie as *const F).as_ref().unwrap() };





        let inst = {

        let inst = {
@@ -377,14 +377,14 @@ impl AccessorProvider { 
    ///

    ///

    /// # Safety

    /// # Safety

    ///

    ///

    /// - The `cookie` parameter must be the cookie for an `Arc<F>` and

    /// - The `cookie` parameter must be the cookie for a `Box<F>` and

    ///   the owner must give up a ref-count to it.

    ///   the owner must give up a ref-count to it.

    unsafe extern "C" fn accessor_cookie_decr_refcount<F>(cookie: *mut c_void)

    unsafe extern "C" fn accessor_cookie_decr_refcount<F>(cookie: *mut c_void)

    where

    where

        F: Fn(&str) -> Option<Accessor> + Send + Sync + 'static,

        F: Fn(&str) -> Option<Accessor> + Send + Sync + 'static,

    {

    {

        // Safety: The caller promises that `cookie` is for an Arc<F>.

        // Safety: The caller promises that `cookie` is for a Box<F>.

        unsafe { Arc::decrement_strong_count(cookie as *const F) };

        unsafe { std::mem::drop(Box::from_raw(cookie as *mut F)) };

    }

    }

}

}