diff --git a/services/backup_sa/include/module_ipc/service.h b/services/backup_sa/include/module_ipc/service.h index d9c1c4bcf388dbda5ad7fdf294f9e71175db5d49..ba0037ae624de2e62ebf60b4003b557f98a74c1c 100644 --- a/services/backup_sa/include/module_ipc/service.h +++ b/services/backup_sa/include/module_ipc/service.h @@ -32,10 +32,10 @@ #include "thread_pool.h" namespace OHOS::FileManagement::Backup { -struct extensionInfo { +struct ExtensionMutexInfo { std::string bundleName; std::mutex callbackMutex; - extensionInfo(std::string bundleName_) : bundleName(bundleName_) {}; + ExtensionMutexInfo(std::string bundleName_) : bundleName(bundleName_) {}; }; class Service : public SystemAbility, public ServiceStub, protected NoCopyable { @@ -281,12 +281,12 @@ public: void DelClearBundleRecord(const std::vector &bundleNames); /** - * @brief 添加extension锁 + * @brief 获取extension锁 * * @param bundleName 应用名称 * */ - void AddExtensionMutex(const BundleName &bundleName); + std::shared_ptr GetExtensionMutex(const BundleName &bundleName); /** * @brief 清理extension锁 @@ -514,6 +514,8 @@ private: void HandleCurGroupIncBackupInfos(vector &bundleInfos, std::map> &bundleNameDetailMap, std::map &isClearDataFlags); + + void TimeoutRadarReport(IServiceReverse::Scenario scenario, std::string &bundleName); private: static sptr instance_; static std::mutex instanceLock_; @@ -533,9 +535,9 @@ private: friend class ServiceTest; OHOS::ThreadPool threadPool_; - std::mutex extensionLock_; + std::mutex extensionMutexLock_; public: - std::map> backupExtMutexMap_; + std::map> backupExtMutexMap_; }; } // namespace OHOS::FileManagement::Backup diff --git a/services/backup_sa/src/module_ipc/service.cpp b/services/backup_sa/src/module_ipc/service.cpp index b363d02d325d381b0735f5e68a498118f7928840..dc71733772b2a9a435c2ec8bd0a93b370793bd1f 100644 --- a/services/backup_sa/src/module_ipc/service.cpp +++ b/services/backup_sa/src/module_ipc/service.cpp @@ -812,7 +812,12 @@ void Service::NotifyCloneBundleFinish(std::string bundleName, const BackupRestor return; } if (session_->OnBundleFileReady(bundleName)) { - std::lock_guard lock(backupExtMutexMap_[bundleName]->callbackMutex); + std::shared_ptr mutexPtr = GetExtensionMutex(bundleName); + if (mutexPtr == nullptr) { + HILOGE("extension mutex ptr is nullptr"); + return; + } + std::lock_guard lock(mutexPtr->callbackMutex); auto backUpConnection = session_->GetExtConnection(bundleName); if (backUpConnection == nullptr) { throw BError(BError::Codes::SA_INVAL_ARG, "backUpConnection is empty"); @@ -938,7 +943,12 @@ void Service::ExtConnectDied(const string &callName) HITRACE_METER_NAME(HITRACE_TAG_FILEMANAGEMENT, __PRETTY_FUNCTION__); try { HILOGI("Begin, bundleName: %{public}s", callName.c_str()); - std::lock_guard lock(backupExtMutexMap_[callName]->callbackMutex); + std::shared_ptr mutexPtr = GetExtensionMutex(callName); + if (mutexPtr == nullptr) { + HILOGE("extension mutex ptr is nullptr"); + return; + } + std::lock_guard lock(mutexPtr->callbackMutex); /* Clear Timer */ session_->StopFwkTimer(callName); session_->StopExtTimer(callName); @@ -1856,6 +1866,20 @@ std::function Service::TimeOutCallback(wptr ptr, std::string bu }; } +void Service::TimeoutRadarReport(IServiceReverse::Scenario scenario, std::string &bundleName) +{ + int32_t errCode = BError(BError::Codes::EXT_ABILITY_TIMEOUT).GetCode(); + if (scenario == IServiceReverse::Scenario::BACKUP) { + AppRadar::Info info(bundleName, "", "on backup timeout"); + AppRadar::GetInstance().RecordBackupFuncRes(info, "Service::TimeOutCallback", GetUserIdDefault(), + BizStageBackup::BIZ_STAGE_ON_BACKUP, errCode); + } else if (scenario == IServiceReverse::Scenario::RESTORE) { + AppRadar::Info info(bundleName, "", "on restore timeout"); + AppRadar::GetInstance().RecordRestoreFuncRes(info, "Service::TimeOutCallback", GetUserIdDefault(), + BizStageRestore::BIZ_STAGE_ON_RESTORE, errCode); + } +} + void Service::DoTimeout(wptr ptr, std::string bundleName) { auto thisPtr = ptr.promote(); @@ -1869,18 +1893,14 @@ void Service::DoTimeout(wptr ptr, std::string bundleName) return; } IServiceReverse::Scenario scenario = sessionPtr->GetScenario(); - int32_t errCode = BError(BError::Codes::EXT_ABILITY_TIMEOUT).GetCode(); - if (scenario == IServiceReverse::Scenario::BACKUP) { - AppRadar::Info info(bundleName, "", "on backup timeout"); - AppRadar::GetInstance().RecordBackupFuncRes(info, "Service::TimeOutCallback", GetUserIdDefault(), - BizStageBackup::BIZ_STAGE_ON_BACKUP, errCode); - } else if (scenario == IServiceReverse::Scenario::RESTORE) { - AppRadar::Info info(bundleName, "", "on restore timeout"); - AppRadar::GetInstance().RecordRestoreFuncRes(info, "Service::TimeOutCallback", GetUserIdDefault(), - BizStageRestore::BIZ_STAGE_ON_RESTORE, errCode); - } + TimeoutRadarReport(scenario, bundleName); try { - std::lock_guard lock(backupExtMutexMap_[bundleName]->callbackMutex); + std::shared_ptr mutexPtr = GetExtensionMutex(bundleName); + if (mutexPtr == nullptr) { + HILOGE("extension mutex ptr is nullptr"); + return; + } + std::lock_guard lock(mutexPtr->callbackMutex); if (SAUtils::IsSABundleName(bundleName)) { auto sessionConnection = sessionPtr->GetSAExtConnection(bundleName); shared_ptr saConnection = sessionConnection.lock(); diff --git a/services/backup_sa/src/module_ipc/service_incremental.cpp b/services/backup_sa/src/module_ipc/service_incremental.cpp index ec2f4efeaabd7375047d9be730e8835d3ce2e0a2..8ce248bc3202327e777cc286d266f305e4e27825 100644 --- a/services/backup_sa/src/module_ipc/service_incremental.cpp +++ b/services/backup_sa/src/module_ipc/service_incremental.cpp @@ -86,14 +86,22 @@ ErrCode Service::Release() return BError(BError::Codes::OK); } -void Service::AddExtensionMutex(const BundleName &bundleName) +std::shared_ptr Service::GetExtensionMutex(const BundleName &bundleName) { - backupExtMutexMap_[bundleName] = std::make_shared(bundleName); + std::unique_lock lock(extensionMutexLock_); + auto it = backupExtMutexMap_.find(bundleName); + if (it == backupExtMutexMap_.end()) { + HILOGI("BackupExtMutexMap not contain %{public}s", bundleName.c_str()); + backupExtMutexMap_[bundleName] = std::make_shared(bundleName); + return backupExtMutexMap_[bundleName]; + } + HILOGI("BackupExtMutexMap contain %{public}s", bundleName.c_str()); + return it->second; } void Service::RemoveExtensionMutex(const BundleName &bundleName) { - std::unique_lock lock(extensionLock_); + std::unique_lock lock(extensionMutexLock_); auto it = backupExtMutexMap_.find(bundleName); if (it == backupExtMutexMap_.end()) { HILOGI("BackupExtMutexMap not contain %{public}s", bundleName.c_str()); @@ -499,7 +507,12 @@ ErrCode Service::AppIncrementalDone(ErrCode errCode) HILOGI("Service AppIncrementalDone start, callerName is %{public}s, errCode is: %{public}d", callerName.c_str(), errCode); if (session_->OnBundleFileReady(callerName) || errCode != BError(BError::Codes::OK)) { - std::lock_guard lock(backupExtMutexMap_[callerName]->callbackMutex); + std::shared_ptr mutexPtr = GetExtensionMutex(callerName); + if (mutexPtr == nullptr) { + HILOGE("extension mutex ptr is nullptr"); + return BError(BError::Codes::SA_INVAL_ARG, "Extension mutex ptr is null."); + } + std::lock_guard lock(mutexPtr->callbackMutex); auto tempBackUpConnection = session_->GetExtConnection(callerName); auto backUpConnection = tempBackUpConnection.promote(); if (backUpConnection == nullptr) { diff --git a/services/backup_sa/src/module_ipc/sub_service.cpp b/services/backup_sa/src/module_ipc/sub_service.cpp index 282247d0a5dfabc5254ed036579efda7df658464..2e85358892a75cf5745c1814d6f859a7981d6d29 100644 --- a/services/backup_sa/src/module_ipc/sub_service.cpp +++ b/services/backup_sa/src/module_ipc/sub_service.cpp @@ -185,7 +185,12 @@ ErrCode Service::AppDone(ErrCode errCode) string callerName = VerifyCallerAndGetCallerName(); HILOGI("Begin, callerName is: %{public}s, errCode: %{public}d", callerName.c_str(), errCode); if (session_->OnBundleFileReady(callerName) || errCode != BError(BError::Codes::OK)) { - std::lock_guard lock(backupExtMutexMap_[callerName]->callbackMutex); + std::shared_ptr mutexPtr = GetExtensionMutex(callerName); + if (mutexPtr == nullptr) { + HILOGE("extension mutex ptr is nullptr"); + return BError(BError::Codes::SA_INVAL_ARG); + } + std::lock_guard lock(mutexPtr->callbackMutex); auto backUpConnection = session_->GetExtConnection(callerName); if (backUpConnection == nullptr) { HILOGE("App finish error, backUpConnection is empty"); diff --git a/services/backup_sa/src/module_sched/sched_scheduler.cpp b/services/backup_sa/src/module_sched/sched_scheduler.cpp index fef2fa40306ded750999d64130642d637472f605..8d85731b3f3e1d6d8cd2a5f723f7bfbe75b03525 100644 --- a/services/backup_sa/src/module_sched/sched_scheduler.cpp +++ b/services/backup_sa/src/module_sched/sched_scheduler.cpp @@ -92,7 +92,6 @@ void SchedScheduler::ExecutingQueueTasks(const string &bundleName) lock.unlock(); // launch extension if (reversePtr_ != nullptr) { - reversePtr_->AddExtensionMutex(bundleName); ErrCode errCode = reversePtr_->LaunchBackupExtension(bundleName); if (errCode) { reversePtr_->ExtConnectFailed(bundleName, errCode); diff --git a/tests/mock/module_ipc/service_mock.cpp b/tests/mock/module_ipc/service_mock.cpp index d10f4476c27e8cc936ec7e54d023e472862cc4f0..55efa64a37bbdd2c240920b9da7ffc6e0553a8bf 100644 --- a/tests/mock/module_ipc/service_mock.cpp +++ b/tests/mock/module_ipc/service_mock.cpp @@ -254,8 +254,9 @@ void Service::ClearResidualBundleData(const std::string &bundleName) { } -void Service::AddExtensionMutex(const BundleName &bundleName) +std::shared_ptr Service::GetExtensionMutex(const BundleName &bundleName) { + return make_shared(bundleName); } void Service::RemoveExtensionMutex(const BundleName &bundleName) diff --git a/tests/unittests/backup_sa/module_ipc/service_incremental_test.cpp b/tests/unittests/backup_sa/module_ipc/service_incremental_test.cpp index 91f6f5be542bc9b2a2275b569d4629bcffeabeb8..dff5599bc07d449216a3689a1e1284e2dda3a3f8 100644 --- a/tests/unittests/backup_sa/module_ipc/service_incremental_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_incremental_test.cpp @@ -718,7 +718,6 @@ HWTEST_F(ServiceIncrementalTest, SUB_ServiceIncremental_AppIncrementalDone_0000, ret = service->AppIncrementalDone(errCode); EXPECT_EQ(ret, BError(BError::Codes::OK).GetCode()); - service->backupExtMutexMap_[""] = make_shared(""); EXPECT_CALL(*session, OnBundleFileReady(_, _)).WillOnce(Return(true)); EXPECT_CALL(*session, GetExtConnection(_)).WillOnce(Return(nullptr)); ret = service->AppIncrementalDone(errCode); diff --git a/tests/unittests/backup_sa/module_ipc/service_other_test.cpp b/tests/unittests/backup_sa/module_ipc/service_other_test.cpp index b7cbc6c73feeba0ac6ac7eee2835763487811335..ca02ebcaac18a7127db8b829d88914e209888725 100644 --- a/tests/unittests/backup_sa/module_ipc/service_other_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_other_test.cpp @@ -110,7 +110,10 @@ void Service::SendUserIdToApp(string&, int32_t) {} void Service::SetCurrentBackupSessProperties(const vector&, int32_t) {} -void Service::AddExtensionMutex(const BundleName&) {} +std::shared_ptr Service::GetExtensionMutex(const BundleName &bundleName) +{ + return make_shared(bundleName); +} void Service::RemoveExtensionMutex(const BundleName&) {} } @@ -969,7 +972,6 @@ HWTEST_F(ServiceTest, SUB_Service_AppDone_0200, TestSize.Level1) ASSERT_TRUE(service != nullptr); ErrCode errCode = BError(BError::Codes::SA_INVAL_ARG).GetCode(); - service->backupExtMutexMap_[""] = make_shared(""); EXPECT_CALL(*skeleton, GetCallingTokenID()).WillOnce(Return(0)); EXPECT_CALL(*token, GetTokenType(_)).WillOnce(Return(Security::AccessToken::ATokenTypeEnum::TOKEN_HAP)); EXPECT_CALL(*token, GetHapTokenInfo(_, _)).WillOnce(Return(0)); @@ -1021,7 +1023,6 @@ HWTEST_F(ServiceTest, SUB_Service_NotifyCloneBundleFinish_0100, TestSize.Level1) service->NotifyCloneBundleFinish(bundleName, senario); EXPECT_TRUE(true); - service->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); EXPECT_CALL(*session, OnBundleFileReady(_, _)).WillOnce(Return(true)); EXPECT_CALL(*session, GetExtConnection(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*session, IsOnAllBundlesFinished()).WillOnce(Return(false)); diff --git a/tests/unittests/backup_sa/module_ipc/service_test.cpp b/tests/unittests/backup_sa/module_ipc/service_test.cpp index 1d45f92dec74b28e3db7b55706022b0ec714e18d..7f7ef75eca9ed0b74ebad47b3b868f121e0427c5 100644 --- a/tests/unittests/backup_sa/module_ipc/service_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_test.cpp @@ -371,18 +371,15 @@ HWTEST_F(ServiceTest, SUB_Service_AppDone_0100, testing::ext::TestSize.Level1) try { GTEST_LOG_(INFO) << "SUB_Service_AppDone Branches Start"; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); auto ret = servicePtr_->AppDone(BError(BError::Codes::OK)); EXPECT_EQ(ret, BError(BError::Codes::OK)); GTEST_LOG_(INFO) << "SUB_Service_AppDone_0100 BACKUP"; ret = Init(IServiceReverse::Scenario::BACKUP); EXPECT_EQ(ret, BError(BError::Codes::OK)); GTEST_LOG_(INFO) << "ServiceTest-AppDone Branches"; - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->AppDone(1); EXPECT_EQ(ret, BError(BError::Codes::OK)); GTEST_LOG_(INFO) << "ServiceTest-AppDone Branches End"; - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->AppDone(BError(BError::Codes::OK)); EXPECT_EQ(ret, BError(BError::Codes::OK)); } catch (...) { @@ -407,13 +404,11 @@ HWTEST_F(ServiceTest, SUB_Service_AppDone_0101, testing::ext::TestSize.Level1) try { GTEST_LOG_(INFO) << "SUB_Service_AppDone Branches Start"; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); auto ret = servicePtr_->AppDone(BError(BError::Codes::OK)); EXPECT_EQ(ret, BError(BError::Codes::OK)); GTEST_LOG_(INFO) << "SUB_Service_AppDone_0101 RESTORE"; ret = Init(IServiceReverse::Scenario::RESTORE); EXPECT_EQ(ret, BError(BError::Codes::OK)); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->AppDone(BError(BError::Codes::OK)); EXPECT_EQ(ret, BError(BError::Codes::OK)); } catch (...) { @@ -439,7 +434,6 @@ HWTEST_F(ServiceTest, SUB_Service_AppDone_0102, testing::ext::TestSize.Level1) GTEST_LOG_(INFO) << "SUB_Service_AppDone Branches Start"; string bundleName = ""; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); auto ret = servicePtr_->AppDone(BError(BError::Codes::OK)); EXPECT_EQ(ret, BError(BError::Codes::OK)); } catch (...) { @@ -465,23 +459,18 @@ HWTEST_F(ServiceTest, SUB_Service_ServiceResultReport_0000, testing::ext::TestSi GTEST_LOG_(INFO) << "SUB_Service_ServiceResultReport Branches Start"; string bundleName = ""; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); auto ret = servicePtr_->ServiceResultReport("test", BackupRestoreScenario::FULL_RESTORE, 0); EXPECT_EQ(ret, BError(BError::Codes::OK)); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->ServiceResultReport("test", BackupRestoreScenario::INCREMENTAL_RESTORE, 0); EXPECT_EQ(ret, BError(BError::Codes::OK)); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->ServiceResultReport("test", BackupRestoreScenario::FULL_BACKUP, 0); EXPECT_EQ(ret, BError(BError::Codes::OK)); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->ServiceResultReport("test", BackupRestoreScenario::INCREMENTAL_BACKUP, 0); EXPECT_EQ(ret, BError(BError::Codes::OK)); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); ret = servicePtr_->ServiceResultReport("test", static_cast(1000), 0); EXPECT_EQ(ret, BError(BError::Codes::OK)); } catch (...) { @@ -626,13 +615,11 @@ HWTEST_F(ServiceTest, SUB_Service_OnBackupExtensionDied_0100, testing::ext::Test EXPECT_EQ(ret, BError(BError::Codes::OK)); string bundleName = BUNDLE_NAME; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); servicePtr_->OnBackupExtensionDied(move(bundleName)); GTEST_LOG_(INFO) << "SUB_Service_OnBackupExtensionDied_0100 BACKUP"; ret = Init(IServiceReverse::Scenario::BACKUP); EXPECT_EQ(ret, BError(BError::Codes::OK)); bundleName = BUNDLE_NAME; - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); servicePtr_->OnBackupExtensionDied(move(bundleName)); } catch (...) { EXPECT_TRUE(false); @@ -667,7 +654,6 @@ HWTEST_F(ServiceTest, SUB_Service_OnBackupExtensionDied_0101, testing::ext::Test impl_.backupExtNameMap[BUNDLE_NAME] = extInfo; impl_.scenario = IServiceReverse::Scenario::RESTORE; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[bundleName] = make_shared(bundleName); servicePtr_->OnBackupExtensionDied(move(bundleName)); GTEST_LOG_(INFO) << "SUB_Service_OnBackupExtensionDied_0101 BACKUP"; @@ -676,7 +662,6 @@ HWTEST_F(ServiceTest, SUB_Service_OnBackupExtensionDied_0101, testing::ext::Test impl_.restoreDataType = RESTORE_DATA_READDY; bundleName = "123456789"; impl_.backupExtNameMap[bundleName] = extInfo; - servicePtr_->backupExtMutexMap_[bundleName] = make_shared(bundleName); servicePtr_->OnBackupExtensionDied(move(bundleName)); } catch (...) { EXPECT_TRUE(false); @@ -1532,7 +1517,6 @@ HWTEST_F(ServiceTest, SUB_Service_NotifyCloneBundleFinish_0100, testing::ext::Te impl_.backupExtNameMap[BUNDLE_NAME] = extInfo; impl_.scenario = IServiceReverse::Scenario::RESTORE; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[BUNDLE_NAME] = make_shared(BUNDLE_NAME); servicePtr_->NotifyCloneBundleFinish(BUNDLE_NAME, BackupRestoreScenario::FULL_RESTORE); } catch (...) { EXPECT_TRUE(false); @@ -1615,9 +1599,7 @@ HWTEST_F(ServiceTest, SUB_Service_ExtConnectDied_0100, testing::ext::TestSize.Le impl_.backupExtNameMap[BUNDLE_NAME] = extInfo; impl_.scenario = IServiceReverse::Scenario::RESTORE; EXPECT_TRUE(servicePtr_ != nullptr); - servicePtr_->backupExtMutexMap_[callName] = make_shared(callName); servicePtr_->ExtConnectDied(callName); - servicePtr_->backupExtMutexMap_[callName] = make_shared(callName); extInfo.backUpConnection->isConnected_.store(true); servicePtr_->ExtConnectDied(callName); } catch (...) { diff --git a/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp b/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp index c0a136bba8dfeca55c83c1d118b9ef2f1de52b16..031be7b9afbed02d9a3cd58352296ebe6e040ac7 100644 --- a/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp @@ -528,7 +528,6 @@ HWTEST_F(ServiceThrowTest, SUB_Service_throw_OnBackupExtensionDied_0100, testing try { EXPECT_NE(service, nullptr); string bundleName; - service->backupExtMutexMap_["bundleName"] = make_shared("bundleName"); EXPECT_CALL(*sessionMock, GetScenario()).WillOnce(Return(IServiceReverse::Scenario::UNDEFINED)) .WillOnce(Return(IServiceReverse::Scenario::UNDEFINED)) .WillOnce(Invoke([]() {