From 230bd2020c64c4bb45dea5d4592df6191f3718cd Mon Sep 17 00:00:00 2001 From: liangshenglin1 Date: Fri, 20 Aug 2021 02:05:20 +0000 Subject: [PATCH] fix dead lock of create proxy Signed-off-by: liangshenglin --- .../ipc_core/include/ipc_object_proxy.h | 2 +- .../src/core/source/ipc_object_proxy.cpp | 63 +++++-------------- .../src/core/source/ipc_process_skeleton.cpp | 29 +++++---- .../mock/source/dbinder_databus_invoker.cpp | 4 +- 4 files changed, 33 insertions(+), 65 deletions(-) diff --git a/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h b/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h index 1d66e4b..889b310 100755 --- a/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h +++ b/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h @@ -70,7 +70,7 @@ public: std::string GetDataBusName(); int GetProto() const; - void WaitForInit(); + void WaitForInit(bool newProxy); std::u16string GetInterfaceDescriptor(); private: diff --git a/ipc/native/src/core/source/ipc_object_proxy.cpp b/ipc/native/src/core/source/ipc_object_proxy.cpp index 1d524ab..96164fd 100755 --- a/ipc/native/src/core/source/ipc_object_proxy.cpp +++ b/ipc/native/src/core/source/ipc_object_proxy.cpp @@ -103,7 +103,6 @@ int IPCObjectProxy::SendRequestInner(bool isLocal, uint32_t code, MessageParcel std::u16string IPCObjectProxy::GetInterfaceDescriptor() { - std::lock_guard lockGuard(initMutex_); if (!remoteDescriptor_.empty()) { return remoteDescriptor_; } @@ -159,34 +158,33 @@ std::string IPCObjectProxy::GetDataBusName() void IPCObjectProxy::OnFirstStrongRef(const void *objectId) { - return WaitForInit(); + IRemoteInvoker *invoker = IPCThreadSkeleton::GetDefaultInvoker(); + if (invoker != nullptr) { + invoker->AcquireHandle(handle_); + } } -void IPCObjectProxy::WaitForInit() +void IPCObjectProxy::WaitForInit(bool newProxy) { #ifndef CONFIG_IPC_SINGLE int type = 0; #endif - { - bool acquire = true; std::lock_guard lockGuard(initMutex_); if (IsObjectDead()) { ZLOGI(LABEL, "check a dead proxy, init again"); isRemoteDead_ = false; isFinishInit_ = false; - acquire = false; } // check again is this object been initialized if (isFinishInit_) { return; } - IRemoteInvoker *invoker = IPCThreadSkeleton::GetDefaultInvoker(); - if (invoker != nullptr && acquire == true) { - invoker->AcquireHandle(handle_); - } #ifndef CONFIG_IPC_SINGLE + if (newProxy == true) { + ReleaseProto(); + } type = UpdateProto(); #endif isFinishInit_ = true; @@ -206,15 +204,15 @@ void IPCObjectProxy::OnLastStrongRef(const void *objectId) return; } - if (current->DetachObject(this)) { + if (current->DetachObject(this)) { // if detach successfully, this proxy will be destroyed +#ifndef CONFIG_IPC_SINGLE + ReleaseProto(); +#endif IRemoteInvoker *invoker = IPCThreadSkeleton::GetDefaultInvoker(); if (invoker != nullptr) { invoker->ReleaseHandle(handle_); } } -#ifndef CONFIG_IPC_SINGLE - ReleaseProto(); -#endif } @@ -412,23 +410,7 @@ void IPCObjectProxy::IncRefToRemote() void IPCObjectProxy::ReleaseProto() { - switch (GetProto()) { - case IRemoteObject::IF_PROT_BINDER: { - ZLOGW(LABEL, "it is normal binder, try to delete handle to index"); - ReleaseBinderProto(); - break; - } - case IRemoteObject::IF_PROT_DATABUS: { - ReleaseDatabusProto(); - break; - } - default: { - ZLOGE(LABEL, "ReleaseProto Invalid Type"); - break; - } - } - - return; + ReleaseDatabusProto(); } void IPCObjectProxy::SetProto(int proto) @@ -637,24 +619,7 @@ void IPCObjectProxy::ReleaseDatabusProto() void IPCObjectProxy::ReleaseBinderProto() { - if (handle_ == 0) { - ZLOGI(LABEL, "%s:handle == 0, do nothing", __func__); - return; - } - - if (GetProto() != IRemoteObject::IF_PROT_BINDER) { - ZLOGI(LABEL, "not binder proxy, need do nothing"); - return; - } - - IPCProcessSkeleton *current = IPCProcessSkeleton::GetCurrent(); - if (current == nullptr) { - ZLOGE(LABEL, "release proto current is null"); - return; - } - - (void)current->DetachHandleToIndex(handle_); - return; + // do nothing } #endif } // namespace OHOS diff --git a/ipc/native/src/core/source/ipc_process_skeleton.cpp b/ipc/native/src/core/source/ipc_process_skeleton.cpp index 8f6c560..7c2f9bf 100755 --- a/ipc/native/src/core/source/ipc_process_skeleton.cpp +++ b/ipc/native/src/core/source/ipc_process_skeleton.cpp @@ -123,6 +123,7 @@ std::u16string IPCProcessSkeleton::MakeHandleDescriptor(int handle) IRemoteObject *IPCProcessSkeleton::FindOrNewObject(int handle) { + bool newProxy = false; IRemoteObject *remoteObject = nullptr; std::u16string descriptor = MakeHandleDescriptor(handle); { @@ -142,20 +143,22 @@ IRemoteObject *IPCProcessSkeleton::FindOrNewObject(int handle) } } - remoteObject = new IPCObjectProxy(handle, descriptor); - remoteObject->AttemptAcquire(this); - + newProxy = true; + auto proxy = new IPCObjectProxy(handle, descriptor); + proxy->AttemptAcquire(this); // AttemptAcquire always returns true as life time is extended + remoteObject = reinterpret_cast(proxy); if (!AttachObjectInner(remoteObject)) { - DBINDER_LOGE("attach object fail"); - delete remoteObject; + DBINDER_LOGE("attach object failed"); + delete proxy; return nullptr; } - return remoteObject; + } else { + remoteObject->AttemptAcquire(this); } } IPCObjectProxy *remoteProxy = reinterpret_cast(remoteObject); - remoteProxy->WaitForInit(); + remoteProxy->WaitForInit(newProxy); return remoteObject; } @@ -233,9 +236,14 @@ bool IPCProcessSkeleton::IsContainsObject(IRemoteObject *object) bool IPCProcessSkeleton::DetachObject(IRemoteObject *object) { std::lock_guard lock(mutex_); + int strongRef = object->GetSptrRefCount(); + if (strongRef > 0) { + DBINDER_LOGI("proxy is still strong referenced:%{public}d", strongRef); + return false; + } + // If it fails, clear it in the destructor. (void)isContainStub_.erase(object); - std::u16string descriptor = object->GetObjectDescriptor(); if (descriptor.empty()) { return false; @@ -277,13 +285,8 @@ IRemoteObject *IPCProcessSkeleton::QueryObjectInner(const std::u16string &descri { auto it = objects_.find(descriptor); if (it != objects_.end()) { - if (it->second == nullptr) { - return nullptr; - } - it->second->AttemptAcquire(this); return it->second.GetRefPtr(); } - return nullptr; } diff --git a/ipc/native/src/mock/source/dbinder_databus_invoker.cpp b/ipc/native/src/mock/source/dbinder_databus_invoker.cpp index 7d0681c..20c0b28 100755 --- a/ipc/native/src/mock/source/dbinder_databus_invoker.cpp +++ b/ipc/native/src/mock/source/dbinder_databus_invoker.cpp @@ -568,12 +568,12 @@ bool DBinderDatabusInvoker::OnDatabusSessionClosed(std::shared_ptr sess std::u16string descriptor = current->MakeHandleDescriptor(*it); IRemoteObject *remoteObject = current->QueryObject(descriptor); if (remoteObject != nullptr) { + (void)current->ProxyDetachDBinderSession(*it); + (void)current->DetachHandleToIndex(*it); IPCObjectProxy *remoteProxy = reinterpret_cast(remoteObject); if (remoteProxy->IsSubscribeDeathNotice()) { remoteProxy->SendObituary(); } - (void)current->ProxyDetachDBinderSession(*it); - (void)current->DetachHandleToIndex(*it); } } -- GitLab