Merge "Support deleting param for preconnected sessions" into main am: 1175227

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/3515750

Change-Id: I95842bbdca78b91195ea1a0fa85d00d17db91acd
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

Author: Treehugger Robot

libs/binder/RpcSession.cpp

@@ -188,7 +188,9 @@ status_t RpcSession::setupInetClient(const char* addr, unsigned int port) {
 }
 
 status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) {
-    return setupClient([&](const std::vector<uint8_t>& sessionId, bool incoming) -> status_t {
+    return setupClient([&, fd = std::move(fd),
+                        request = std::move(request)](const std::vector<uint8_t>& sessionId,
+                                                      bool incoming) mutable -> status_t {
         if (!fd.ok()) {
             fd = request();
             if (!fd.ok()) return BAD_VALUE;
@@ -476,8 +478,10 @@ sp<RpcServer> RpcSession::server() {
     return server;
 }
 
-status_t RpcSession::setupClient(const std::function<status_t(const std::vector<uint8_t>& sessionId,
-                                                              bool incoming)>& connectAndInit) {
+template <typename Fn,
+          typename /* = std::enable_if_t<std::is_invocable_r_v<
+                      status_t, Fn, const std::vector<uint8_t>&, bool>> */>
+status_t RpcSession::setupClient(Fn&& connectAndInit) {
     {
         RpcMutexLockGuard _l(mMutex);
         LOG_ALWAYS_FATAL_IF(mStartedSetup, "Must only setup session once");

libs/binder/include/binder/RpcSession.h

@@ -25,6 +25,7 @@
 
 #include <map>
 #include <optional>
+#include <type_traits>
 #include <vector>
 
 namespace android {
@@ -291,9 +292,14 @@ class RpcSession final : public virtual RefBase {
     // join on thread passed to preJoinThreadOwnership
     static void join(sp<RpcSession>&& session, PreJoinSetupResult&& result);
 
-    [[nodiscard]] status_t setupClient(
-            const std::function<status_t(const std::vector<uint8_t>& sessionId, bool incoming)>&
-                    connectAndInit);
+    // This is a workaround to support move-only functors.
+    // TODO: use std::move_only_function when it becomes available.
+    template <typename Fn,
+              // Fn must be a callable type taking (const std::vector<uint8_t>&, bool) and returning
+              // status_t
+              typename = std::enable_if_t<
+                      std::is_invocable_r_v<status_t, Fn, const std::vector<uint8_t>&, bool>>>
+    [[nodiscard]] status_t setupClient(Fn&& connectAndInit);
     [[nodiscard]] status_t setupSocketClient(const RpcSocketAddress& address);
     [[nodiscard]] status_t setupOneSocketConnection(const RpcSocketAddress& address,
                                                     const std::vector<uint8_t>& sessionId,

libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp

@@ -134,9 +134,14 @@ AIBinder* ARpcSession_setupInet(ARpcSession* session, const char* address, unsig
 //
 // param will be passed to requestFd. Callers can use param to pass contexts to
 // the requestFd function.
+//
+// paramDeleteFd will be called when requestFd and param are no longer in use.
+// Callers can pass a function deleting param if necessary. Callers can set
+// paramDeleteFd to null if callers doesn't need to clean up param.
 AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* session,
                                               int (*requestFd)(void* param),
-                                              void* param);
+                                              void* param,
+                                              void (*paramDeleteFd)(void* param));
 
 // Sets the file descriptor transport mode for this session.
 void ARpcSession_setFileDescriptorTransportMode(ARpcSession* session,

libs/binder/libbinder_rpc_unstable.cpp

@@ -248,9 +248,18 @@ AIBinder* ARpcSession_setupInet(ARpcSession* handle, const char* address, unsign
 #endif // __TRUSTY__
 
 AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* handle, int (*requestFd)(void* param),
-                                              void* param) {
+                                              void* param, void (*paramDeleteFd)(void* param)) {
     auto session = handleToStrongPointer<RpcSession>(handle);
-    auto request = [=] { return unique_fd{requestFd(param)}; };
+    auto deleter = [=](void* param) {
+        if (paramDeleteFd) {
+            paramDeleteFd(param);
+        }
+    };
+    // TODO: use unique_ptr once setupPreconnectedClient uses std::move_only_function.
+    std::shared_ptr<void> sharedParam(param, deleter);
+    auto request = [=, sharedParam = std::move(sharedParam)] {
+        return unique_fd{requestFd(sharedParam.get())};
+    };
     if (status_t status = session->setupPreconnectedClient(unique_fd{}, request); status != OK) {
         ALOGE("Failed to set up preconnected client. error: %s", statusToString(status).c_str());
         return nullptr;

libs/binder/rust/rpcbinder/src/session.rs

@@ -195,11 +195,13 @@ impl RpcSessionRef {
     /// take ownership of) file descriptors already connected to it.
     pub fn setup_preconnected_client<T: FromIBinder + ?Sized>(
         &self,
-        mut request_fd: impl FnMut() -> Option<RawFd>,
+        request_fd: impl FnMut() -> Option<RawFd>,
     ) -> Result<Strong<T>, StatusCode> {
-        // Double reference the factory because trait objects aren't FFI safe.
-        let mut request_fd_ref: RequestFd = &mut request_fd;
-        let param = &mut request_fd_ref as *mut RequestFd as *mut c_void;
+        // Trait objects aren't FFI safe, so *mut c_void can't be converted back to
+        // *mut dyn FnMut() -> Option<RawFd>>. Double box the factory to make it possible to get
+        // the factory from *mut c_void (to *mut Box<dyn<...>>) in the callbacks.
+        let request_fd_box: Box<dyn FnMut() -> Option<RawFd>> = Box::new(request_fd);
+        let param = Box::into_raw(Box::new(request_fd_box)) as *mut c_void;
 
         // SAFETY: AIBinder returned by RpcPreconnectedClient has correct reference count, and the
         // ownership can be safely taken by new_spibinder. RpcPreconnectedClient does not take ownership
@@ -209,6 +211,7 @@ impl RpcSessionRef {
                 self.as_ptr(),
                 Some(request_fd_wrapper),
                 param,
+                Some(param_delete_fd_wrapper),
             ))
         };
         Self::get_interface(service)
@@ -225,13 +228,18 @@ impl RpcSessionRef {
     }
 }
 
-type RequestFd<'a> = &'a mut dyn FnMut() -> Option<RawFd>;
-
 unsafe extern "C" fn request_fd_wrapper(param: *mut c_void) -> c_int {
-    let request_fd_ptr = param as *mut RequestFd;
+    let request_fd_ptr = param as *mut Box<dyn FnMut() -> Option<RawFd>>;
     // SAFETY: This is only ever called by RpcPreconnectedClient, within the lifetime of the
     // BinderFdFactory reference, with param being a properly aligned non-null pointer to an
     // initialized instance.
     let request_fd = unsafe { request_fd_ptr.as_mut().unwrap() };
     request_fd().unwrap_or(-1)
 }
+
+unsafe extern "C" fn param_delete_fd_wrapper(param: *mut c_void) {
+    // SAFETY: This is only ever called by RpcPreconnectedClient, with param being the
+    // pointer returned from Box::into_raw.
+    let request_fd_box = unsafe { Box::from_raw(param as *mut Box<dyn FnMut() -> Option<RawFd>>) };
+    drop(request_fd_box);
+}

libs/binder/trusty/rust/binder_rpc_unstable_bindgen/rules.mk

@@ -30,6 +30,8 @@ MODULE_LIBRARY_DEPS += \
 	trusty/user/base/lib/libstdc++-trusty \
 	trusty/user/base/lib/trusty-sys \
 
+MODULE_SRCDEPS := $(LIBBINDER_DIR)/include_rpc_unstable/binder_rpc_unstable.hpp
+
 MODULE_BINDGEN_SRC_HEADER := $(LOCAL_DIR)/BinderBindings.hpp
 
 MODULE_BINDGEN_FLAGS += \