Description:fix rpc mutex_ of used

Feature or Bugfix:fix rpc mutex_ of used
Binary Source: No
Signed-off-by: NYippo <liuyibo12@huawei.com>

ipc/native/src/core/include/ipc_process_skeleton.h

@@ -100,9 +100,7 @@ public:
     sptr<IRemoteObject> FindOrNewObject(int handle);
     bool IsContainsObject(IRemoteObject *object);
     sptr<IRemoteObject> QueryObject(const std::u16string &descriptor);
-    IRemoteObject *QueryObjectInner(const std::u16string &descriptor);
     bool AttachObject(IRemoteObject *object);
-    bool AttachObjectInner(IRemoteObject *object);
     bool DetachObject(IRemoteObject *object);
 
     bool OnThreadTerminated(const std::string &threadName);

ipc/native/src/core/source/ipc_object_proxy.cpp

@@ -382,6 +382,7 @@ bool IPCObjectProxy::RemoveDeathRecipient(const sptr<DeathRecipient> &recipient)
 
 void IPCObjectProxy::SendObituary()
 {
+    ZLOGW(LABEL, "%{public}s: enter, handle: %{public}d", __func__, handle_);
 #ifndef CONFIG_IPC_SINGLE
     if (handle_ < IPCProcessSkeleton::DBINDER_HANDLE_BASE) {
         if (proto_ == IRemoteObject::IF_PROT_DATABUS || proto_ == IRemoteObject::IF_PROT_ERROR) {
@@ -443,6 +444,7 @@ int IPCObjectProxy::GetProto() const
 
 int32_t IPCObjectProxy::NoticeServiceDie()
 {
+    ZLOGW(LABEL, "%{public}s: handle: %{public}d", __func__, handle_);
     MessageParcel data;
     MessageParcel reply;
     MessageOption option(MessageOption::TF_ASYNC);

ipc/native/src/core/source/ipc_object_stub.cpp

@@ -94,6 +94,7 @@ int IPCObjectStub::OnRemoteRequest(uint32_t code, MessageParcel &data, MessagePa
     switch (code) {
 #ifndef CONFIG_IPC_SINGLE
         case DBINDER_OBITUARY_TRANSACTION: {
+            ZLOGW(LABEL, "%{public}s: recv DBINDER_OBITUARY_TRANSACTION", __func__);
             if (data.ReadInt32() == IRemoteObject::DeathRecipient::NOTICE_DEATH_RECIPIENT) {
                 result = NoticeServiceDie(data, reply, option);
             } else {
@@ -453,6 +454,7 @@ int32_t IPCObjectStub::InvokerDataBusThread(MessageParcel &data, MessageParcel &
 
 int32_t IPCObjectStub::NoticeServiceDie(MessageParcel &data, MessageParcel &reply, MessageOption &option)
 {
+    ZLOGE(LABEL, "%{public}s enter, desc:%{public}s", __func__, Str16ToStr8(descriptor_).c_str());
     IPCProcessSkeleton *current = IPCProcessSkeleton::GetCurrent();
     if (current == nullptr) {
         ZLOGE(LABEL, "%{public}s: current is null", __func__);

ipc/native/src/core/source/ipc_process_skeleton.cpp

@@ -144,9 +144,8 @@ sptr<IRemoteObject> IPCProcessSkeleton::FindOrNewObject(int handle)
         return nullptr;
     }
     {
-        std::unique_lock<std::shared_mutex> lockGuard(mutex_);
-        IRemoteObject *remoteObject = QueryObjectInner(descriptor);
-        if (remoteObject == nullptr || !remoteObject->AttemptIncStrong(this)) {
+        result = QueryObject(descriptor);
+        if (result == nullptr) {
             // Either this is a new handle or attemptIncStrong failed(strong refcount has been decreased to zero),
             // we need to create a new proxy and initialize it. Meanwhile, the old proxy is destroying concurrently.
             if (handle == REGISTRY_HANDLE) {
@@ -162,9 +161,7 @@ sptr<IRemoteObject> IPCProcessSkeleton::FindOrNewObject(int handle)
             }
             // OnFirstStrongRef will be called.
             result = new (std::nothrow) IPCObjectProxy(handle, descriptor);
-            AttachObjectInner(result.GetRefPtr());
-        } else {
-            result = remoteObject;
+            AttachObject(result.GetRefPtr());
         }
     }
 
@@ -251,7 +248,7 @@ bool IPCProcessSkeleton::OnThreadTerminated(const std::string &threadName)
 bool IPCProcessSkeleton::IsContainsObject(IRemoteObject *object)
 {
     // check whether it is a valid IPCObjectStub object.
-    std::unique_lock<std::shared_mutex> lockGuard(mutex_);
+    std::shared_lock<std::shared_mutex> lockGuard(mutex_);
     auto it = isContainStub_.find(object);
     if (it != isContainStub_.end()) {
         return it->second;
@@ -282,11 +279,6 @@ bool IPCProcessSkeleton::DetachObject(IRemoteObject *object)
 bool IPCProcessSkeleton::AttachObject(IRemoteObject *object)
 {
     std::unique_lock<std::shared_mutex> lockGuard(mutex_);
-    return AttachObjectInner(object);
-}
-
-bool IPCProcessSkeleton::AttachObjectInner(IRemoteObject *object)
-{
     (void)isContainStub_.insert(std::pair<IRemoteObject *, bool>(object, true));
 
     std::u16string descriptor = object->GetObjectDescriptor();
@@ -306,24 +298,19 @@ sptr<IRemoteObject> IPCProcessSkeleton::QueryObject(const std::u16string &descri
         return result;
     }
 
-    std::unique_lock<std::shared_mutex> lockGuard(mutex_);
-    IRemoteObject *remoteObject = QueryObjectInner(descriptor);
-    if (remoteObject == nullptr || !remoteObject->AttemptIncStrong(this)) {
-        return result;
-    }
-    result = remoteObject;
-    return result;
-}
-
-IRemoteObject *IPCProcessSkeleton::QueryObjectInner(const std::u16string &descriptor)
-{
+    std::shared_lock<std::shared_mutex> lockGuard(mutex_);
+    IRemoteObject *remoteObject = nullptr;
     auto it = objects_.find(descriptor);
     if (it != objects_.end()) {
         // Life-time of IPCObjectProxy is extended to WEAK
         // now it's weak reference counted, so it's safe to get raw pointer
-        return it->second.GetRefPtr();
+        remoteObject = it->second.GetRefPtr();
     }
-    return nullptr;
+    if (remoteObject == nullptr || !remoteObject->AttemptIncStrong(this)) {
+        return result;
+    }
+    result = remoteObject;
+    return result;
 }
 
 #ifndef CONFIG_IPC_SINGLE
@@ -966,7 +953,7 @@ bool IPCProcessSkeleton::AttachCallbackStub(IPCObjectProxy *ipcProxy, sptr<IPCOb
 sptr<IPCObjectStub> IPCProcessSkeleton::DetachCallbackStub(IPCObjectProxy *ipcProxy)
 {
     sptr<IPCObjectStub> ret = nullptr;
-    std::shared_lock<std::shared_mutex> lockGuard(callbackStubMutex_);
+    std::unique_lock<std::shared_mutex> lockGuard(callbackStubMutex_);
     auto it = noticeStub_.find(ipcProxy);
     if (it != noticeStub_.end()) {
         ret = it->second;
@@ -1163,7 +1150,7 @@ bool IPCProcessSkeleton::DetachDBinderCallbackStubByProxy(sptr<IRemoteObject> pr
 
 void IPCProcessSkeleton::DetachDBinderCallbackStub(DBinderCallbackStub *stub)
 {
-    std::shared_lock<std::shared_mutex> lockGuard(dbinderSentMutex_);
+    std::unique_lock<std::shared_mutex> lockGuard(dbinderSentMutex_);
     for (auto it = dbinderSentCallback.begin(); it != dbinderSentCallback.end(); it++) {
         if (it->second == stub) {
             dbinderSentCallback.erase(it);

ipc/native/src/mock/source/dbinder_databus_invoker.cpp

@@ -731,7 +731,7 @@ int DBinderDatabusInvoker::CheckAndSetCallerInfo(uint32_t listenFd, uint64_t stu
 {
     std::shared_ptr<DBinderSessionObject> sessionObject = QueryClientSessionObject(listenFd);
     if (sessionObject == nullptr) {
-        ZLOGE(LOG_LABEL, "session is not exist for listenFd = %u", listenFd);
+        ZLOGE(LOG_LABEL, "session is not exist for listenFd = %{public}u", listenFd);
         return RPC_DATABUS_INVOKER_INVALID_DATA_ERR;
     }
 

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

@@ -247,7 +247,7 @@ HWTEST_F(IPCProcessSkeletonUnitTest, IsContainsObjectTest001, TestSize.Level1)
     ASSERT_TRUE(skeleton != nullptr);
 
     sptr<IRemoteObject> object = new IPCObjectStub(u"testObject");
-    skeleton->AttachObjectInner(object);
+    skeleton->AttachObject(object);
     bool ret = skeleton->IsContainsObject(object.GetRefPtr());
 
     EXPECT_EQ(ret, true);

services/dbinder/dbinder_service/src/dbinder_service.cpp

@@ -36,6 +36,8 @@ bool DBinderService::mainThreadCreated_ = false;
 std::mutex DBinderService::instanceMutex_;
 std::shared_ptr<DBinderRemoteListener> DBinderService::remoteListener_ = nullptr;
 constexpr unsigned int BINDER_MASK = 0xffff;
+// DBinderServiceStub's reference count in a MakeRemoteBinder call.
+constexpr int DBINDER_STUB_REF_COUNT = 2;
 
 DBinderService::DBinderService()
 {
@@ -206,7 +208,7 @@ bool DBinderService::IsSameStubObject(const sptr<DBinderServiceStub> &stub, cons
         return false;
     }
     if (IsSameTextStr(stub->GetServiceName(), Str16ToStr8(service)) && IsSameTextStr(stub->GetDeviceID(), device)) {
-        DBINDER_LOGI(LOG_LABEL, "found registered service with name = %s", Str16ToStr8(service).c_str());
+        DBINDER_LOGI(LOG_LABEL, "found registered service with name = %{public}s", Str16ToStr8(service).c_str());
         return true;
     }
     return false;
@@ -288,11 +290,15 @@ sptr<DBinderServiceStub> DBinderService::MakeRemoteBinder(const std::u16string &
     } while (ret == WAIT_REPLY_TIMEOUT && (retryTimes < RETRY_TIMES));
 
     if (ret != DBINDER_OK) {
-        DBINDER_LOGE(LOG_LABEL, "fail to invoke service, service name = %{public}s, device = %{public}s",
-            Str16ToStr8(serviceName).c_str(), DBinderService::ConvertToSecureDeviceID(deviceID).c_str());
-        /* invoke fail, delete dbinder stub info */
-        (void)DeleteDBinderStub(serviceName, deviceID);
-        (void)DetachSessionObject(reinterpret_cast<binder_uintptr_t>(dBinderServiceStub.GetRefPtr()));
+        DBINDER_LOGE(LOG_LABEL, "fail to invoke service, service name = %{public}s, device = %{public}s "
+            "DBinderServiceStub refcount = %{public}d",
+            Str16ToStr8(serviceName).c_str(), DBinderService::ConvertToSecureDeviceID(deviceID).c_str(),
+            dBinderServiceStub->GetSptrRefCount());
+        if (dBinderServiceStub->GetSptrRefCount() <= DBINDER_STUB_REF_COUNT) {
+            /* invoke fail, delete dbinder stub info */
+            (void)DeleteDBinderStub(serviceName, deviceID);
+            (void)DetachSessionObject(reinterpret_cast<binder_uintptr_t>(dBinderServiceStub.GetRefPtr()));
+        }
         return nullptr;
     }
 
@@ -448,9 +454,7 @@ void DBinderService::LoadSystemAbilityComplete(const std::string& srcNetworkId,
                 continue;
             }
             if (!AttachProxyObject(remoteObject, binderObject)) {
-                SendMessageToRemote(MESSAGE_AS_REMOTE_ERROR, SA_NOT_FOUND, replyMessage);
-                DBINDER_LOGE(LOG_LABEL, "attach proxy object fail");
-                continue;
+                DBINDER_LOGW(LOG_LABEL, "attach proxy object is already existed");
             }
         }
         std::string deviceId = replyMessage->deviceIdInfo.fromDeviceId;
@@ -978,6 +982,9 @@ bool DBinderService::AttachCallbackProxy(sptr<IRemoteObject> object, DBinderServ
 
 bool DBinderService::NoticeCallbackProxy(sptr<DBinderServiceStub> dbStub)
 {
+    DBINDER_LOGI(LOG_LABEL, "%{public}s: enter, service:%{public}s devicId:%{public}s", 
+        __func__, dbStub->GetServiceName().c_str(), 
+        DBinderService::ConvertToSecureDeviceID(dbStub->GetDeviceID()).c_str());
     bool status = true;
     const binder_uintptr_t binderObject = reinterpret_cast<binder_uintptr_t>(dbStub.GetRefPtr());
     if (!DetachSessionObject(binderObject)) {
@@ -1033,6 +1040,9 @@ int32_t DBinderService::NoticeServiceDieInner(const std::u16string &serviceName,
         return DBINDER_SERVICE_INVALID_DATA_ERR;
     }
 
+    DBINDER_LOGI(LOG_LABEL, "%{public}s: service:%{public}s devicId:%{public}s", 
+        __func__, Str16ToStr8(serviceName).c_str(), 
+        DBinderService::ConvertToSecureDeviceID(deviceID).c_str());
     sptr<DBinderServiceStub> dbStub = FindDBinderStub(serviceName, deviceID);
     if (dbStub == nullptr) {
         DBINDER_LOGE(LOG_LABEL, "find null stub, do not need notice death");

services/dbinder/dbinder_service/src/dbinder_service_stub.cpp

@@ -113,6 +113,7 @@ int32_t DBinderServiceStub::OnRemoteRequest(uint32_t code, MessageParcel &data,
             break;
         }
         case DBINDER_OBITUARY_TRANSACTION: {
+            DBINDER_LOGE(LOG_LABEL, "%{public}s: recv DBINDER_OBITUARY_TRANSACTION", __func__);
             result = ProcessDeathRecipient(data, reply);
             break;
         }
@@ -129,6 +130,7 @@ int32_t DBinderServiceStub::OnRemoteRequest(uint32_t code, MessageParcel &data,
 int32_t DBinderServiceStub::ProcessDeathRecipient(MessageParcel &data, MessageParcel &reply)
 {
     int32_t processType = data.ReadInt32();
+    DBINDER_LOGE(LOG_LABEL, "%{public}s: enter, processType:%{public}d", __func__, processType);
     if (processType == IRemoteObject::DeathRecipient::ADD_DEATH_RECIPIENT) {
         return AddDbinderDeathRecipient(data, reply);
     }
@@ -150,6 +152,8 @@ int32_t DBinderServiceStub::AddDbinderDeathRecipient(MessageParcel &data, Messag
 
     IPCObjectProxy *callbackProxy = reinterpret_cast<IPCObjectProxy *>(object.GetRefPtr());
     sptr<IRemoteObject::DeathRecipient> death(new DbinderDeathRecipient());
+    DBINDER_LOGE(LOG_LABEL, "%{public}s: stub desc:%{public}s", 
+        __func__, DBinderService::ConvertToSecureDeviceID(Str16ToStr8(descriptor_)).c_str());
 
     // If the client dies, notify DBS to delete information of callbackProxy
     if (!callbackProxy->AddDeathRecipient(death)) {
@@ -184,7 +188,8 @@ int32_t DBinderServiceStub::RemoveDbinderDeathRecipient(MessageParcel &data, Mes
     }
 
     IPCObjectProxy *callbackProxy = reinterpret_cast<IPCObjectProxy *>(object.GetRefPtr());
-
+    DBINDER_LOGE(LOG_LABEL, "%{public}s: stub desc:%{public}s", 
+        __func__, DBinderService::ConvertToSecureDeviceID(Str16ToStr8(descriptor_)).c_str());
     sptr<DBinderService> dBinderService = DBinderService::GetInstance();
     if (dBinderService == nullptr) {
         DBINDER_LOGE(LOG_LABEL, "dBinder service is null");