From b4ade59e6f6bd8ccda460bf82c49b1e0b4f73631 Mon Sep 17 00:00:00 2001 From: htt1997 Date: Wed, 7 Feb 2024 20:19:26 +0800 Subject: [PATCH 1/5] refactor:RegisterClientDeathObserver Signed-off-by: htt1997 --- .../app/src/kvstore_data_service.cpp | 78 +++++++++++++++---- .../app/src/kvstore_data_service.h | 6 ++ .../service/cloud/cloud_service_impl.cpp | 2 +- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.cpp b/services/distributeddataservice/app/src/kvstore_data_service.cpp index d8305e9ac..1c1858915 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.cpp +++ b/services/distributeddataservice/app/src/kvstore_data_service.cpp @@ -194,22 +194,28 @@ Status KvStoreDataService::RegisterClientDeathObserver(const AppId &appId, sptr< ZLOGW("check bundleName:%{public}s uid:%{public}d failed.", appId.appId.c_str(), info.uid); return Status::PERMISSION_DENIED; } - - std::lock_guard lg(mutex_); - auto iter = clients_.find(info.tokenId); - // Ignore register with same tokenId and pid - if (iter != clients_.end() && IPCSkeleton::GetCallingPid() == iter->second.GetPid()) { - ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", - appId.appId.c_str(), info.uid, IPCSkeleton::GetCallingPid()); - return Status::SUCCESS; + KvStoreClientDeathObserverImpl impl(*this); + Status status = Status::SUCCESS; + { + std::lock_guard lg(mutex_); + auto iter = clients_.find(info.tokenId); + // Ignore register with same tokenId and pid + if (iter != clients_.end() && IPCSkeleton::GetCallingPid() == iter->second.GetPid()) { + ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", appId.appId.c_str(), + info.uid, IPCSkeleton::GetCallingPid()); + return Status::SUCCESS; + } + if (iter != clients_.end()) { + impl = std::move(iter->second); + clients_.erase(iter); + } + auto it = clients_.emplace(std::piecewise_construct, std::forward_as_tuple(info.tokenId), + std::forward_as_tuple(appId, *this, std::move(observer))); + status = it.second ? Status::SUCCESS : Status::ERROR; } - - clients_.erase(info.tokenId); - auto it = clients_.emplace(std::piecewise_construct, std::forward_as_tuple(info.tokenId), - std::forward_as_tuple(appId, *this, std::move(observer))); - ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d inserted:%{public}s.", - appId.appId.c_str(), info.uid, IPCSkeleton::GetCallingPid(), it.second ? "success" : "failed"); - return it.second ? Status::SUCCESS : Status::ERROR; + ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d inserted:%{public}s.", appId.appId.c_str(), info.uid, + IPCSkeleton::GetCallingPid(), status == Status::SUCCESS ? "success" : "failed"); + return status; } Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const AppId &appId) @@ -218,8 +224,15 @@ Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const A // memory of parameter appId locates in a member of clientDeathObserverMap_ and will be freed after // clientDeathObserverMap_ erase, so we have to take a copy if we want to use this parameter after erase operation. AppId appIdTmp = appId; - std::lock_guard lg(mutex_); - clients_.erase(token); + KvStoreClientDeathObserverImpl impl(*this); + { + std::lock_guard lg(mutex_); + auto iter = clients_.find(token); + if (iter != clients_.end()) { + impl = std::move(iter->second); + clients_.erase(iter); + } + } return Status::SUCCESS; } @@ -553,10 +566,31 @@ KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverIm ZLOGW("observerProxy_ is nullptr"); } } +KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl(KvStoreDataService& service) + : dataService_(service) +{ + Reset(); +} + +KvStoreDataService::KvStoreClientDeathObserverImpl& KvStoreDataService::KvStoreClientDeathObserverImpl::operator=( + KvStoreDataService::KvStoreClientDeathObserverImpl&& impl) +{ + uid_ = std::move(impl.uid_); + pid_ = std::move(impl.pid_); + token_ = std::move(impl.token_); + appId_.appId = std::move(impl.appId_.appId); + observerProxy_ = std::move(impl.observerProxy_); + deathRecipient_ = std::move(impl.deathRecipient_); + impl.Reset(); + return *this; +} KvStoreDataService::KvStoreClientDeathObserverImpl::~KvStoreClientDeathObserverImpl() { ZLOGI("~KvStoreClientDeathObserverImpl"); + if (uid_ == INVALID_UID || pid_ == INVALID_PID || token_ == INVALID_TOKEN || !appId_.IsValid()) { + return; + } if (deathRecipient_ != nullptr && observerProxy_ != nullptr) { ZLOGI("remove death recipient"); observerProxy_->RemoveDeathRecipient(deathRecipient_); @@ -578,6 +612,16 @@ pid_t KvStoreDataService::KvStoreClientDeathObserverImpl::GetPid() const return pid_; } +void KvStoreDataService::KvStoreClientDeathObserverImpl::Reset() +{ + uid_ = INVALID_UID; + pid_ = INVALID_PID; + token_ = INVALID_TOKEN; + appId_.appId = ""; + observerProxy_ = nullptr; + deathRecipient_ = nullptr; +} + KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreDeathRecipient::KvStoreDeathRecipient( KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl) : kvStoreClientDeathObserverImpl_(kvStoreClientDeathObserverImpl) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.h b/services/distributeddataservice/app/src/kvstore_data_service.h index 09f7784e2..b3253b8b2 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.h +++ b/services/distributeddataservice/app/src/kvstore_data_service.h @@ -121,6 +121,8 @@ private: class KvStoreClientDeathObserverImpl { public: KvStoreClientDeathObserverImpl(const AppId &appId, KvStoreDataService &service, sptr observer); + KvStoreClientDeathObserverImpl(KvStoreDataService &service); + KvStoreClientDeathObserverImpl& operator=(KvStoreClientDeathObserverImpl &&impl); virtual ~KvStoreClientDeathObserverImpl(); @@ -137,6 +139,7 @@ private: KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl_; }; void NotifyClientDie(); + void Reset(); pid_t uid_; pid_t pid_; uint32_t token_; @@ -180,6 +183,9 @@ private: static constexpr char FORMAT_BLANK_SPACE = ' '; static constexpr int32_t PRINTF_COUNT_2 = 2; static constexpr int MAXIMUM_PARAMETER_LIMIT = 3; + static constexpr pid_t INVALID_UID = -1; + static constexpr pid_t INVALID_PID = -1; + static constexpr uint32_t INVALID_TOKEN = 0; }; } #endif // KVSTORE_DATASERVICE_H diff --git a/services/distributeddataservice/service/cloud/cloud_service_impl.cpp b/services/distributeddataservice/service/cloud/cloud_service_impl.cpp index 7025e5dbe..aaf05a1c7 100644 --- a/services/distributeddataservice/service/cloud/cloud_service_impl.cpp +++ b/services/distributeddataservice/service/cloud/cloud_service_impl.cpp @@ -576,7 +576,7 @@ std::pair CloudServiceImpl::GetCloudInfo(int32_t userId) return { ERROR, cloudInfo }; } std::tie(status, cloudInfo) = GetCloudInfoFromServer(userId); - if (status == SUCCESS) { + if (status != SUCCESS) { return { status, cloudInfo }; } MetaDataManager::GetInstance().SaveMeta(cloudInfo.GetKey(), cloudInfo, true); -- Gitee From 1bae653c88845bb6ad81bd2acae2e7a96bdee16b Mon Sep 17 00:00:00 2001 From: htt1997 Date: Sun, 18 Feb 2024 20:20:15 +0800 Subject: [PATCH 2/5] refactor:code check Signed-off-by: htt1997 --- .../app/src/kvstore_data_service.cpp | 19 ++++++++++++++++--- .../app/src/kvstore_data_service.h | 3 ++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.cpp b/services/distributeddataservice/app/src/kvstore_data_service.cpp index 1c1858915..62f5ea225 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.cpp +++ b/services/distributeddataservice/app/src/kvstore_data_service.cpp @@ -572,12 +572,25 @@ KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverIm Reset(); } +KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl( + KvStoreDataService::KvStoreClientDeathObserverImpl&& impl) + : dataService_(impl.dataService_) +{ + uid_ = impl.uid_; + pid_ = impl.pid_; + token_ = impl.token_; + appId_.appId = std::move(impl.appId_.appId); + observerProxy_ = std::move(impl.observerProxy_); + deathRecipient_ = std::move(impl.deathRecipient_); + impl.Reset(); +} + KvStoreDataService::KvStoreClientDeathObserverImpl& KvStoreDataService::KvStoreClientDeathObserverImpl::operator=( KvStoreDataService::KvStoreClientDeathObserverImpl&& impl) { - uid_ = std::move(impl.uid_); - pid_ = std::move(impl.pid_); - token_ = std::move(impl.token_); + uid_ = impl.uid_; + pid_ = impl.pid_; + token_ = impl.token_; appId_.appId = std::move(impl.appId_.appId); observerProxy_ = std::move(impl.observerProxy_); deathRecipient_ = std::move(impl.deathRecipient_); diff --git a/services/distributeddataservice/app/src/kvstore_data_service.h b/services/distributeddataservice/app/src/kvstore_data_service.h index b3253b8b2..2b613867f 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.h +++ b/services/distributeddataservice/app/src/kvstore_data_service.h @@ -121,7 +121,8 @@ private: class KvStoreClientDeathObserverImpl { public: KvStoreClientDeathObserverImpl(const AppId &appId, KvStoreDataService &service, sptr observer); - KvStoreClientDeathObserverImpl(KvStoreDataService &service); + explicit KvStoreClientDeathObserverImpl(KvStoreDataService &service); + explicit KvStoreClientDeathObserverImpl(KvStoreClientDeathObserverImpl &&impl); KvStoreClientDeathObserverImpl& operator=(KvStoreClientDeathObserverImpl &&impl); virtual ~KvStoreClientDeathObserverImpl(); -- Gitee From cb935fef42269963b6105e88bf5dfce944aef23f Mon Sep 17 00:00:00 2001 From: htt1997 Date: Thu, 22 Feb 2024 15:16:28 +0800 Subject: [PATCH 3/5] refacotr:Review comments modification Signed-off-by: htt1997 --- .../app/src/kvstore_data_service.cpp | 50 ++++++++----------- .../app/src/kvstore_data_service.h | 3 +- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.cpp b/services/distributeddataservice/app/src/kvstore_data_service.cpp index 62f5ea225..4dde6799e 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.cpp +++ b/services/distributeddataservice/app/src/kvstore_data_service.cpp @@ -72,7 +72,7 @@ using DBConfig = DistributedDB::RuntimeConfig; REGISTER_SYSTEM_ABILITY_BY_ID(KvStoreDataService, DISTRIBUTED_KV_DATA_SERVICE_ABILITY_ID, true); KvStoreDataService::KvStoreDataService(bool runOnCreate) - : SystemAbility(runOnCreate), mutex_(), clients_() + : SystemAbility(runOnCreate), clients_() { ZLOGI("begin."); if (executors_ == nullptr) { @@ -84,7 +84,7 @@ KvStoreDataService::KvStoreDataService(bool runOnCreate) } KvStoreDataService::KvStoreDataService(int32_t systemAbilityId, bool runOnCreate) - : SystemAbility(systemAbilityId, runOnCreate), mutex_(), clients_() + : SystemAbility(systemAbilityId, runOnCreate), clients_() { ZLOGI("begin"); if (executors_ == nullptr) { @@ -98,7 +98,7 @@ KvStoreDataService::KvStoreDataService(int32_t systemAbilityId, bool runOnCreate KvStoreDataService::~KvStoreDataService() { ZLOGI("begin."); - clients_.clear(); + clients_.Clear(); features_.Clear(); } @@ -194,28 +194,24 @@ Status KvStoreDataService::RegisterClientDeathObserver(const AppId &appId, sptr< ZLOGW("check bundleName:%{public}s uid:%{public}d failed.", appId.appId.c_str(), info.uid); return Status::PERMISSION_DENIED; } - KvStoreClientDeathObserverImpl impl(*this); - Status status = Status::SUCCESS; - { - std::lock_guard lg(mutex_); - auto iter = clients_.find(info.tokenId); - // Ignore register with same tokenId and pid - if (iter != clients_.end() && IPCSkeleton::GetCallingPid() == iter->second.GetPid()) { + KvStoreClientDeathObserverImpl kvStoreClientDeathObserver(*this); + clients_.Compute(info.tokenId, [&info, &appId, &kvStoreClientDeathObserver, this, &observer](auto&, + KvStoreClientDeathObserverImpl& impl) mutable { + if (IPCSkeleton::GetCallingPid() == impl.GetPid()) { ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", appId.appId.c_str(), info.uid, IPCSkeleton::GetCallingPid()); - return Status::SUCCESS; + return true; } - if (iter != clients_.end()) { - impl = std::move(iter->second); - clients_.erase(iter); + if (impl.GetPid() == INVALID_PID) { + impl = KvStoreClientDeathObserverImpl(appId, *this, std::move(observer)); + return true; } - auto it = clients_.emplace(std::piecewise_construct, std::forward_as_tuple(info.tokenId), - std::forward_as_tuple(appId, *this, std::move(observer))); - status = it.second ? Status::SUCCESS : Status::ERROR; - } - ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d inserted:%{public}s.", appId.appId.c_str(), info.uid, - IPCSkeleton::GetCallingPid(), status == Status::SUCCESS ? "success" : "failed"); - return status; + kvStoreClientDeathObserver = std::move(impl); + return false; + }, KvStoreClientDeathObserverImpl(*this)); + ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d.", appId.appId.c_str(), info.uid, + IPCSkeleton::GetCallingPid()); + return Status::SUCCESS; } Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const AppId &appId) @@ -225,14 +221,10 @@ Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const A // clientDeathObserverMap_ erase, so we have to take a copy if we want to use this parameter after erase operation. AppId appIdTmp = appId; KvStoreClientDeathObserverImpl impl(*this); - { - std::lock_guard lg(mutex_); - auto iter = clients_.find(token); - if (iter != clients_.end()) { - impl = std::move(iter->second); - clients_.erase(iter); - } - } + clients_.ComputeIfPresent(token, [&impl](auto&, auto& value) { + impl = std::move(value); + return false; + }); return Status::SUCCESS; } diff --git a/services/distributeddataservice/app/src/kvstore_data_service.h b/services/distributeddataservice/app/src/kvstore_data_service.h index 2b613867f..d3d4d893d 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.h +++ b/services/distributeddataservice/app/src/kvstore_data_service.h @@ -170,8 +170,7 @@ private: static constexpr int TEN_SEC = 10; - std::mutex mutex_; - std::map clients_; + ConcurrentMap clients_; std::shared_ptr accountEventObserver_; std::shared_ptr security_; -- Gitee From 3df4abf09715852786e4ff29b929f234a0be20ff Mon Sep 17 00:00:00 2001 From: htt1997 Date: Thu, 22 Feb 2024 20:58:39 +0800 Subject: [PATCH 4/5] refactor:using smart ptr Signed-off-by: htt1997 --- .../app/src/kvstore_data_service.cpp | 62 +++---------------- .../app/src/kvstore_data_service.h | 9 +-- 2 files changed, 10 insertions(+), 61 deletions(-) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.cpp b/services/distributeddataservice/app/src/kvstore_data_service.cpp index 4dde6799e..6ec738c8d 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.cpp +++ b/services/distributeddataservice/app/src/kvstore_data_service.cpp @@ -194,21 +194,21 @@ Status KvStoreDataService::RegisterClientDeathObserver(const AppId &appId, sptr< ZLOGW("check bundleName:%{public}s uid:%{public}d failed.", appId.appId.c_str(), info.uid); return Status::PERMISSION_DENIED; } - KvStoreClientDeathObserverImpl kvStoreClientDeathObserver(*this); + std::shared_ptr kvStoreClientDeathObserver; clients_.Compute(info.tokenId, [&info, &appId, &kvStoreClientDeathObserver, this, &observer](auto&, - KvStoreClientDeathObserverImpl& impl) mutable { - if (IPCSkeleton::GetCallingPid() == impl.GetPid()) { - ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", appId.appId.c_str(), - info.uid, IPCSkeleton::GetCallingPid()); + std::shared_ptr& impl) mutable { + if (impl == nullptr) { + impl = std::make_shared(appId, *this, std::move(observer)); return true; } - if (impl.GetPid() == INVALID_PID) { - impl = KvStoreClientDeathObserverImpl(appId, *this, std::move(observer)); + if (IPCSkeleton::GetCallingPid() == impl->GetPid()) { + ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", appId.appId.c_str(), + info.uid, IPCSkeleton::GetCallingPid()); return true; } kvStoreClientDeathObserver = std::move(impl); return false; - }, KvStoreClientDeathObserverImpl(*this)); + }); ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d.", appId.appId.c_str(), info.uid, IPCSkeleton::GetCallingPid()); return Status::SUCCESS; @@ -220,7 +220,7 @@ Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const A // memory of parameter appId locates in a member of clientDeathObserverMap_ and will be freed after // clientDeathObserverMap_ erase, so we have to take a copy if we want to use this parameter after erase operation. AppId appIdTmp = appId; - KvStoreClientDeathObserverImpl impl(*this); + std::shared_ptr impl; clients_.ComputeIfPresent(token, [&impl](auto&, auto& value) { impl = std::move(value); return false; @@ -558,44 +558,10 @@ KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverIm ZLOGW("observerProxy_ is nullptr"); } } -KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl(KvStoreDataService& service) - : dataService_(service) -{ - Reset(); -} - -KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl( - KvStoreDataService::KvStoreClientDeathObserverImpl&& impl) - : dataService_(impl.dataService_) -{ - uid_ = impl.uid_; - pid_ = impl.pid_; - token_ = impl.token_; - appId_.appId = std::move(impl.appId_.appId); - observerProxy_ = std::move(impl.observerProxy_); - deathRecipient_ = std::move(impl.deathRecipient_); - impl.Reset(); -} - -KvStoreDataService::KvStoreClientDeathObserverImpl& KvStoreDataService::KvStoreClientDeathObserverImpl::operator=( - KvStoreDataService::KvStoreClientDeathObserverImpl&& impl) -{ - uid_ = impl.uid_; - pid_ = impl.pid_; - token_ = impl.token_; - appId_.appId = std::move(impl.appId_.appId); - observerProxy_ = std::move(impl.observerProxy_); - deathRecipient_ = std::move(impl.deathRecipient_); - impl.Reset(); - return *this; -} KvStoreDataService::KvStoreClientDeathObserverImpl::~KvStoreClientDeathObserverImpl() { ZLOGI("~KvStoreClientDeathObserverImpl"); - if (uid_ == INVALID_UID || pid_ == INVALID_PID || token_ == INVALID_TOKEN || !appId_.IsValid()) { - return; - } if (deathRecipient_ != nullptr && observerProxy_ != nullptr) { ZLOGI("remove death recipient"); observerProxy_->RemoveDeathRecipient(deathRecipient_); @@ -617,16 +583,6 @@ pid_t KvStoreDataService::KvStoreClientDeathObserverImpl::GetPid() const return pid_; } -void KvStoreDataService::KvStoreClientDeathObserverImpl::Reset() -{ - uid_ = INVALID_UID; - pid_ = INVALID_PID; - token_ = INVALID_TOKEN; - appId_.appId = ""; - observerProxy_ = nullptr; - deathRecipient_ = nullptr; -} - KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreDeathRecipient::KvStoreDeathRecipient( KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl) : kvStoreClientDeathObserverImpl_(kvStoreClientDeathObserverImpl) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.h b/services/distributeddataservice/app/src/kvstore_data_service.h index d3d4d893d..07d32d294 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.h +++ b/services/distributeddataservice/app/src/kvstore_data_service.h @@ -121,9 +121,6 @@ private: class KvStoreClientDeathObserverImpl { public: KvStoreClientDeathObserverImpl(const AppId &appId, KvStoreDataService &service, sptr observer); - explicit KvStoreClientDeathObserverImpl(KvStoreDataService &service); - explicit KvStoreClientDeathObserverImpl(KvStoreClientDeathObserverImpl &&impl); - KvStoreClientDeathObserverImpl& operator=(KvStoreClientDeathObserverImpl &&impl); virtual ~KvStoreClientDeathObserverImpl(); @@ -140,7 +137,6 @@ private: KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl_; }; void NotifyClientDie(); - void Reset(); pid_t uid_; pid_t pid_; uint32_t token_; @@ -170,7 +166,7 @@ private: static constexpr int TEN_SEC = 10; - ConcurrentMap clients_; + ConcurrentMap> clients_; std::shared_ptr accountEventObserver_; std::shared_ptr security_; @@ -183,9 +179,6 @@ private: static constexpr char FORMAT_BLANK_SPACE = ' '; static constexpr int32_t PRINTF_COUNT_2 = 2; static constexpr int MAXIMUM_PARAMETER_LIMIT = 3; - static constexpr pid_t INVALID_UID = -1; - static constexpr pid_t INVALID_PID = -1; - static constexpr uint32_t INVALID_TOKEN = 0; }; } #endif // KVSTORE_DATASERVICE_H -- Gitee From c01ed96ea04c16d3cc53d7680e3ae8d9227905e5 Mon Sep 17 00:00:00 2001 From: htt1997 Date: Mon, 26 Feb 2024 15:51:44 +0800 Subject: [PATCH 5/5] refactor Signed-off-by: htt1997 --- .../app/src/kvstore_data_service.cpp | 77 ++++++++++++++----- .../app/src/kvstore_data_service.h | 9 ++- 2 files changed, 67 insertions(+), 19 deletions(-) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.cpp b/services/distributeddataservice/app/src/kvstore_data_service.cpp index 6ec738c8d..45409734e 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.cpp +++ b/services/distributeddataservice/app/src/kvstore_data_service.cpp @@ -194,23 +194,26 @@ Status KvStoreDataService::RegisterClientDeathObserver(const AppId &appId, sptr< ZLOGW("check bundleName:%{public}s uid:%{public}d failed.", appId.appId.c_str(), info.uid); return Status::PERMISSION_DENIED; } - std::shared_ptr kvStoreClientDeathObserver; - clients_.Compute(info.tokenId, [&info, &appId, &kvStoreClientDeathObserver, this, &observer](auto&, - std::shared_ptr& impl) mutable { - if (impl == nullptr) { - impl = std::make_shared(appId, *this, std::move(observer)); - return true; - } - if (IPCSkeleton::GetCallingPid() == impl->GetPid()) { - ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", appId.appId.c_str(), - info.uid, IPCSkeleton::GetCallingPid()); + KvStoreClientDeathObserverImpl kvStoreClientDeathObserver(*this); + auto inserted = clients_.Emplace( + [&info, &appId, &kvStoreClientDeathObserver](decltype(clients_)::map_type &entries) { + auto it = entries.find(info.tokenId); + if (it == entries.end()) { + return true; + } + if (IPCSkeleton::GetCallingPid() == it->second.GetPid()) { + ZLOGW("bundleName:%{public}s, uid:%{public}d, pid:%{public}d has already registered.", + appId.appId.c_str(), info.uid, IPCSkeleton::GetCallingPid()); + return false; + } + kvStoreClientDeathObserver = std::move(it->second); + entries.erase(it); return true; - } - kvStoreClientDeathObserver = std::move(impl); - return false; - }); - ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d.", appId.appId.c_str(), info.uid, - IPCSkeleton::GetCallingPid()); + }, + std::piecewise_construct, std::forward_as_tuple(info.tokenId), + std::forward_as_tuple(appId, *this, std::move(observer))); + ZLOGI("bundleName:%{public}s, uid:%{public}d, pid:%{public}d, inserted:%{public}s.", appId.appId.c_str(), info.uid, + IPCSkeleton::GetCallingPid(), inserted ? "success" : "failed"); return Status::SUCCESS; } @@ -220,8 +223,8 @@ Status KvStoreDataService::AppExit(pid_t uid, pid_t pid, uint32_t token, const A // memory of parameter appId locates in a member of clientDeathObserverMap_ and will be freed after // clientDeathObserverMap_ erase, so we have to take a copy if we want to use this parameter after erase operation. AppId appIdTmp = appId; - std::shared_ptr impl; - clients_.ComputeIfPresent(token, [&impl](auto&, auto& value) { + KvStoreClientDeathObserverImpl impl(*this); + clients_.ComputeIfPresent(token, [&impl](auto &, auto &value) { impl = std::move(value); return false; }); @@ -558,6 +561,33 @@ KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverIm ZLOGW("observerProxy_ is nullptr"); } } +KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl(KvStoreDataService &service) + : dataService_(service) +{ + Reset(); +} + +KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreClientDeathObserverImpl( + KvStoreDataService::KvStoreClientDeathObserverImpl &&impl) + : dataService_(impl.dataService_) +{ + uid_ = impl.uid_; + pid_ = impl.pid_; + token_ = impl.token_; + appId_.appId = std::move(impl.appId_.appId); + impl.Reset(); +} + +KvStoreDataService::KvStoreClientDeathObserverImpl &KvStoreDataService::KvStoreClientDeathObserverImpl::operator=( + KvStoreDataService::KvStoreClientDeathObserverImpl &&impl) +{ + uid_ = impl.uid_; + pid_ = impl.pid_; + token_ = impl.token_; + appId_.appId = std::move(impl.appId_.appId); + impl.Reset(); + return *this; +} KvStoreDataService::KvStoreClientDeathObserverImpl::~KvStoreClientDeathObserverImpl() { @@ -566,6 +596,9 @@ KvStoreDataService::KvStoreClientDeathObserverImpl::~KvStoreClientDeathObserverI ZLOGI("remove death recipient"); observerProxy_->RemoveDeathRecipient(deathRecipient_); } + if (uid_ == INVALID_UID || pid_ == INVALID_PID || token_ == INVALID_TOKEN || !appId_.IsValid()) { + return; + } dataService_.features_.ForEachCopies([this](const auto &, sptr &value) { value->OnAppExit(uid_, pid_, token_, appId_); return false; @@ -583,6 +616,14 @@ pid_t KvStoreDataService::KvStoreClientDeathObserverImpl::GetPid() const return pid_; } +void KvStoreDataService::KvStoreClientDeathObserverImpl::Reset() +{ + uid_ = INVALID_UID; + pid_ = INVALID_PID; + token_ = INVALID_TOKEN; + appId_.appId = ""; +} + KvStoreDataService::KvStoreClientDeathObserverImpl::KvStoreDeathRecipient::KvStoreDeathRecipient( KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl) : kvStoreClientDeathObserverImpl_(kvStoreClientDeathObserverImpl) diff --git a/services/distributeddataservice/app/src/kvstore_data_service.h b/services/distributeddataservice/app/src/kvstore_data_service.h index 07d32d294..1f7d001ac 100644 --- a/services/distributeddataservice/app/src/kvstore_data_service.h +++ b/services/distributeddataservice/app/src/kvstore_data_service.h @@ -121,6 +121,9 @@ private: class KvStoreClientDeathObserverImpl { public: KvStoreClientDeathObserverImpl(const AppId &appId, KvStoreDataService &service, sptr observer); + explicit KvStoreClientDeathObserverImpl(KvStoreDataService &service); + explicit KvStoreClientDeathObserverImpl(KvStoreClientDeathObserverImpl &&impl); + KvStoreClientDeathObserverImpl &operator=(KvStoreClientDeathObserverImpl &&impl); virtual ~KvStoreClientDeathObserverImpl(); @@ -137,6 +140,7 @@ private: KvStoreClientDeathObserverImpl &kvStoreClientDeathObserverImpl_; }; void NotifyClientDie(); + void Reset(); pid_t uid_; pid_t pid_; uint32_t token_; @@ -166,7 +170,7 @@ private: static constexpr int TEN_SEC = 10; - ConcurrentMap> clients_; + ConcurrentMap clients_; std::shared_ptr accountEventObserver_; std::shared_ptr security_; @@ -179,6 +183,9 @@ private: static constexpr char FORMAT_BLANK_SPACE = ' '; static constexpr int32_t PRINTF_COUNT_2 = 2; static constexpr int MAXIMUM_PARAMETER_LIMIT = 3; + static constexpr pid_t INVALID_UID = -1; + static constexpr pid_t INVALID_PID = -1; + static constexpr uint32_t INVALID_TOKEN = 0; }; } #endif // KVSTORE_DATASERVICE_H -- Gitee