!575 Fixup remote stub reference bug

Merge pull request !575 from Yanying/master

interfaces/innerkits/rust/src/ipc/macros.rs

@@ -46,8 +46,11 @@ macro_rules! define_remote_object {
 
         impl $proxy {
             /// Create proxy object by RemoteObj
-            fn from_remote_object(remote: RemoteObj) -> $crate::Result<Self> {
-                Ok(Self {remote, $($item_name: $item_init),* })
+            fn from_remote_object(remote: &RemoteObj) -> $crate::Result<Self> {
+                Ok(Self {
+                    remote: remote.clone(),
+                    $($item_name: $item_init),*
+                })
             }
 
             /// Get proxy object descriptor
@@ -99,8 +102,9 @@ macro_rules! define_remote_object {
 
         impl $crate::FromRemoteObj for dyn $remote_broker {
             /// For example, convert RemoteObj to RemoteObjRef<dyn ITest>
-            fn from(object: $crate::RemoteObj) -> $crate::Result<$crate::RemoteObjRef<dyn $remote_broker>> {
-                Ok($crate::RemoteObjRef::new(Box::new($proxy::from_remote_object(object)?)))
+            fn from(object: $crate::RemoteObj)
+                -> $crate::Result<$crate::RemoteObjRef<dyn $remote_broker>> {
+                Ok($crate::RemoteObjRef::new(Box::new($proxy::from_remote_object(&object)?)))
             }
         }
     };

interfaces/innerkits/rust/src/ipc/remote_stub.rs

@@ -82,13 +82,12 @@ impl<T: IRemoteStub> Drop for RemoteStub<T> {
 
 /// C call Rust
 impl<T: IRemoteStub> RemoteStub<T> {
-    unsafe extern "C" fn on_remote_request(stub: *mut CRemoteObject, code: u32,
+    unsafe extern "C" fn on_remote_request(user_data: *mut c_void, code: u32,
         data: *const CParcel, reply: *mut CParcel) -> i32 {
         let res = {
             let mut reply = BorrowedMsgParcel::from_raw(reply).unwrap();
             let data = BorrowedMsgParcel::from_raw(data as *mut CParcel).unwrap();
-            let object = ipc_binding::RemoteObjectGetUserData(stub);
-            let rust_object: &T = &*(object as *const T);
+            let rust_object: &T = &*(user_data as *mut T);
             rust_object.on_remote_request(code, &data, &mut reply)
         };
         res

interfaces/innerkits/rust/src/ipc_binding.rs

@@ -40,7 +40,7 @@ pub struct CAshmem {
 // Callback function type for OnRemoteRequest() from native, this
 // callback will be called when native recive client IPC request.
 pub type OnRemoteRequest = unsafe extern "C" fn (
-    stub: *mut CRemoteObject,
+    user_data: *mut c_void,
     code: u32,
     data: *const CParcel,
     reply: *mut CParcel
@@ -49,7 +49,7 @@ pub type OnRemoteRequest = unsafe extern "C" fn (
 // Callback function type for OnRemoteObjectDestroy() from native,
 // this callback will be called when native remote object destroyed.
 pub type OnRemoteObjectDestroy = unsafe extern "C" fn (
-    stub: *mut c_void
+    user_data: *mut c_void
 );
 
 // Callback function type for OnDeathRecipientCb() from native,
@@ -116,7 +116,6 @@ extern "C" {
     pub fn RemoteObjectIncStrongRef(object: *mut CRemoteObject);
     pub fn RemoteObjectDecStrongRef(object: *mut CRemoteObject);
 
-    pub fn RemoteObjectGetUserData(object: *mut CRemoteObject) -> *const c_void;
     pub fn RemoteObjectSendRequest(object: *mut CRemoteObject, code: u32,
         data: *const CParcel, reply: *mut CParcel, is_async: bool) -> i32;
     pub fn RemoteObjectLessThan(object: *const CRemoteObject,

ipc/native/src/c_wrapper/include/c_remote_object.h

@@ -31,7 +31,8 @@ struct CDeathRecipient;
 typedef struct CDeathRecipient CDeathRecipient;
 
 // Callback as remote stub
-typedef int (*OnRemoteRequestCb)(const CRemoteObject *stub, int code, const CParcel *data, CParcel *reply);
+typedef int (*OnRemoteRequestCb)(const void *userData, int code, 
+    const CParcel *data, CParcel *reply);
 typedef void (*OnRemoteObjectDestroyCb)(const void *userData);
 // Callback as death recipient
 typedef void (*OnDeathRecipientCb)(const void *userData);
@@ -43,7 +44,6 @@ CRemoteObject *CreateRemoteStub(const char *desc, OnRemoteRequestCb callback,
 void RemoteObjectIncStrongRef(CRemoteObject *object);
 void RemoteObjectDecStrongRef(CRemoteObject *object);
 
-const void *RemoteObjectGetUserData(CRemoteObject *object);
 bool RemoteObjectLessThan(const CRemoteObject *lhs, const CRemoteObject *rhs);
 int RemoteObjectSendRequest(const CRemoteObject *object, uint32_t code,
     const CParcel *data, CParcel *reply, bool isAsync);

ipc/native/src/c_wrapper/include/c_remote_object_internal.h

@@ -29,28 +29,9 @@ struct CRemoteObjectHolder : public virtual OHOS::RefBase {
     CRemoteObjectHolder() = default;
     virtual ~CRemoteObjectHolder() = default;
 
-    virtual const void *GetUserData() = 0;
     OHOS::sptr<OHOS::IRemoteObject> remote_;
 };
 
-struct CRemoteStubHolder : public CRemoteObjectHolder {
-    CRemoteStubHolder(const void *userData, OnRemoteObjectDestroyCb onRemoteObjectDestroy);
-    virtual ~CRemoteStubHolder();
-
-    const void *GetUserData() { return userData_; };
-
-private:
-    const void *userData_;
-    OnRemoteObjectDestroyCb onRemoteObjectDestroy_;
-};
-
-struct CRemoteProxyHolder : public CRemoteObjectHolder {
-    CRemoteProxyHolder();
-    virtual ~CRemoteProxyHolder();
-
-    const void *GetUserData() { return nullptr; };
-};
-
 struct CDeathRecipient: public virtual OHOS::IRemoteObject::DeathRecipient {
 public:
     CDeathRecipient(OnDeathRecipientCb onDeathRecipient,
@@ -67,15 +48,17 @@ private:
 
 class RemoteServiceHolderStub: public OHOS::IPCObjectStub {
 public:
-    explicit RemoteServiceHolderStub(CRemoteObject *holder, std::u16string &desc, OnRemoteRequestCb callback);
+    explicit RemoteServiceHolderStub(std::u16string &desc, OnRemoteRequestCb callback,
+        const void *userData, OnRemoteObjectDestroyCb destroy);
     ~RemoteServiceHolderStub();
 
     int OnRemoteRequest(uint32_t code, OHOS::MessageParcel &data,
         OHOS::MessageParcel &reply, OHOS::MessageOption &option) override;
 
 private:
-    CRemoteObject *holder_;
     OnRemoteRequestCb callback_;
+    const void *userData_;
+    OnRemoteObjectDestroyCb destroy_;
 };
 
 bool IsValidRemoteObject(const CRemoteObject *object, const char *promot);

ipc/native/src/c_wrapper/source/c_parcel.cpp

@@ -388,13 +388,9 @@ CRemoteObject *CParcelReadRemoteObject(const CParcel *parcel)
         return nullptr;
     }
     CRemoteObject *holder = nullptr;
-    if (remote->IsProxyObject()) {
-        holder = new (std::nothrow) CRemoteProxyHolder();
-    } else {
-        holder = new (std::nothrow) CRemoteStubHolder(nullptr, nullptr);
-    }
+    holder = new (std::nothrow) CRemoteObjectHolder();
     if (holder == nullptr) {
-        printf("%s: craete remote object holder failed\n", __func__);
+        printf("%s: create remote object holder failed\n", __func__);
         return nullptr;
     }
     holder->remote_ = remote;

ipc/native/src/c_wrapper/source/c_process.cpp

@@ -26,7 +26,7 @@ CRemoteObject *GetContextManager(void)
     if (saMgr == nullptr) {
         return nullptr;
     }
-    CRemoteObject *holder = new (std::nothrow) CRemoteProxyHolder();
+    CRemoteObject *holder = new (std::nothrow) CRemoteObjectHolder();
     if (holder == nullptr) {
         printf("%s: create samgr proxy holder failed\n", __func__);
         return nullptr;

ipc/native/src/c_wrapper/source/c_remote_object.cpp

@@ -21,39 +21,32 @@
 
 using namespace OHOS;
 
-RemoteServiceHolderStub::RemoteServiceHolderStub(CRemoteObject *holder, std::u16string &desc, OnRemoteRequestCb callback)
-    : IPCObjectStub(desc), holder_(holder), callback_(callback)
+RemoteServiceHolderStub::RemoteServiceHolderStub(std::u16string &desc,
+    OnRemoteRequestCb callback, const void *userData, OnRemoteObjectDestroyCb destroy)
+    : IPCObjectStub(desc), callback_(callback)
+    , userData_(userData), destroy_(destroy)
 {
 }
 
 RemoteServiceHolderStub::~RemoteServiceHolderStub()
 {
+    if (destroy_) {
+        destroy_(userData_);
+    }
+    destroy_ = nullptr;
 }
 
 int RemoteServiceHolderStub::OnRemoteRequest(uint32_t code, OHOS::MessageParcel &data,
     OHOS::MessageParcel &reply, OHOS::MessageOption &option)
 {
     (void)option;
-    if (callback_ == nullptr || holder_ == nullptr) {
+    if (callback_ == nullptr) {
         printf("%s: callback is null for code: %u\n", __func__, code);
         return -1;
     }
     CParcel parcelData(&data);
     CParcel parcelReply(&reply);
-    return callback_(holder_, code, &parcelData, &parcelReply);
-}
-
-CRemoteStubHolder::CRemoteStubHolder(const void *userData, OnRemoteObjectDestroyCb onRemoteObjectDestroy)
-    : userData_(userData), onRemoteObjectDestroy_(onRemoteObjectDestroy)
-{
-}
-
-CRemoteStubHolder::~CRemoteStubHolder()
-{
-    if (onRemoteObjectDestroy_) {
-        onRemoteObjectDestroy_(GetUserData());
-    }
-    onRemoteObjectDestroy_ = nullptr;
+    return callback_(userData_, code, &parcelData, &parcelReply);
 }
 
 CDeathRecipient::CDeathRecipient(OnDeathRecipientCb onDeathRecipient,
@@ -81,14 +74,6 @@ void CDeathRecipient::OnRemoteDied(const wptr<IRemoteObject> &object)
     }
 }
 
-CRemoteProxyHolder::CRemoteProxyHolder()
-{
-}
-
-CRemoteProxyHolder::~CRemoteProxyHolder()
-{
-}
-
 bool IsValidRemoteObject(const CRemoteObject *object, const char *promot)
 {
     if (object == nullptr) {
@@ -108,13 +93,14 @@ CRemoteObject *CreateRemoteStub(const char *desc, OnRemoteRequestCb callback,
     if (desc == nullptr || callback == nullptr || destroy == nullptr) {
         return nullptr;
     }
-    auto holder = new (std::nothrow) CRemoteStubHolder(userData, destroy);
+    auto holder = new (std::nothrow) CRemoteObjectHolder();
     if (holder == nullptr) {
-        printf("%s: new CRemoteStubHolder failed\n", __func__);
+        printf("%s: new CRemoteObjectHolder failed\n", __func__);
         return nullptr;
     }
     std::u16string descriptor = Str8ToStr16(std::string(desc));
-    holder->remote_ = new (std::nothrow) RemoteServiceHolderStub(holder, descriptor, callback);
+    holder->remote_ = new (std::nothrow) RemoteServiceHolderStub(
+        descriptor, callback, userData, destroy);
     if (holder->remote_ == nullptr) {
         printf("%s: new RemoteServiceHolderStub failed\n", __func__);
         delete holder;
@@ -142,15 +128,6 @@ void RemoteObjectDecStrongRef(CRemoteObject *object)
     object->DecStrongRef(nullptr);
 }
 
-const void *RemoteObjectGetUserData(CRemoteObject *object)
-{
-    if (object == nullptr) {
-        printf("%s: unexpected CRemoteObject\n", __func__);
-        return nullptr;
-    }
-    return object->GetUserData();
-}
-
 bool RemoteObjectLessThan(const CRemoteObject *lhs, const CRemoteObject *rhs)
 {
     if (!IsValidRemoteObject(lhs, __func__) || !IsValidRemoteObject(rhs, __func__)) {
@@ -214,9 +191,8 @@ bool AddDeathRecipient(CRemoteObject *object, CDeathRecipient *recipient)
         printf("%s: this is not a proxy object", __func__);
         return false;
     }
-    struct CRemoteProxyHolder *proxy = reinterpret_cast<struct CRemoteProxyHolder *>(object);
     sptr<IRemoteObject::DeathRecipient> callback(recipient);
-    return proxy->remote_->AddDeathRecipient(callback);
+    return object->remote_->AddDeathRecipient(callback);
 }
 
 bool RemoveDeathRecipient(CRemoteObject *object, CDeathRecipient *recipient)
@@ -228,8 +204,7 @@ bool RemoveDeathRecipient(CRemoteObject *object, CDeathRecipient *recipient)
     if (!object->remote_->IsProxyObject()) {
         printf("%s: this is not a proxy object\n", __func__);
         return false;
-    }
-    struct CRemoteProxyHolder *proxy = reinterpret_cast<struct CRemoteProxyHolder *>(object);
+    };
     sptr<IRemoteObject::DeathRecipient> callback(recipient);
-    return proxy->remote_->RemoveDeathRecipient(callback);
+    return object->remote_->RemoveDeathRecipient(callback);
 }

ipc/native/test/unittest/common/ipc_c_remote_object_unittest.cpp

@@ -45,7 +45,7 @@ void IpcCRemoteObjectUnitTest::SetUp()
 void IpcCRemoteObjectUnitTest::TearDown()
 {}
 
-static int OnRemoteRequest(const CRemoteObject *stub, int code, const CParcel *data, CParcel *reply)
+static int OnRemoteRequest(const void *stub, int code, const CParcel *data, CParcel *reply)
 {
     (void)stub;
     (void)code;
@@ -92,22 +92,6 @@ HWTEST_F(IpcCRemoteObjectUnitTest, CRemoteObjectRefCount, TestSize.Level1)
     RemoteObjectDecStrongRef(remote);
 }
 
-/**
- * @tc.name: CRemoteObjectUserData
- * @tc.desc: Verify the CRemoteObject user data function
- * @tc.type: FUNC
- */
-HWTEST_F(IpcCRemoteObjectUnitTest, CRemoteObjectUserData, TestSize.Level1)
-{
-    EXPECT_EQ(RemoteObjectGetUserData(nullptr), nullptr);
-    int8_t userData;
-    CRemoteObject *remote = CreateRemoteStub(SERVICE_NAME, OnRemoteRequest, OnRemoteObjectDestroy, &userData);
-    EXPECT_NE(remote, nullptr);
-    EXPECT_EQ(RemoteObjectGetUserData(remote), &userData);
-    // destroy the CRemoteObject object
-    RemoteObjectDecStrongRef(remote);
-}
-
 /**
  * @tc.name: CRemoteObjectCompare
  * @tc.desc: Verify the CRemoteObject less than function