From dfd093314d0c1a602905d3f62257acebc3bead03 Mon Sep 17 00:00:00 2001 From: g00613291 Date: Fri, 27 Sep 2024 18:42:33 +0800 Subject: [PATCH 1/3] fix appIncrementalDone crash Signed-off-by: g00613291 --- .../backup_sa/include/module_ipc/service.h | 24 +++++++++++++++++++ services/backup_sa/src/module_ipc/service.cpp | 7 +++++- .../src/module_ipc/service_incremental.cpp | 18 ++++++++++++++ .../backup_sa/src/module_ipc/sub_service.cpp | 2 ++ .../src/module_ipc/svc_backup_connection.cpp | 1 - .../src/module_sched/sched_scheduler.cpp | 1 + tests/mock/module_ipc/service_mock.cpp | 8 +++++++ 7 files changed, 59 insertions(+), 2 deletions(-) diff --git a/services/backup_sa/include/module_ipc/service.h b/services/backup_sa/include/module_ipc/service.h index f281efe3c..b550b30d0 100644 --- a/services/backup_sa/include/module_ipc/service.h +++ b/services/backup_sa/include/module_ipc/service.h @@ -32,6 +32,12 @@ #include "thread_pool.h" namespace OHOS::FileManagement::Backup { +struct extensionInfo { + std::string bundleName; + std::mutex callbackMutex; + extensionInfo(std::string bundleName_) : bundleName(bundleName_) {}; +}; + class Service : public SystemAbility, public ServiceStub, protected NoCopyable { DECLARE_SYSTEM_ABILITY(Service); @@ -273,6 +279,21 @@ public: */ void DelClearBundleRecord(const std::vector &bundleNames); + /** + * @brief 添加extension锁 + * + * @param bundleName 应用名称 + * + */ + void AddExtensionMutex(const BundleName &bundleName); + + /** + * @brief 清理extension锁 + * + * @param bundleName 应用名称 + * + */ + void RemoveExtensionMutex(const BundleName &bundleName); public: explicit Service(int32_t saID, bool runOnCreate = false) : SystemAbility(saID, runOnCreate) { @@ -503,6 +524,9 @@ private: friend class ServiceTest; OHOS::ThreadPool threadPool_; + std::mutex extensionLock_; +public: + 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 8d59dd804..7d20794f3 100644 --- a/services/backup_sa/src/module_ipc/service.cpp +++ b/services/backup_sa/src/module_ipc/service.cpp @@ -805,6 +805,7 @@ void Service::NotifyCloneBundleFinish(std::string bundleName, const BackupRestor return; } if (session_->OnBundleFileReady(bundleName)) { + std::lock_guard lock(backupExtMutexMap_[bundleName]->callbackMutex); auto backUpConnection = session_->GetExtConnection(bundleName); if (backUpConnection == nullptr) { throw BError(BError::Codes::SA_INVAL_ARG, "backUpConnection is empty"); @@ -819,6 +820,7 @@ void Service::NotifyCloneBundleFinish(std::string bundleName, const BackupRestor backUpConnection->DisconnectBackupExtAbility(); ClearSessionAndSchedInfo(bundleName); } + RemoveExtensionMutex(bundleName); SendEndAppGalleryNotify(bundleName); OnAllBundlesFinished(BError(BError::Codes::OK)); } catch (...) { @@ -923,6 +925,7 @@ 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); /* Clear Timer */ session_->StopFwkTimer(callName); session_->StopExtTimer(callName); @@ -942,8 +945,8 @@ void Service::ExtConnectDied(const string &callName) HILOGE("Unexpected exception, bundleName: %{public}s", callName.c_str()); ClearSessionAndSchedInfo(callName); NoticeClientFinish(callName, BError(BError::Codes::EXT_ABILITY_DIED)); - return; } + RemoveExtensionMutex(callName); } void Service::ExtStart(const string &bundleName) @@ -1866,6 +1869,7 @@ void Service::DoTimeout(wptr ptr, std::string bundleName) BizStageRestore::BIZ_STAGE_ON_RESTORE, errCode); } try { + std::lock_guard lock(backupExtMutexMap_[bundleName]->callbackMutex); if (SAUtils::IsSABundleName(bundleName)) { auto sessionConnection = sessionPtr->GetSAExtConnection(bundleName); shared_ptr saConnection = sessionConnection.lock(); @@ -1887,6 +1891,7 @@ void Service::DoTimeout(wptr ptr, std::string bundleName) thisPtr->ClearSessionAndSchedInfo(bundleName); thisPtr->NoticeClientFinish(bundleName, BError(BError::Codes::EXT_ABILITY_TIMEOUT)); } + RemoveExtensionMutex(bundleName); } void Service::AddClearBundleRecord(const std::string &bundleName) diff --git a/services/backup_sa/src/module_ipc/service_incremental.cpp b/services/backup_sa/src/module_ipc/service_incremental.cpp index ae0358d81..4ee35440f 100644 --- a/services/backup_sa/src/module_ipc/service_incremental.cpp +++ b/services/backup_sa/src/module_ipc/service_incremental.cpp @@ -85,6 +85,22 @@ ErrCode Service::Release() return BError(BError::Codes::OK); } +void Service::AddExtensionMutex(const BundleName &bundleName) +{ + backupExtMutexMap_[bundleName] = std::make_shared(bundleName); +} + +void Service::RemoveExtensionMutex(const BundleName &bundleName) +{ + std::unique_lock lock(extensionLock_); + auto it = backupExtMutexMap_.find(bundleName); + if (it == backupExtMutexMap_.end()) { + HILOGI("BackupExtMutexMap not contain %{public}s", bundleName.c_str()); + return; + } + backupExtMutexMap_.erase(it); +} + UniqueFd Service::GetLocalCapabilitiesIncremental(const std::vector &bundleNames) { HITRACE_METER_NAME(HITRACE_TAG_FILEMANAGEMENT, __PRETTY_FUNCTION__); @@ -484,6 +500,7 @@ 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); auto tempBackUpConnection = session_->GetExtConnection(callerName); auto backUpConnection = tempBackUpConnection.promote(); if (backUpConnection == nullptr) { @@ -500,6 +517,7 @@ ErrCode Service::AppIncrementalDone(ErrCode errCode) ClearSessionAndSchedInfo(callerName); NotifyCallerCurAppIncrementDone(errCode, callerName); } + RemoveExtensionMutex(callerName); OnAllBundlesFinished(BError(BError::Codes::OK)); return BError(BError::Codes::OK); } catch (const BError &e) { diff --git a/services/backup_sa/src/module_ipc/sub_service.cpp b/services/backup_sa/src/module_ipc/sub_service.cpp index b235f1443..0a6b15501 100644 --- a/services/backup_sa/src/module_ipc/sub_service.cpp +++ b/services/backup_sa/src/module_ipc/sub_service.cpp @@ -181,6 +181,7 @@ 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); auto backUpConnection = session_->GetExtConnection(callerName); if (backUpConnection == nullptr) { HILOGE("App finish error, backUpConnection is empty"); @@ -197,6 +198,7 @@ ErrCode Service::AppDone(ErrCode errCode) ClearSessionAndSchedInfo(callerName); NotifyCallerCurAppDone(errCode, callerName); } + RemoveExtensionMutex(callerName); OnAllBundlesFinished(BError(BError::Codes::OK)); return BError(BError::Codes::OK); } catch (const BError &e) { diff --git a/services/backup_sa/src/module_ipc/svc_backup_connection.cpp b/services/backup_sa/src/module_ipc/svc_backup_connection.cpp index 53413cc90..0492a8e4d 100644 --- a/services/backup_sa/src/module_ipc/svc_backup_connection.cpp +++ b/services/backup_sa/src/module_ipc/svc_backup_connection.cpp @@ -78,7 +78,6 @@ void SvcBackupConnection::OnAbilityDisconnectDone(const AppExecFwk::ElementName { HILOGI("called begin"); isConnected_.store(false); - backupProxy_ = nullptr; string bundleName = element.GetBundleName(); HILOGI("bundleName:%{public}s, OnAbilityDisconnectDone, bundleNameIndexInfo:%{public}s", bundleName.c_str(), bundleNameIndexInfo_.c_str()); diff --git a/services/backup_sa/src/module_sched/sched_scheduler.cpp b/services/backup_sa/src/module_sched/sched_scheduler.cpp index 8d85731b3..fef2fa403 100644 --- a/services/backup_sa/src/module_sched/sched_scheduler.cpp +++ b/services/backup_sa/src/module_sched/sched_scheduler.cpp @@ -92,6 +92,7 @@ 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 f762791b9..b45465b75 100644 --- a/tests/mock/module_ipc/service_mock.cpp +++ b/tests/mock/module_ipc/service_mock.cpp @@ -253,4 +253,12 @@ void Service::OnSARestore(const std::string &bundleName, const std::string &resu void Service::ClearResidualBundleData(const std::string &bundleName) { } + +void Service::AddExtensionMutex(const BundleName &bundleName) +{ +} + +void Service::RemoveExtensionMutex(const BundleName &bundleName) +{ +} } // namespace OHOS::FileManagement::Backup -- Gitee From 95a20cb638ecd06cd3998e9177e9c38d93dfebdf Mon Sep 17 00:00:00 2001 From: wangpggg Date: Tue, 15 Oct 2024 20:21:02 +0800 Subject: [PATCH 2/3] modif failed ut Signed-off-by: wangpeng --- .../backup_sa/module_ipc/service_test.cpp | 18 ++++++++++++++++++ .../module_ipc/service_throw_test.cpp | 1 + 2 files changed, 19 insertions(+) diff --git a/tests/unittests/backup_sa/module_ipc/service_test.cpp b/tests/unittests/backup_sa/module_ipc/service_test.cpp index 214931079..a49cdbd7f 100644 --- a/tests/unittests/backup_sa/module_ipc/service_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_test.cpp @@ -371,15 +371,18 @@ 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 (...) { @@ -404,11 +407,13 @@ 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 (...) { @@ -434,6 +439,7 @@ 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 (...) { @@ -459,18 +465,23 @@ 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 (...) { @@ -614,11 +625,13 @@ 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); @@ -653,6 +666,7 @@ 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"; @@ -661,6 +675,7 @@ 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); @@ -1515,6 +1530,7 @@ 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); @@ -1596,7 +1612,9 @@ 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 45729864b..01a7b4024 100644 --- a/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp @@ -526,6 +526,7 @@ 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([]() { -- Gitee From 313b715694e987b87f6ff15ac6a79e7fdf1a1f86 Mon Sep 17 00:00:00 2001 From: wangpggg Date: Fri, 18 Oct 2024 16:43:03 +0800 Subject: [PATCH 3/3] add ut Signed-off-by: wangpeng --- .../backup_sa/include/module_ipc/service.h | 14 +++--- services/backup_sa/src/module_ipc/service.cpp | 46 +++++++++++++------ .../src/module_ipc/service_incremental.cpp | 21 +++++++-- .../backup_sa/src/module_ipc/sub_service.cpp | 7 ++- .../src/module_sched/sched_scheduler.cpp | 1 - tests/mock/module_ipc/service_mock.cpp | 3 +- .../backup_sa/module_ipc/service_test.cpp | 18 -------- .../module_ipc/service_throw_test.cpp | 1 - 8 files changed, 66 insertions(+), 45 deletions(-) diff --git a/services/backup_sa/include/module_ipc/service.h b/services/backup_sa/include/module_ipc/service.h index b550b30d0..e46077d05 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 { @@ -280,12 +280,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锁 @@ -505,6 +505,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_; @@ -524,9 +526,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 7d20794f3..576548870 100644 --- a/services/backup_sa/src/module_ipc/service.cpp +++ b/services/backup_sa/src/module_ipc/service.cpp @@ -805,7 +805,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"); @@ -925,7 +930,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); @@ -1845,6 +1855,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(); @@ -1858,18 +1882,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 4ee35440f..1942a5aad 100644 --- a/services/backup_sa/src/module_ipc/service_incremental.cpp +++ b/services/backup_sa/src/module_ipc/service_incremental.cpp @@ -85,14 +85,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()); @@ -500,7 +508,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 0a6b15501..b3c4cd259 100644 --- a/services/backup_sa/src/module_ipc/sub_service.cpp +++ b/services/backup_sa/src/module_ipc/sub_service.cpp @@ -181,7 +181,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 fef2fa403..8d85731b3 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 b45465b75..cc295c7f4 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 nullptr; } void Service::RemoveExtensionMutex(const BundleName &bundleName) diff --git a/tests/unittests/backup_sa/module_ipc/service_test.cpp b/tests/unittests/backup_sa/module_ipc/service_test.cpp index a49cdbd7f..214931079 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 (...) { @@ -625,13 +614,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); @@ -666,7 +653,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"; @@ -675,7 +661,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); @@ -1530,7 +1515,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); @@ -1612,9 +1596,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 01a7b4024..45729864b 100644 --- a/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp +++ b/tests/unittests/backup_sa/module_ipc/service_throw_test.cpp @@ -526,7 +526,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([]() { -- Gitee