diff --git a/src/core/cm/alerts/alerts.cpp b/src/core/cm/alerts/alerts.cpp index 994461f0e..2242134a4 100644 --- a/src/core/cm/alerts/alerts.cpp +++ b/src/core/cm/alerts/alerts.cpp @@ -44,11 +44,12 @@ class SetTimestamp : public StaticVisitor { * Public **********************************************************************************************************************/ -Error Alerts::Init( - const alerts::Config& config, cm::alerts::SenderItf& sender, cloudconnection::CloudConnectionItf& cloudConnection) +Error Alerts::Init(AllocatorItf& allocator, const alerts::Config& config, cm::alerts::SenderItf& sender, + cloudconnection::CloudConnectionItf& cloudConnection) { LOG_DBG() << "Init alerts" << Log::Field("sendPeriod", config.mSendPeriod); + mAllocator = &allocator; mConfig = config; mSender = &sender; mCloudConnection = &cloudConnection; @@ -242,6 +243,9 @@ Error Alerts::SendAlerts() while (!mAlerts.IsEmpty()) { auto package = CreatePackage(); + if (!package) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } LOG_INF() << "Send alerts" << Log::Field("alertsCount", package->mItems.Size()); @@ -257,7 +261,12 @@ Error Alerts::SendAlerts() bool Alerts::IsDuplicated(const AlertVariant& alert) { - auto alertCopy = MakeUnique(&mAllocator, alert); + auto alertCopy = MakeUnique(mAllocator, alert); + if (!alertCopy) { + LOG_ERR() << "Can't allocate alert copy" << Log::Field(ErrorEnum::eNoMemory); + + return false; + } return mAlerts.FindIf([&alertCopy](const AlertVariant& item) { alertCopy->ApplyVisitor(SetTimestamp(item.ApplyVisitor(GetTimestamp()))); @@ -268,7 +277,12 @@ bool Alerts::IsDuplicated(const AlertVariant& alert) UniquePtr Alerts::CreatePackage() { - auto package = MakeUnique(&mAllocator); + auto package = MakeUnique(mAllocator); + if (!package) { + LOG_ERR() << "Can't allocate alerts package" << Log::Field(ErrorEnum::eNoMemory); + + return package; + } const auto count = Min(cAlertItemsCount, mAlerts.Size()); diff --git a/src/core/cm/alerts/alerts.hpp b/src/core/cm/alerts/alerts.hpp index 67c02d33a..177a4dbeb 100644 --- a/src/core/cm/alerts/alerts.hpp +++ b/src/core/cm/alerts/alerts.hpp @@ -40,12 +40,13 @@ class Alerts : public ReceiverItf, /** * Initializes alerts. * + * @param allocator allocator to use for temporary objects. * @param config configuration object. * @param sender alerts sender object. * @param cloudConnection cloud connection. * @return Error. */ - Error Init(const alerts::Config& config, cm::alerts::SenderItf& sender, + Error Init(AllocatorItf& allocator, const alerts::Config& config, cm::alerts::SenderItf& sender, cloudconnection::CloudConnectionItf& cloudConnection); /** @@ -96,7 +97,6 @@ class Alerts : public ReceiverItf, Error UnsubscribeListener(AlertsListenerItf& listener) override; private: - static constexpr auto cAllocatorSize = sizeof(AlertVariant) + sizeof(aos::Alerts); static constexpr auto cListenersMaxCount = 4; static constexpr auto cAlertTagsCount = static_cast(AlertTagEnum::eNumAlertTags); @@ -111,7 +111,7 @@ class Alerts : public ReceiverItf, void ShrinkCache(size_t count); void NotifyListeners(const AlertVariant& alert); - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; alerts::Config mConfig; cm::alerts::SenderItf* mSender {}; cloudconnection::CloudConnectionItf* mCloudConnection {}; diff --git a/src/core/cm/alerts/tests/alerts.cpp b/src/core/cm/alerts/tests/alerts.cpp index d5c42fd12..64a3f3ebe 100644 --- a/src/core/cm/alerts/tests/alerts.cpp +++ b/src/core/cm/alerts/tests/alerts.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include @@ -125,6 +126,10 @@ class AlertsTest : public Test { protected: void SetUp() override { tests::utils::InitLog(); } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + alerts::Config mConfig {Time::cSeconds * 1}; SenderStub mCommunication; cloudconnection::CloudConnectionMock mCloudConnection; @@ -159,7 +164,7 @@ TEST_F(AlertsTest, DuplicatesAreSkipped) return ErrorEnum::eNone; })); - auto err = mAlerts->Init(mConfig, mCommunication, mCloudConnection); + auto err = mAlerts->Init(mAllocator, mConfig, mCommunication, mCloudConnection); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mAlerts->Start(); @@ -198,16 +203,16 @@ TEST_F(AlertsTest, AlertIsSkippedIfBufferIsFull) std::string message; + auto err = mAlerts->Init(mAllocator, mConfig, mCommunication, mCloudConnection); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + for (size_t i = 0; i < cAlertsCacheSize; ++i) { auto alert = CreateCoreAlert(cTime, "node1", std::to_string(i)); - auto err = mAlerts->OnAlertReceived(*alert); + err = mAlerts->OnAlertReceived(*alert); EXPECT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); } - auto err = mAlerts->Init(mConfig, mCommunication, mCloudConnection); - ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); - err = mAlerts->OnAlertReceived(*CreateCoreAlert(cTime, "node1", "skipped alert")); EXPECT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); @@ -264,7 +269,7 @@ TEST_F(AlertsTest, PackagesAreSent) mConfig.mSendPeriod = Time::cSeconds * 3; - auto err = mAlerts->Init(mConfig, mCommunication, mCloudConnection); + auto err = mAlerts->Init(mAllocator, mConfig, mCommunication, mCloudConnection); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mAlerts->Start(); @@ -311,7 +316,7 @@ TEST_F(AlertsTest, PackagesAreSentOnReconnect) std::vector receivedPackages; - auto err = mAlerts->Init(mConfig, mCommunication, mCloudConnection); + auto err = mAlerts->Init(mAllocator, mConfig, mCommunication, mCloudConnection); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mAlerts->Start(); @@ -353,7 +358,7 @@ TEST_F(AlertsTest, ListenersAreNotified) AlertTagEnum::eCoreAlert, }; - auto err = mAlerts->Init(mConfig, mCommunication, mCloudConnection); + auto err = mAlerts->Init(mAllocator, mConfig, mCommunication, mCloudConnection); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mAlerts->SubscribeListener( diff --git a/src/core/cm/imagemanager/imagemanager.cpp b/src/core/cm/imagemanager/imagemanager.cpp index 9f2c46bb7..9ec61bdbe 100644 --- a/src/core/cm/imagemanager/imagemanager.cpp +++ b/src/core/cm/imagemanager/imagemanager.cpp @@ -14,14 +14,15 @@ namespace aos::cm::imagemanager { * Public **********************************************************************************************************************/ -Error ImageManager::Init(const Config& config, StorageItf& storage, BlobInfoProviderItf& blobInfoProvider, - spaceallocator::SpaceAllocatorItf& downloadingSpaceAllocator, +Error ImageManager::Init(AllocatorItf& allocator, const Config& config, StorageItf& storage, + BlobInfoProviderItf& blobInfoProvider, spaceallocator::SpaceAllocatorItf& downloadingSpaceAllocator, spaceallocator::SpaceAllocatorItf& installSpaceAllocator, downloader::DownloaderItf& downloader, fileserver::FileServerItf& fileserver, crypto::CryptoHelperItf& cryptoHelper, fs::FileInfoProviderItf& fileInfoProvider, oci::OCISpecItf& ociSpec) { LOG_DBG() << "Init image manager"; + mAllocator = &allocator; mConfig = config; mStorage = &storage; mBlobInfoProvider = &blobInfoProvider; @@ -33,7 +34,7 @@ Error ImageManager::Init(const Config& config, StorageItf& storage, BlobInfoProv mFileInfoProvider = &fileInfoProvider; mOCISpec = &ociSpec; - auto items = MakeUnique>(&mAllocator); + auto items = MakeUnique>(mAllocator); if (!items) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -130,7 +131,7 @@ Error ImageManager::DownloadUpdateItems(const Array& itemsInfo, statuses[i].mError = ErrorEnum::eNone; } - auto storedItems = MakeUnique>(&mAllocator); + auto storedItems = MakeUnique>(mAllocator); if (!storedItems) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -220,7 +221,7 @@ Error ImageManager::InstallUpdateItems(const Array& itemsInfo, A statuses[i].mError = ErrorEnum::eNone; } - auto storedItems = MakeUnique>(&mAllocator); + auto storedItems = MakeUnique>(mAllocator); if (!storedItems) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -300,7 +301,7 @@ Error ImageManager::GetUpdateItemsStatuses(Array& statuses) LOG_DBG() << "Get update items statuses"; - auto items = MakeUnique>(&mAllocator); + auto items = MakeUnique>(mAllocator); if (!items) { return ErrorEnum::eNoMemory; } @@ -358,7 +359,7 @@ Error ImageManager::GetIndexDigest(const String& itemID, const String& version, LOG_DBG() << "Get index digest" << Log::Field("itemID", itemID) << Log::Field("version", version); - auto items = MakeUnique>(&mAllocator); + auto items = MakeUnique>(mAllocator); if (!items) { return ErrorEnum::eNoMemory; } @@ -429,7 +430,7 @@ Error ImageManager::GetItemCurrentVersion(const String& itemID, String& version) LOG_DBG() << "Get item current version" << Log::Field("itemID", itemID); - auto items = MakeUnique>(&mAllocator); + auto items = MakeUnique>(mAllocator); if (!items) { return ErrorEnum::eNoMemory; } @@ -458,7 +459,7 @@ RetWithError ImageManager::RemoveItem(const String& id, const String& ve LOG_DBG() << "Remove item" << Log::Field("id", id) << Log::Field("version", version); - auto storedItems = MakeUnique>(&mAllocator); + auto storedItems = MakeUnique>(mAllocator); if (!storedItems) { return {0, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; } @@ -508,7 +509,7 @@ Error ImageManager::RemoveOutdatedItems() LOG_DBG() << "Remove outdated items"; - auto items = MakeUnique>(&mAllocator); + auto items = MakeUnique>(mAllocator); if (!items) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -586,7 +587,7 @@ Error ImageManager::AllocateSpaceForPartialDownloads() auto fileName = fileIterator->mPath; auto filePath = fs::JoinPath(algorithmDir, fileName); - auto [fileSize, sizeErr] = fs::CalculateSize(filePath); + auto [fileSize, sizeErr] = fs::CalculateSize(*mAllocator, filePath); if (!sizeErr.IsNone()) { LOG_WRN() << "Failed to get size for partial download" << Log::Field("path", filePath) << Log::Field(sizeErr); @@ -884,7 +885,7 @@ Error ImageManager::DownloadItem(const UpdateItemInfo& itemInfo, const Array(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); if (!imageIndex) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -898,7 +899,7 @@ Error ImageManager::DownloadItem(const UpdateItemInfo& itemInfo, const ArraymManifests.Size()); for (const auto& manifestDescriptor : imageIndex->mManifests) { - auto manifest = MakeUnique(&mAllocator); + auto manifest = MakeUnique(mAllocator); if (!manifest) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1080,7 +1081,11 @@ Error ImageManager::EnsureBlob(const String& digest, const String& downloadPath, { LOG_DBG() << "Ensure blob" << Log::Field("digest", digest); - auto blobInfo = MakeUnique(&mAllocator); + auto blobInfo = MakeUnique(mAllocator); + if (!blobInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + UniquePtr downloadingSpace; do { @@ -1132,7 +1137,7 @@ Error ImageManager::GetBlobInfo(const String& digest, BlobInfo& blobInfo) return AOS_ERROR_WRAP(err); } - auto blobsInfo = MakeUnique>(&mAllocator); + auto blobsInfo = MakeUnique>(mAllocator); if (!blobsInfo) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1190,7 +1195,7 @@ Error ImageManager::CheckExistingBlob(const String& installPath) return AOS_ERROR_WRAP(err); } - auto expectedSHA256 = MakeUnique>(&mAllocator); + auto expectedSHA256 = MakeUnique>(mAllocator); if (!expectedSHA256) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1223,7 +1228,7 @@ Error ImageManager::PrepareDownloadSpace(const String& downloadPath, const BlobI partialDownloadSize = 0; if (downloadExists) { - auto [dirSize, getSizeErr] = fs::CalculateSize(downloadPath); + auto [dirSize, getSizeErr] = fs::CalculateSize(*mAllocator, downloadPath); if (!getSizeErr.IsNone()) { return AOS_ERROR_WRAP(getSizeErr); } @@ -1275,7 +1280,7 @@ Error ImageManager::PerformDownload(const BlobInfo& blobInfo, const String& down << Log::Field("path", downloadPath) << Log::Field(AOS_ERROR_WRAP(err)); if (err = WaitForStop(); !err.IsNone()) { - auto [newPartialSize, retrySizeErr] = fs::CalculateSize(downloadPath); + auto [newPartialSize, retrySizeErr] = fs::CalculateSize(*mAllocator, downloadPath); if (!retrySizeErr.IsNone()) { LOG_WRN() << "Failed to get partial download size" << Log::Field("path", downloadPath) << Log::Field(retrySizeErr); @@ -1514,7 +1519,7 @@ Error ImageManager::VerifyBlobChecksum(const String& digest, const fs::FileInfo& return AOS_ERROR_WRAP(err); } - auto expectedSHA256 = MakeUnique>(&mAllocator); + auto expectedSHA256 = MakeUnique>(mAllocator); if (!expectedSHA256) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1543,7 +1548,7 @@ Error ImageManager::VerifyItemBlobs(const String& indexDigest) return AOS_ERROR_WRAP(err); } - auto imageIndex = MakeUnique(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); if (!imageIndex) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1562,7 +1567,7 @@ Error ImageManager::VerifyItemBlobs(const String& indexDigest) return AOS_ERROR_WRAP(err); } - auto manifest = MakeUnique(&mAllocator); + auto manifest = MakeUnique(mAllocator); if (!manifest) { return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } @@ -1605,7 +1610,7 @@ bool ImageManager::IsBlobUsedByItems(const String& blobDigest, const Array(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); if (!imageIndex) { continue; } @@ -1625,7 +1630,7 @@ bool ImageManager::IsBlobUsedByItems(const String& blobDigest, const Array(&mAllocator); + auto manifest = MakeUnique(mAllocator); if (!manifest) { continue; } @@ -1659,7 +1664,7 @@ RetWithError ImageManager::CleanupOrphanedBlobs() size_t totalSize = 0; - auto storedItems = MakeUnique>(&mAllocator); + auto storedItems = MakeUnique>(mAllocator); if (!storedItems) { return {0, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; } @@ -1685,7 +1690,7 @@ RetWithError ImageManager::CleanupOrphanedBlobs() if (!IsBlobUsedByItems(blobDigest, *storedItems)) { auto filePath = fs::JoinPath(algorithmDir, hash); - auto [blobSize, sizeErr] = fs::CalculateSize(filePath); + auto [blobSize, sizeErr] = fs::CalculateSize(*mAllocator, filePath); if (!sizeErr.IsNone()) { LOG_WRN() << "Failed to get blob size" << Log::Field("path", filePath) << Log::Field(sizeErr); } else { diff --git a/src/core/cm/imagemanager/imagemanager.hpp b/src/core/cm/imagemanager/imagemanager.hpp index f72698ba6..0f8271f41 100644 --- a/src/core/cm/imagemanager/imagemanager.hpp +++ b/src/core/cm/imagemanager/imagemanager.hpp @@ -36,6 +36,7 @@ class ImageManager : public ImageManagerItf, public ItemInfoProviderItf, public /** * Initializes image manager. * + * @param allocator allocator to use for temporary objects. * @param config image manager config. * @param storage stores internal persistent data. * @param blobInfoProvider retrieves blobs info. @@ -48,8 +49,8 @@ class ImageManager : public ImageManagerItf, public ItemInfoProviderItf, public * @param ociSpec parses OCI spec files. * @return Error. */ - Error Init(const Config& config, StorageItf& storage, BlobInfoProviderItf& blobInfoProvider, - spaceallocator::SpaceAllocatorItf& downloadingSpaceAllocator, + Error Init(AllocatorItf& allocator, const Config& config, StorageItf& storage, + BlobInfoProviderItf& blobInfoProvider, spaceallocator::SpaceAllocatorItf& downloadingSpaceAllocator, spaceallocator::SpaceAllocatorItf& installSpaceAllocator, downloader::DownloaderItf& downloader, fileserver::FileServerItf& fileserver, crypto::CryptoHelperItf& cryptoHelper, fs::FileInfoProviderItf& fileInfoProvider, oci::OCISpecItf& ociSpec); @@ -253,10 +254,8 @@ class ImageManager : public ImageManagerItf, public ItemInfoProviderItf, public ConditionalVariable mCondVar; bool mCancel {}; bool mInProgress {}; - mutable StaticAllocator<(sizeof(StaticArray) * 2) + sizeof(oci::ImageIndex) - + sizeof(oci::ImageManifest) + sizeof(StaticArray) - + sizeof(StaticArray) + sizeof(BlobInfo)> - mAllocator; + + AllocatorItf* mAllocator {}; }; } // namespace aos::cm::imagemanager diff --git a/src/core/cm/imagemanager/tests/imagemanager.cpp b/src/core/cm/imagemanager/tests/imagemanager.cpp index 283890ada..95245bab2 100644 --- a/src/core/cm/imagemanager/tests/imagemanager.cpp +++ b/src/core/cm/imagemanager/tests/imagemanager.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "mocks/blobinfoprovidermock.hpp" #include "mocks/itemstatuslistenermock.hpp" @@ -50,7 +51,7 @@ class ImageManagerTest : public testing::Test { EXPECT_CALL(mStorageMock, GetAllItemsInfos(_)).WillRepeatedly(Return(ErrorEnum::eNone)); EXPECT_TRUE(mImageManager - .Init(mConfig, mStorageMock, mBlobInfoProviderMock, mDownloadingSpaceAllocatorMock, + .Init(mAllocator, mConfig, mStorageMock, mBlobInfoProviderMock, mDownloadingSpaceAllocatorMock, mInstallSpaceAllocatorMock, mDownloaderMock, mFileServerMock, mCryptoHelperMock, mFileInfoProviderMock, mOCISpecMock) .IsNone()); @@ -62,9 +63,12 @@ class ImageManagerTest : public testing::Test { fs::RemoveAll(mConfig.mDownloadPath); } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + Config mConfig; ImageManager mImageManager; - StaticAllocator<1024 * 5, 20> mAllocator; StrictMock mStorageMock; StrictMock mBlobInfoProviderMock; StrictMock mDownloadingSpaceAllocatorMock; @@ -213,6 +217,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_Success_NewItem) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -222,6 +228,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_Success_NewItem) EXPECT_CALL(mInstallSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -339,6 +347,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_AlreadyInstalled) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -348,6 +358,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_AlreadyInstalled) EXPECT_CALL(mInstallSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -522,6 +534,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_MultipleItems_Success) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -531,6 +545,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_MultipleItems_Success) EXPECT_CALL(mInstallSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -670,6 +686,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_Cancel_DownloadFailed) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); testing::Mock::AllowLeak(space.Get()); @@ -759,6 +777,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_RemovesOldPendingVersion) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -768,6 +788,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_RemovesOldPendingVersion) EXPECT_CALL(mInstallSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -876,6 +898,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_RemovesOldFailedVersion) EXPECT_CALL(mDownloadingSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); @@ -885,6 +909,8 @@ TEST_F(ImageManagerTest, DownloadUpdateItems_RemovesOldFailedVersion) EXPECT_CALL(mInstallSpaceAllocatorMock, AllocateSpace(_)) .WillRepeatedly(Invoke([this](size_t) -> RetWithError> { auto space = MakeUnique(&mAllocator); + EXPECT_TRUE(space); + EXPECT_CALL(*space, Accept()).Times(AtLeast(0)); EXPECT_CALL(*space, Release()).Times(AtLeast(0)); diff --git a/src/core/cm/launcher/balancer.cpp b/src/core/cm/launcher/balancer.cpp index 6c1d15138..1e5ea27db 100644 --- a/src/core/cm/launcher/balancer.cpp +++ b/src/core/cm/launcher/balancer.cpp @@ -14,9 +14,10 @@ namespace aos::cm::launcher { * Public **********************************************************************************************************************/ -void Balancer::Init(InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider, NodeManager& nodeManager, - MonitoringProviderItf& monitorProvider, InstanceRunnerItf& runner) +void Balancer::Init(AllocatorItf& allocator, InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider, + NodeManager& nodeManager, MonitoringProviderItf& monitorProvider, InstanceRunnerItf& runner) { + mAllocator = &allocator; mInstanceManager = &instanceManager; mImageInfoProvider = &imageInfoProvider; mNodeManager = &nodeManager; @@ -91,12 +92,20 @@ Error Balancer::PerformNodeBalancing(Array>& instances) continue; } - auto imageIndex = MakeUnique(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); + if (!imageIndex) { + LOG_ERR() << "Can't allocate image index" << Log::Field("instance", id) << Log::Field(ErrorEnum::eNoMemory); + + mInstanceManager->ScheduleInstance(instance, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)); + + continue; + } if (auto err = mImageInfoProvider->GetImageIndex(id.mItemID, info.mVersion, *imageIndex); !err.IsNone()) { LOG_ERR() << "Can't get images" << Log::Field("instance", id) << Log::Field(err); mInstanceManager->ScheduleInstance(instance, AOS_ERROR_WRAP(err)); + continue; } @@ -106,8 +115,7 @@ Error Balancer::PerformNodeBalancing(Array>& instances) LOG_DBG() << "Try to schedule instance" << Log::Field("instance", id) << Log::Field("manifest", manifest.mDigest); - scheduleErr = ScheduleInstance(instance, manifest); - if (scheduleErr.IsNone()) { + if (scheduleErr = ScheduleInstance(instance, manifest); scheduleErr.IsNone()) { LOG_DBG() << "Instance scheduled successfully" << Log::Field("nodeID", info.mNodeID); break; @@ -126,7 +134,10 @@ Error Balancer::PerformNodeBalancing(Array>& instances) Error Balancer::ScheduleInstance(SharedPtr& instance, const oci::IndexContentDescriptor& imageDescriptor) { - auto nodes = MakeUnique>(&mAllocator); + auto nodes = MakeUnique>(mAllocator); + if (!nodes) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto releaseConfigs = DeferRelease(reinterpret_cast(1), [&](int*) { instance->ResetConfigs(); }); @@ -196,7 +207,10 @@ void Balancer::FilterNodesByResources(Instance& instance, Array& nodes) RetWithError> Balancer::SelectRuntime(Instance& instance, const Array& nodes) { - auto nodeRuntimes = MakeUnique(&mAllocator); + auto nodeRuntimes = MakeUnique(mAllocator); + if (!nodeRuntimes) { + return {nullptr, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } if (auto err = CreateRuntimes(nodes, *nodeRuntimes); !err.IsNone()) { return {nullptr, AOS_ERROR_WRAP(err)}; @@ -384,7 +398,10 @@ void Balancer::FilterTopPriorityNodes(NodeRuntimes& nodes) Error Balancer::PerformPolicyBalancing(Array>& instances) { - auto imageIndex = MakeUnique(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); + if (!imageIndex) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } for (auto& instance : instances) { const auto& info = instance->GetInfo(); @@ -465,7 +482,10 @@ Error Balancer::UpdateMonitoringData(bool isInitialUpdate) for (auto& node : mNodeManager->GetNodes()) { const auto& nodeID = node.GetInfo().mNodeID; - auto nodeMonitoring = MakeUnique(&mAllocator); + auto nodeMonitoring = MakeUnique(mAllocator); + if (!nodeMonitoring) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } // Monitoring data immediately after startup is not availble. // Assign zero consumption on start. diff --git a/src/core/cm/launcher/balancer.hpp b/src/core/cm/launcher/balancer.hpp index 9d0846b49..bf22bf06e 100644 --- a/src/core/cm/launcher/balancer.hpp +++ b/src/core/cm/launcher/balancer.hpp @@ -7,6 +7,8 @@ #ifndef AOS_CORE_CM_LAUNCHER_BALANCER_HPP_ #define AOS_CORE_CM_LAUNCHER_BALANCER_HPP_ +#include + #include "itf/instancerunner.hpp" #include "itf/launcher.hpp" #include "itf/monitoringprovider.hpp" @@ -29,14 +31,15 @@ class Balancer { /** * Initializes runner with required managers and providers. * + * @param allocator allocator to use for temporary objects. * @param instanceManager instance manager. * @param imageInfoProvider image info provider. * @param nodeManager node manager. * @param monitorProvider monitoring provider. * @param runner instance runner interface. */ - void Init(InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider, NodeManager& nodeManager, - MonitoringProviderItf& monitorProvider, InstanceRunnerItf& runner); + void Init(AllocatorItf& allocator, InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider, + NodeManager& nodeManager, MonitoringProviderItf& monitorProvider, InstanceRunnerItf& runner); /** * Runs instances. @@ -57,13 +60,6 @@ class Balancer { private: using NodeRuntimes = StaticMap, cMaxNumInstances>; - static constexpr size_t cScheduleInstanceSize - = sizeof(oci::ImageIndex) + sizeof(StaticArray) + sizeof(NodeRuntimes); - static constexpr size_t cPolicyBalancingSize = sizeof(oci::ImageIndex); - static constexpr size_t cMonitoringSize = sizeof(monitoring::NodeMonitoringData); - - static constexpr size_t cAllocatorSize = Max(cScheduleInstanceSize, cPolicyBalancingSize, cMonitoringSize); - Error PerformNodeBalancing(Array>& instances); Error ScheduleInstance(SharedPtr& instance, const oci::IndexContentDescriptor& imageDescriptor); @@ -99,7 +95,7 @@ class Balancer { InstanceRunnerItf* mRunner {}; SubjectArray mSubjects; - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; }; /** @}*/ diff --git a/src/core/cm/launcher/imageinfoprovider.cpp b/src/core/cm/launcher/imageinfoprovider.cpp index 7198363e0..d45b4c355 100644 --- a/src/core/cm/launcher/imageinfoprovider.cpp +++ b/src/core/cm/launcher/imageinfoprovider.cpp @@ -11,17 +11,30 @@ namespace aos::cm::launcher { -void ImageInfoProvider::Init(imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec) +void ImageInfoProvider::Init( + AllocatorItf& allocator, imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec) { + mAllocator = &allocator; mItemInfoProvider = &itemInfoProvider; mOCISpec = &ociSpec; } Error ImageInfoProvider::GetImageConfig(const oci::IndexContentDescriptor& imageDescriptor, oci::ImageConfig& config) { - auto manifestPath = MakeUnique>(&mAllocator); - auto manifest = MakeUnique(&mAllocator); - auto configPath = MakeUnique>(&mAllocator); + auto manifestPath = MakeUnique>(mAllocator); + if (!manifestPath) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto manifest = MakeUnique(mAllocator); + if (!manifest) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto configPath = MakeUnique>(mAllocator); + if (!configPath) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mItemInfoProvider->GetBlobPath(imageDescriptor.mDigest, *manifestPath); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -44,9 +57,20 @@ Error ImageInfoProvider::GetImageConfig(const oci::IndexContentDescriptor& image Error ImageInfoProvider::GetItemConfig(const oci::IndexContentDescriptor& imageDescriptor, oci::ItemConfig& itemConfig) { - auto manifestPath = MakeUnique>(&mAllocator); - auto manifest = MakeUnique(&mAllocator); - auto servicePath = MakeUnique>(&mAllocator); + auto manifestPath = MakeUnique>(mAllocator); + if (!manifestPath) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto manifest = MakeUnique(mAllocator); + if (!manifest) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto servicePath = MakeUnique>(mAllocator); + if (!servicePath) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mItemInfoProvider->GetBlobPath(imageDescriptor.mDigest, *manifestPath); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -73,8 +97,15 @@ Error ImageInfoProvider::GetItemConfig(const oci::IndexContentDescriptor& imageD Error ImageInfoProvider::GetImageIndex(const String& itemID, const String& version, oci::ImageIndex& imageIndex) { - auto indexDigest = MakeUnique>(&mAllocator); - auto indexPath = MakeUnique>(&mAllocator); + auto indexDigest = MakeUnique>(mAllocator); + if (!indexDigest) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto indexPath = MakeUnique>(mAllocator); + if (!indexPath) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mItemInfoProvider->GetIndexDigest(itemID, version, *indexDigest); !err.IsNone()) { return AOS_ERROR_WRAP(err); diff --git a/src/core/cm/launcher/imageinfoprovider.hpp b/src/core/cm/launcher/imageinfoprovider.hpp index 0d54417eb..37dbb2878 100644 --- a/src/core/cm/launcher/imageinfoprovider.hpp +++ b/src/core/cm/launcher/imageinfoprovider.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -25,10 +26,11 @@ class ImageInfoProvider { /** * @brief Initializes image info provider. * + * @param allocator allocator to use for temporary objects. * @param itemInfoProvider item info provider. * @param ociSpec OCI spec. */ - void Init(imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec); + void Init(AllocatorItf& allocator, imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec); /** * Returns OCI image config for the specified instance/image identifiers. @@ -59,13 +61,10 @@ class ImageInfoProvider { Error GetImageIndex(const String& itemID, const String& version, oci::ImageIndex& imageIndex); private: - static constexpr auto cAllocatorSize = Max(sizeof(oci::ImageManifest) + sizeof(StaticString) * 3, - sizeof(StaticString) + sizeof(StaticString)); - imagemanager::ItemInfoProviderItf* mItemInfoProvider {}; oci::OCISpecItf* mOCISpec {}; - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; }; } // namespace aos::cm::launcher diff --git a/src/core/cm/launcher/instance.cpp b/src/core/cm/launcher/instance.cpp index 48fbf62a7..a360a65c0 100644 --- a/src/core/cm/launcher/instance.cpp +++ b/src/core/cm/launcher/instance.cpp @@ -16,7 +16,7 @@ namespace aos::cm::launcher { **********************************************************************************************************************/ Instance::Instance( - const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider, Allocator& allocator) + AllocatorItf& allocator, const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider) : mInfo(info) , mStorage(storage) , mImageInfoProvider(imageInfoProvider) @@ -37,8 +37,15 @@ Instance::Instance( Error Instance::LoadConfigs(const oci::IndexContentDescriptor& imageDescriptor) { - mItemConfig = MakeUnique(&mAllocator); + mItemConfig = MakeUnique(&mAllocator); + if (!mItemConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + mImageConfig = MakeUnique(&mAllocator); + if (!mImageConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto releaseConfigs = DeferRelease(reinterpret_cast(1), [&](int*) { ResetConfigs(); }); if (auto err = mImageInfoProvider.GetItemConfig(imageDescriptor, *mItemConfig); !err.IsNone()) { @@ -74,6 +81,11 @@ bool Instance::IsImageValid() } auto imageIndex = MakeUnique(&mAllocator); + if (!imageIndex) { + LOG_ERR() << "Can't allocate image index" << Log::Field(ErrorEnum::eNoMemory); + + return false; + } auto err = mImageInfoProvider.GetImageIndex(mInfo.mInstanceIdent.mItemID, mInfo.mVersion, *imageIndex); if (!err.IsNone()) { @@ -196,6 +208,9 @@ bool Instance::AreNodeLabelsOk(const LabelsArray& nodeLabels) RetWithError Instance::OverrideEnvVars(const OverrideEnvVarsRequest& envVars) { auto newEnvVars = MakeUnique(&mAllocator); + if (!newEnvVars) { + return {false, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } for (const auto& item : envVars.mItems) { if (!item.Match(mInfo.mInstanceIdent)) { @@ -280,8 +295,8 @@ Error Instance::SetDefaultRuntimes() **********************************************************************************************************************/ ComponentInstance::ComponentInstance( - const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider, Allocator& allocator) - : Instance(info, storage, imageInfoProvider, allocator) + AllocatorItf& allocator, const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider) + : Instance(allocator, info, storage, imageInfoProvider) { } @@ -377,9 +392,9 @@ Error ComponentInstance::LoadSMInfo(NodeItf& node, const String& runtimeID) * ServiceInstance implementation **********************************************************************************************************************/ -ServiceInstance::ServiceInstance(const InstanceInfo& info, UIDPool& uidPool, GIDPool& gidPool, StorageItf& storage, - StorageState& storageState, ImageInfoProvider& imageInfoProvider, Allocator& allocator) - : Instance(info, storage, imageInfoProvider, allocator) +ServiceInstance::ServiceInstance(AllocatorItf& allocator, const InstanceInfo& info, UIDPool& uidPool, GIDPool& gidPool, + StorageItf& storage, StorageState& storageState, ImageInfoProvider& imageInfoProvider) + : Instance(allocator, info, storage, imageInfoProvider) , mUIDPool(uidPool) , mGIDPool(gidPool) , mStorageState(storageState) diff --git a/src/core/cm/launcher/instance.hpp b/src/core/cm/launcher/instance.hpp index 156b69724..46893231e 100644 --- a/src/core/cm/launcher/instance.hpp +++ b/src/core/cm/launcher/instance.hpp @@ -35,12 +35,13 @@ class Instance { /** * Constructs instance. * + * @param allocator instance allocator. * @param info instance information. * @param storage interface to persistent storage. * @param imageInfoProvider interface for retrieving service information from image. - * @param allocator instance allocator. */ - Instance(const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider, Allocator& allocator); + Instance( + AllocatorItf& allocator, const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider); /** * Destructor. @@ -236,7 +237,7 @@ class Instance { StorageItf& mStorage; ImageInfoProvider& mImageInfoProvider; - Allocator& mAllocator; + AllocatorItf& mAllocator; MonitoringData mMonitoringData; @@ -252,13 +253,13 @@ class ComponentInstance : public Instance { /** * Constructs component instance. * + * @param allocator instance allocator. * @param info instance information. * @param storage interface to persistent storage. * @param imageInfoProvider interface for retrieving service information from image. - * @param allocator instance allocator. */ ComponentInstance( - const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider, Allocator& allocator); + AllocatorItf& allocator, const InstanceInfo& info, StorageItf& storage, ImageInfoProvider& imageInfoProvider); /** * Initializes component instance. @@ -343,14 +344,14 @@ class ServiceInstance : public Instance { /** * Constructs service instance. * + * @param allocator instance allocator. * @param info instance information. * @param uidPool pool for managing user identifiers. * @param storage interface to persistent storage. * @param storageState interface for managing storage and state partitions. - * @param allocator instance allocator. */ - ServiceInstance(const InstanceInfo& info, UIDPool& uidPool, GIDPool& gidPool, StorageItf& storage, - StorageState& storageState, ImageInfoProvider& imageInfoProvider, Allocator& allocator); + ServiceInstance(AllocatorItf& allocator, const InstanceInfo& info, UIDPool& uidPool, GIDPool& gidPool, + StorageItf& storage, StorageState& storageState, ImageInfoProvider& imageInfoProvider); /** * Initializes service instance. diff --git a/src/core/cm/launcher/instancemanager.cpp b/src/core/cm/launcher/instancemanager.cpp index c4f024be2..b042b89f8 100644 --- a/src/core/cm/launcher/instancemanager.cpp +++ b/src/core/cm/launcher/instancemanager.cpp @@ -38,15 +38,17 @@ Error InstanceManager::RemoveInstances(Array>& instances, Pr * Public **********************************************************************************************************************/ -Error InstanceManager::Init(const Config& config, imagemanager::ItemInfoProviderItf& itemInfoProvider, - storagestate::StorageStateItf& storageState, oci::OCISpecItf& ociSpec, IdentifierPoolValidator gidValidator, - IdentifierPoolValidator uidValidator, StorageItf& storage) +Error InstanceManager::Init(AllocatorItf& allocator, const Config& config, + imagemanager::ItemInfoProviderItf& itemInfoProvider, storagestate::StorageStateItf& storageState, + oci::OCISpecItf& ociSpec, IdentifierPoolValidator gidValidator, IdentifierPoolValidator uidValidator, + StorageItf& storage) { - mConfig = config; - mStorage = &storage; + mAllocator = &allocator; + mConfig = config; + mStorage = &storage; - mImageInfoProvider.Init(itemInfoProvider, ociSpec); - mStorageState.Init(storageState); + mImageInfoProvider.Init(allocator, itemInfoProvider, ociSpec); + mStorageState.Init(allocator, storageState); if (auto err = mUIDPool.Init(uidValidator); !err.IsNone()) { return err; @@ -218,6 +220,9 @@ RetWithError> InstanceManager::CreateInstance(const RunInsta } auto instanceInfo = CreateInfo(id, "", "", request); + if (!instanceInfo) { + return {nullptr, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } if (auto err = mStorage->AddInstance(*instanceInfo); !err.IsNone()) { return {nullptr, AOS_ERROR_WRAP(err)}; @@ -241,6 +246,9 @@ RetWithError> InstanceManager::CreateInstance(const RunInsta auto id = InstanceIdent {request.mItemID, request.mSubjectInfo.mSubjectID, index, request.mUpdateItemType}; auto instanceInfo = CreateInfo(id, nodeID, runtimeID, request); + if (!instanceInfo) { + return {nullptr, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } if (auto err = mStorage->AddInstance(*instanceInfo); !err.IsNone()) { return {nullptr, AOS_ERROR_WRAP(err)}; @@ -415,7 +423,11 @@ Error InstanceManager::LoadInstancesFromStorage() mActiveInstances.Clear(); mCachedInstances.Clear(); - auto instances = MakeUnique>(&mAllocator); + auto instances = MakeUnique>(mAllocator); + if (!instances) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + if (auto err = mStorage->LoadActiveInstances(*instances); !err.IsNone()) { return AOS_ERROR_WRAP(err); } @@ -549,18 +561,21 @@ RetWithError> InstanceManager::CreateInstance(const Instance switch (info.mInstanceIdent.mType.GetValue()) { case UpdateItemTypeEnum::eService: newInstance = MakeShared( - &mAllocator, info, mUIDPool, mGIDPool, *mStorage, mStorageState, mImageInfoProvider, mInstanceAllocator); + mAllocator, *mAllocator, info, mUIDPool, mGIDPool, *mStorage, mStorageState, mImageInfoProvider); break; case UpdateItemTypeEnum::eComponent: - newInstance - = MakeShared(&mAllocator, info, *mStorage, mImageInfoProvider, mInstanceAllocator); + newInstance = MakeShared(mAllocator, *mAllocator, info, *mStorage, mImageInfoProvider); break; default: return {{}, AOS_ERROR_WRAP(ErrorEnum::eNotSupported)}; } + if (!newInstance) { + return {nullptr, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } + if (auto err = newInstance->Init(); !err.IsNone()) { // Do not leave invalid instance in storage. if (auto rmErr = newInstance->Remove(); !rmErr.IsNone()) { @@ -722,7 +737,12 @@ uint64_t InstanceManager::FindIndexForNewInstance(const String& itemID, const St UniquePtr InstanceManager::CreateInfo( const InstanceIdent& id, const String& nodeID, const String& runtimeID, const RunInstanceRequest& request) { - auto info = MakeUnique(&mAllocator); + auto info = MakeUnique(mAllocator); + if (!info) { + LOG_ERR() << "Can't allocate instance info" << Log::Field(ErrorEnum::eNoMemory); + + return info; + } info->mInstanceIdent = id; info->mManifestDigest = ""; diff --git a/src/core/cm/launcher/instancemanager.hpp b/src/core/cm/launcher/instancemanager.hpp index 29827465c..25187f88e 100644 --- a/src/core/cm/launcher/instancemanager.hpp +++ b/src/core/cm/launcher/instancemanager.hpp @@ -34,6 +34,7 @@ class InstanceManager { /** * Initializes the instance manager with configuration and required interfaces. * + * @param allocator allocator to use for temporary objects. * @param config Configuration object. * @param imageInfoProvider Interface for retrieving service information from images. * @param storageState Interface for managing storage and state partitions. @@ -42,7 +43,7 @@ class InstanceManager { * @param storage Interface to persistent storage. * @return Error. */ - Error Init(const Config& config, imagemanager::ItemInfoProviderItf& itemInfoProvider, + Error Init(AllocatorItf& allocator, const Config& config, imagemanager::ItemInfoProviderItf& itemInfoProvider, storagestate::StorageStateItf& storageState, oci::OCISpecItf& ociSpec, IdentifierPoolValidator gidValidator, IdentifierPoolValidator uidValidator, StorageItf& storage); @@ -250,15 +251,6 @@ class InstanceManager { private: static constexpr auto cRemovePeriod = Time::cDay; - // LoadInstancesFromStorage: 1 StaticArray alive throughout loop - // + up to cMaxNumInstances instances. CreateInstance(RunInstanceRequest): up to (cMaxNumInstances-1) - // existing instances + 1 InstanceInfo (CreateInfo) + 1 new instance. Both paths peak at - // cMaxNumInstances+1 simultaneous allocations. - static constexpr auto cAllocatorSize = sizeof(StaticArray) - + Max(sizeof(ComponentInstance), sizeof(ServiceInstance)) * cMaxNumInstances * 2; - static constexpr auto cMaxNumAllocations = 2 * cMaxNumInstances + 1; - static constexpr auto cInstanceAllocatorSize = sizeof(oci::ImageConfig) + sizeof(oci::ItemConfig) - + sizeof(InstanceStatus) + sizeof(oci::ImageIndex) + sizeof(EnvVarArray); Error SetStatus(const InstanceStatus& status); Error SetStatus(Array& statuses, const InstanceStatus& status); @@ -305,8 +297,7 @@ class InstanceManager { Timer mCleanInstancesTimer; Timer mInitTimer; - StaticAllocator mAllocator; - StaticAllocator mInstanceAllocator; + AllocatorItf* mAllocator {}; StaticArray, cMaxNumInstances> mActiveInstances; StaticArray, cMaxNumInstances> mScheduledInstances; diff --git a/src/core/cm/launcher/launcher.cpp b/src/core/cm/launcher/launcher.cpp index b2cf1dde6..98f4c9f0b 100644 --- a/src/core/cm/launcher/launcher.cpp +++ b/src/core/cm/launcher/launcher.cpp @@ -33,8 +33,9 @@ class ShouldRebalanceVisitor : public StaticVisitor { * Public **********************************************************************************************************************/ -Error Launcher::Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, - InstanceRunnerItf& runner, imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec, +Error Launcher::Init(AllocatorItf& allocator, const Config& config, + nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, InstanceRunnerItf& runner, + imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec, unitconfig::NodeConfigProviderItf& nodeConfigProvider, storagestate::StorageStateItf& storageState, MonitoringProviderItf& monitorProvider, alerts::AlertsProviderItf& alertsProvider, iamclient::IdentProviderItf& identProvider, IdentifierPoolValidator gidValidator, @@ -42,6 +43,7 @@ Error Launcher::Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf { LOG_DBG() << "Init Launcher"; + mAllocator = &allocator; mConfig = config; mStorage = &storage; mNodeInfoProvider = &nodeInfoProvider; @@ -53,17 +55,17 @@ Error Launcher::Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf mIdentProvider = &identProvider; mSender = &sender; - auto err - = mInstanceManager.Init(config, itemInfoProvider, storageState, ociSpec, gidValidator, uidValidator, storage); + auto err = mInstanceManager.Init( + allocator, config, itemInfoProvider, storageState, ociSpec, gidValidator, uidValidator, storage); if (!err.IsNone()) { return AOS_ERROR_WRAP(err); } - mImageInfoProvider.Init(itemInfoProvider, ociSpec); + mImageInfoProvider.Init(allocator, itemInfoProvider, ociSpec); - mRunRequestsLoader.Init(storage, mInstanceManager, mImageInfoProvider); - mNodeManager.Init(*mNodeInfoProvider, *mNodeConfigProvider, *mRunner, mOverrideEnvVarsProcessor); - mBalancer.Init(mInstanceManager, mImageInfoProvider, mNodeManager, *mMonitorProvider, *mRunner); + mRunRequestsLoader.Init(allocator, storage, mInstanceManager, mImageInfoProvider); + mNodeManager.Init(allocator, *mNodeInfoProvider, *mNodeConfigProvider, *mRunner, mOverrideEnvVarsProcessor); + mBalancer.Init(allocator, mInstanceManager, mImageInfoProvider, mNodeManager, *mMonitorProvider, *mRunner); if (err = mOverrideEnvVarsProcessor.Init(config, storage, sender, *this); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -109,7 +111,10 @@ Error Launcher::Start() } // Set initial subjects list. - auto subjects = MakeUnique(&mAllocator); + auto subjects = MakeUnique(mAllocator); + if (!subjects) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mIdentProvider->GetSubjects(*subjects); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -300,7 +305,13 @@ void Launcher::UpdateInstanceStatuses() const auto totalSize = activeInstances.Size() + preinstalledComponents.Size(); // Copy old statuses. - auto oldInstanceStatuses = MakeUnique>(&mAllocator); + auto oldInstanceStatuses = MakeUnique>(mAllocator); + if (!oldInstanceStatuses) { + LOG_ERR() << "Failed to allocate old instance statuses" << Log::Field(AOS_ERROR_WRAP(ErrorEnum::eNoMemory)); + + return; + } + if (auto err = oldInstanceStatuses->Assign(mInstanceStatuses); !err.IsNone()) { LOG_ERR() << "Failed to copy old instance statuses" << Log::Field(AOS_ERROR_WRAP(err)); @@ -340,7 +351,12 @@ void Launcher::UpdateInstanceStatuses() } // Find new statuses. - auto changedStatuses = MakeUnique>(&mAllocator); + auto changedStatuses = MakeUnique>(mAllocator); + if (!changedStatuses) { + LOG_ERR() << "Failed to allocate changed statuses" << Log::Field(AOS_ERROR_WRAP(ErrorEnum::eNoMemory)); + + return; + } for (size_t i = 0; i < mInstanceStatuses.Size(); ++i) { auto newStatus = !oldInstanceStatuses->Contains(mInstanceStatuses[i]); @@ -388,7 +404,11 @@ Error Launcher::BalanceInstances(UniqueLock& lock, bool rebalance) LOG_DBG() << "Balance instances" << Log::Field("rebalance", rebalance); // Create instances from run requests. - auto instances = MakeUnique, cMaxNumInstances>>(&mAllocator); + auto instances = MakeUnique, cMaxNumInstances>>(mAllocator); + if (!instances) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + mRunRequestsLoader.CreateInstances(mNodeManager.GetNodes(), *instances); auto runErr = mBalancer.RunInstances(lock, *instances, rebalance); diff --git a/src/core/cm/launcher/launcher.hpp b/src/core/cm/launcher/launcher.hpp index f717d5d61..b82383a63 100644 --- a/src/core/cm/launcher/launcher.hpp +++ b/src/core/cm/launcher/launcher.hpp @@ -47,6 +47,7 @@ class Launcher : public LauncherItf, /** * Initializes launcher object instance. * + * @param allocator allocator to use for temporary objects. * @param config configuration. * @param nodeInfoProvider interface providing information about all unit nodes. * @param runner instance runner interface. @@ -63,8 +64,8 @@ class Launcher : public LauncherItf, * @param sender sender interface. * @return Error. */ - Error Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, InstanceRunnerItf& runner, - imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec, + Error Init(AllocatorItf& allocator, const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, + InstanceRunnerItf& runner, imagemanager::ItemInfoProviderItf& itemInfoProvider, oci::OCISpecItf& ociSpec, unitconfig::NodeConfigProviderItf& nodeConfigProvider, storagestate::StorageStateItf& storageState, MonitoringProviderItf& monitorProvider, alerts::AlertsProviderItf& alertsProvider, iamclient::IdentProviderItf& identProvider, IdentifierPoolValidator gidValidator, @@ -137,8 +138,6 @@ class Launcher : public LauncherItf, private: static constexpr auto cMaxNumInstanceStatusListeners = 8; - static constexpr auto cAllocatorSize = 2 * sizeof(StaticArray) - + sizeof(StaticArray, cMaxNumInstances>); void SendRunStatus(); @@ -169,6 +168,7 @@ class Launcher : public LauncherItf, void OnOverrideEnvVarsChanged() override; // External dependencies + AllocatorItf* mAllocator {}; Config mConfig; StorageItf* mStorage {}; nodeinfoprovider::NodeInfoProviderItf* mNodeInfoProvider {}; @@ -208,7 +208,6 @@ class Launcher : public LauncherItf, Mutex mBalancingMutex; ConditionalVariable mAllNodesConnectedCondVar; bool mIsRunning {}; - StaticAllocator mAllocator; }; /** @}*/ diff --git a/src/core/cm/launcher/node.cpp b/src/core/cm/launcher/node.cpp index 4eb880a0d..c4be15573 100644 --- a/src/core/cm/launcher/node.cpp +++ b/src/core/cm/launcher/node.cpp @@ -31,12 +31,12 @@ auto FilterByNode(const Array>& array, const String& nodeID) * Public **********************************************************************************************************************/ -void Node::Init(const String& id, unitconfig::NodeConfigProviderItf& nodeConfigProvider, - InstanceRunnerItf& instanceRunner, Allocator* allocator) +void Node::Init(AllocatorItf& allocator, const String& id, unitconfig::NodeConfigProviderItf& nodeConfigProvider, + InstanceRunnerItf& instanceRunner) { mNodeConfigProvider = &nodeConfigProvider; mInstanceRunner = &instanceRunner; - mAllocator = allocator; + mAllocator = &allocator; mInfo.mNodeID = id; mInfo.mState = NodeStateEnum::eUnprovisioned; @@ -268,8 +268,15 @@ Error Node::ReserveResources(const InstanceIdent& instanceIdent, const String& r Error Node::SendScheduledInstances( const Array>& scheduledInstances, const Array& runningInstances) { - auto stopInstances = MakeUnique>(mAllocator); + auto stopInstances = MakeUnique>(mAllocator); + if (!stopInstances) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + auto startInstances = MakeUnique>(mAllocator); + if (!startInstances) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } for (const auto& status : FilterActiveNodeInstances(runningInstances, mInfo.mNodeID)) { // Check if the instance is scheduled on this node (ident, runtime, node, and service version must match). @@ -318,8 +325,16 @@ Error Node::SendScheduledInstances( RetWithError Node::ResendInstances( const Array>& activeInstances, const Array& runningInstances, bool forceRestart) { - auto stopInstances = MakeUnique>(mAllocator); - auto startInstances = MakeUnique>(mAllocator); + auto stopInstances = MakeUnique>(mAllocator); + if (!stopInstances) { + return {false, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } + + auto startInstances = MakeUnique>(mAllocator); + if (!startInstances) { + return {false, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } + size_t runningNodeInstances = 0; for (const auto& status : FilterActiveNodeInstances(runningInstances, mInfo.mNodeID)) { diff --git a/src/core/cm/launcher/node.hpp b/src/core/cm/launcher/node.hpp index 9b69e7f2d..aae04776f 100644 --- a/src/core/cm/launcher/node.hpp +++ b/src/core/cm/launcher/node.hpp @@ -30,13 +30,13 @@ class Node : public NodeItf { /** * Initializes node. * + * @param allocator allocator. * @param info node information. * @param nodeConfigProvider node config provider. * @param instanceRunner instance runner interface. - * @param allocator allocator. */ - void Init(const String& id, unitconfig::NodeConfigProviderItf& nodeConfigProvider, - InstanceRunnerItf& instanceRunner, Allocator* allocator); + void Init(AllocatorItf& allocator, const String& id, unitconfig::NodeConfigProviderItf& nodeConfigProvider, + InstanceRunnerItf& instanceRunner); /** * Prepares node for balancing. @@ -196,7 +196,7 @@ class Node : public NodeItf { StaticMap, size_t, cMaxNumNodeRuntimes> mRuntimeAvailableCPU; StaticMap, size_t, cMaxNumNodeResources> mMaxInstances; - Allocator* mAllocator {}; + AllocatorItf* mAllocator {}; }; /** @}*/ diff --git a/src/core/cm/launcher/nodemanager.cpp b/src/core/cm/launcher/nodemanager.cpp index e35b345fa..a6dfefe07 100644 --- a/src/core/cm/launcher/nodemanager.cpp +++ b/src/core/cm/launcher/nodemanager.cpp @@ -23,10 +23,11 @@ auto FilterActiveNodes(Array& array) * Public **********************************************************************************************************************/ -void NodeManager::Init(nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, +void NodeManager::Init(AllocatorItf& allocator, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, unitconfig::NodeConfigProviderItf& nodeConfigProvider, InstanceRunnerItf& runner, OverrideEnvVarsProcessor& overrideEnvVarsProcessor) { + mAllocator = &allocator; mNodeInfoProvider = &nodeInfoProvider; mNodeConfigProvider = &nodeConfigProvider; mRunner = &runner; @@ -35,7 +36,10 @@ void NodeManager::Init(nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, Error NodeManager::Start() { - auto nodes = MakeUnique, cMaxNumNodes>>(&mAllocator); + auto nodes = MakeUnique, cMaxNumNodes>>(mAllocator); + if (!nodes) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mNodeInfoProvider->GetAllNodeIDs(*nodes); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -43,7 +47,10 @@ Error NodeManager::Start() LOG_DBG() << "Start node manager" << Log::Field("nodes", nodes->Size()); - auto nodeInfo = MakeUnique(&mAllocator); + auto nodeInfo = MakeUnique(mAllocator); + if (!nodeInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } for (const auto& nodeID : *nodes) { if (auto err = mNodeInfoProvider->GetNodeInfo(nodeID, *nodeInfo); !err.IsNone()) { @@ -59,7 +66,7 @@ Error NodeManager::Start() // Add online provisioned node mNodes.EmplaceBack(); - mNodes.Back().Init(nodeInfo->mNodeID, *mNodeConfigProvider, *mRunner, &mNodeAllocator); + mNodes.Back().Init(*mAllocator, nodeInfo->mNodeID, *mNodeConfigProvider, *mRunner); mNodes.Back().UpdateInfo(*nodeInfo); } @@ -105,11 +112,19 @@ Error NodeManager::LoadSMDataForActiveInstances( if (node == nullptr) { LOG_ERR() << "Can't find node" << Log::Field("instanceID", instanceID) << Log::Field("nodeID", nodeID) << Log::Field(AOS_ERROR_WRAP(ErrorEnum::eNotFound)); + + continue; + } + + auto imageDescriptor = MakeUnique(mAllocator); + if (!imageDescriptor) { + LOG_ERR() << "Can't allocate image descriptor" << Log::Field("instanceID", instanceID) + << Log::Field(AOS_ERROR_WRAP(ErrorEnum::eNoMemory)); + continue; } - auto imageDescriptor = MakeUnique(&mAllocator); - auto findDescErr = FindImageDescriptor( + auto findDescErr = FindImageDescriptor( instanceID.mItemID, instance->GetInfo().mVersion, manifestDigest, imageInfoProvider, *imageDescriptor); if (!findDescErr.IsNone()) { LOG_ERR() << "Can't find image descriptor" << Log::Field("instanceID", instanceID) @@ -146,7 +161,7 @@ Error NodeManager::NotifyNodeStatusReceived(const String& nodeID) return AOS_ERROR_WRAP(err); } - mNodes.Back().Init(nodeID, *mNodeConfigProvider, *mRunner, &mNodeAllocator); + mNodes.Back().Init(*mAllocator, nodeID, *mNodeConfigProvider, *mRunner); node = FindNode(nodeID); } @@ -337,7 +352,7 @@ bool NodeManager::UpdateNodeInfo(const UnitNodeInfo& info) return false; } - mNodes.Back().Init(info.mNodeID, *mNodeConfigProvider, *mRunner, &mNodeAllocator); + mNodes.Back().Init(*mAllocator, info.mNodeID, *mNodeConfigProvider, *mRunner); mNodes.Back().UpdateInfo(info); return true; @@ -351,7 +366,10 @@ bool NodeManager::UpdateNodeInfo(const UnitNodeInfo& info) Error NodeManager::FindImageDescriptor(const String& itemID, const String& version, const String& manifestDigest, ImageInfoProvider& imageInfoProvider, oci::IndexContentDescriptor& imageDescriptor) { - auto imageIndex = MakeUnique(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); + if (!imageIndex) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = imageInfoProvider.GetImageIndex(itemID, version, *imageIndex); !err.IsNone()) { return AOS_ERROR_WRAP(err); diff --git a/src/core/cm/launcher/nodemanager.hpp b/src/core/cm/launcher/nodemanager.hpp index baf7367d5..441fe7384 100644 --- a/src/core/cm/launcher/nodemanager.hpp +++ b/src/core/cm/launcher/nodemanager.hpp @@ -9,7 +9,7 @@ #include #include -#include +#include #include #include "node.hpp" @@ -29,12 +29,13 @@ class NodeManager { /** * Initializes node manager. * + * @param allocator allocator to use for temporary objects. * @param nodeInfoProvider node info provider. * @param nodeConfigProvider node config provider. * @param runner instance runner interface. * @param overrideEnvVarsProcessor override env vars processor. */ - void Init(nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, + void Init(AllocatorItf& allocator, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, unitconfig::NodeConfigProviderItf& nodeConfigProvider, InstanceRunnerItf& runner, OverrideEnvVarsProcessor& overrideEnvVarsProcessor); @@ -144,11 +145,6 @@ class NodeManager { private: static constexpr auto cStatusUpdateTimeout = Time::cMinutes * 10; - static constexpr auto cAllocatorSize - = sizeof(StaticArray, cMaxNumNodes>) + sizeof(UnitNodeInfo); - - static constexpr auto cNodeAllocatorSize = sizeof(StaticArray) * 2; - Error FindImageDescriptor(const String& itemID, const String& version, const String& manifestDigest, ImageInfoProvider& imageInfoProvider, oci::IndexContentDescriptor& imageDescriptor); @@ -159,8 +155,7 @@ class NodeManager { InstanceRunnerItf* mRunner {}; OverrideEnvVarsProcessor* mOverrideEnvVarsProcessor {}; - StaticAllocator mAllocator; - StaticAllocator mNodeAllocator; + AllocatorItf* mAllocator {}; StaticArray mNodes; diff --git a/src/core/cm/launcher/runrequestsloader.cpp b/src/core/cm/launcher/runrequestsloader.cpp index cf865001c..49752ac24 100644 --- a/src/core/cm/launcher/runrequestsloader.cpp +++ b/src/core/cm/launcher/runrequestsloader.cpp @@ -10,9 +10,10 @@ namespace aos::cm::launcher { -void RunRequestsLoader::Init( - StorageItf& storage, InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider) +void RunRequestsLoader::Init(AllocatorItf& allocator, StorageItf& storage, InstanceManager& instanceManager, + ImageInfoProvider& imageInfoProvider) { + mAllocator = &allocator; mStorage = &storage; mInstanceManager = &instanceManager; mImageInfoProvider = &imageInfoProvider; @@ -88,14 +89,24 @@ void RunRequestsLoader::CreateInstances(const Array& nodes, Array& nodes, Array>& instances) { - auto imageIndex = MakeUnique(&mAllocator); + auto imageIndex = MakeUnique(mAllocator); + if (!imageIndex) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mImageInfoProvider->GetImageIndex(request.mItemID, request.mVersion, *imageIndex); !err.IsNone()) { return AOS_ERROR_WRAP(err); } - auto combinedRuntimes = MakeUnique(&mAllocator); - auto itemConfig = MakeUnique(&mAllocator); + auto combinedRuntimes = MakeUnique(mAllocator); + if (!combinedRuntimes) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto itemConfig = MakeUnique(mAllocator); + if (!itemConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = CombinedRuntimes(*imageIndex, *combinedRuntimes, *itemConfig); !err.IsNone()) { return AOS_ERROR_WRAP(err); diff --git a/src/core/cm/launcher/runrequestsloader.hpp b/src/core/cm/launcher/runrequestsloader.hpp index 5211fe372..56b3cd4b2 100644 --- a/src/core/cm/launcher/runrequestsloader.hpp +++ b/src/core/cm/launcher/runrequestsloader.hpp @@ -7,6 +7,8 @@ #ifndef AOS_CORE_CM_LAUNCHER_RUNREQUESTSLOADER_HPP_ #define AOS_CORE_CM_LAUNCHER_RUNREQUESTSLOADER_HPP_ +#include + #include "instancemanager.hpp" #include "itf/storage.hpp" #include "node.hpp" @@ -21,11 +23,13 @@ class RunRequestsLoader { /** * Initializes the loader. * + * @param allocator allocator to use for temporary objects. * @param storage storage interface. * @param instanceManager instance manager. * @param imageInfoProvider image info provider. */ - void Init(StorageItf& storage, InstanceManager& instanceManager, ImageInfoProvider& imageInfoProvider); + void Init(AllocatorItf& allocator, StorageItf& storage, InstanceManager& instanceManager, + ImageInfoProvider& imageInfoProvider); /** * Saves run requests to internal buffer and to storage. @@ -55,9 +59,6 @@ class RunRequestsLoader { using CombinedRuntimesArray = StaticArray, oci::cMaxNumManifests * oci::cMaxNumRunners>; - static constexpr auto cCombinedRuntimesSize = sizeof(CombinedRuntimesArray); - static constexpr auto cAllocatorSize = sizeof(oci::ImageIndex) + cCombinedRuntimesSize + sizeof(oci::ItemConfig); - Error GenerateInstances( const RunInstanceRequest& request, const Array& nodes, Array>& instances); Error CombinedRuntimes( @@ -71,7 +72,7 @@ class RunRequestsLoader { StaticArray mRunRequests; - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; }; } // namespace aos::cm::launcher diff --git a/src/core/cm/launcher/storagestate.cpp b/src/core/cm/launcher/storagestate.cpp index a3f1f259f..f19ef0037 100644 --- a/src/core/cm/launcher/storagestate.cpp +++ b/src/core/cm/launcher/storagestate.cpp @@ -14,8 +14,9 @@ namespace aos::cm::launcher { * Public **********************************************************************************************************************/ -void StorageState::Init(storagestate::StorageStateItf& storageState) +void StorageState::Init(AllocatorItf& allocator, storagestate::StorageStateItf& storageState) { + mAllocator = &allocator; mStorageStateManager = &storageState; } @@ -38,10 +39,14 @@ Error StorageState::PrepareForBalancing() mAvailableStorage.Reset(); if (mStorageStateManager->IsSamePartition()) { - mAvailableState = mAvailableStorage = MakeShared(&mAllocator, 0); + mAvailableState = mAvailableStorage = MakeShared(mAllocator, 0); } else { - mAvailableState = MakeShared(&mAllocator, 0); - mAvailableStorage = MakeShared(&mAllocator, 0); + mAvailableState = MakeShared(mAllocator, 0); + mAvailableStorage = MakeShared(mAllocator, 0); + } + + if (!mAvailableState || !mAvailableStorage) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); } const auto& [stateSize, stateErr] = mStorageStateManager->GetTotalStateSize(); diff --git a/src/core/cm/launcher/storagestate.hpp b/src/core/cm/launcher/storagestate.hpp index 70556a094..dec07d82c 100644 --- a/src/core/cm/launcher/storagestate.hpp +++ b/src/core/cm/launcher/storagestate.hpp @@ -8,7 +8,7 @@ #define AOS_CORE_CM_LAUNCHER_STORAGESTATE_HPP_ #include -#include +#include namespace aos::cm::launcher { @@ -21,9 +21,10 @@ class StorageState { /** * Initializes storage state. * + * @param allocator allocator to use for temporary objects. * @param storageState storage state interface. */ - void Init(storagestate::StorageStateItf& storageState); + void Init(AllocatorItf& allocator, storagestate::StorageStateItf& storageState); /** * Starts storage state. @@ -77,13 +78,11 @@ class StorageState { size_t requestedStorageSize, size_t requestedStateSize, String& storagePath, String& statePath); private: - static constexpr auto cAllocatorSize = sizeof(size_t) * 2; - storagestate::StorageStateItf* mStorageStateManager {}; - StaticAllocator mAllocator; - SharedPtr mAvailableState; - SharedPtr mAvailableStorage; + AllocatorItf* mAllocator {}; + SharedPtr mAvailableState; + SharedPtr mAvailableStorage; }; } // namespace aos::cm::launcher diff --git a/src/core/cm/launcher/tests/launcher.cpp b/src/core/cm/launcher/tests/launcher.cpp index 91b073655..6ddefae17 100644 --- a/src/core/cm/launcher/tests/launcher.cpp +++ b/src/core/cm/launcher/tests/launcher.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "stubs/alertsproviderstub.hpp" #include "stubs/identproviderstub.hpp" @@ -143,6 +144,9 @@ class CMLauncherTest : public testing::Test { return mImageStore.GetManifestDigest(id.c_str(), imageID.c_str()); } + // Allocator must be declared before any object allocated from it to ensure correct destruction order. + HeapAllocator mAllocator; + // Stub objects alerts::AlertsProviderStub mAlertsProvider; imagemanager::ImageStoreStub mImageStore; @@ -607,7 +611,7 @@ TEST_F(CMLauncherTest, InstancesWithInvalidImageAreRemovedOnStart) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -653,7 +657,7 @@ TEST_F(CMLauncherTest, InstancesWithOutdatedTTLRemovedOnStart) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -718,7 +722,7 @@ TEST_F(CMLauncherTest, CacheInstances) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -816,7 +820,7 @@ TEST_F(CMLauncherTest, Components) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1505,7 +1509,7 @@ TEST_F(CMLauncherTest, Balancing) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1610,7 +1614,7 @@ TEST_F(CMLauncherTest, PlatformFiltering) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1703,7 +1707,7 @@ TEST_F(CMLauncherTest, ResendInstancesOnMismatchedNodeStatus) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1778,7 +1782,7 @@ TEST_F(CMLauncherTest, SubjectChanged) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1851,7 +1855,7 @@ TEST_F(CMLauncherTest, TestSentInstanceInfo) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -1937,7 +1941,7 @@ TEST_F(CMLauncherTest, PreinstalledComponents) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2025,7 +2029,7 @@ TEST_F(CMLauncherTest, SetStatusOnStart) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2092,7 +2096,7 @@ TEST_F(CMLauncherTest, OverrideEnvVars) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2215,7 +2219,7 @@ TEST_F(CMLauncherTest, MultiNodeInstance) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2345,7 +2349,7 @@ TEST_F(CMLauncherTest, RebalancingWithStoredNotScheduledInstances) // Init launcher. ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2442,7 +2446,7 @@ TEST_F(CMLauncherTest, CpuAlertRebalancingMovesLowerPriorityService) // Init launcher. ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2546,7 +2550,7 @@ TEST_F(CMLauncherTest, ServiceUpdate) // Init launcher ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); @@ -2646,7 +2650,7 @@ TEST_F(CMLauncherTest, UnlimitedSharedResource) mInstanceRunner.Init(mLauncher, true, aos::InstanceStateEnum::eActive); ASSERT_TRUE(mLauncher - .Init(CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, + .Init(mAllocator, CreateConfig(), mNodeInfoProvider, mInstanceRunner, mImageStore, mImageStore, mResourceManager, mStorageState, mMonitoringProvider, mAlertsProvider, mIdentProvider, ValidateGID, ValidateUID, mStorage, mSender) .IsNone()); diff --git a/src/core/cm/nodeinfoprovider/nodeinfoprovider.cpp b/src/core/cm/nodeinfoprovider/nodeinfoprovider.cpp index e29e2cd6d..4acbda10f 100644 --- a/src/core/cm/nodeinfoprovider/nodeinfoprovider.cpp +++ b/src/core/cm/nodeinfoprovider/nodeinfoprovider.cpp @@ -14,10 +14,12 @@ namespace aos::cm::nodeinfoprovider { * Public **********************************************************************************************************************/ -Error NodeInfoProvider::Init(const Config& config, iamclient::NodeInfoProviderItf& nodeInfoProvider) +Error NodeInfoProvider::Init( + AllocatorItf& allocator, const Config& config, iamclient::NodeInfoProviderItf& nodeInfoProvider) { LOG_DBG() << "Init node info provider"; + mAllocator = &allocator; mNodeInfoProvider = &nodeInfoProvider; mConfig = config; @@ -35,14 +37,20 @@ Error NodeInfoProvider::Start() return ErrorEnum::eWrongState; } - auto ids = MakeUnique, cMaxNumNodes>>(&mAllocator); + auto ids = MakeUnique, cMaxNumNodes>>(mAllocator); + if (!ids) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mNodeInfoProvider->GetAllNodeIDs(*ids); !err.IsNone()) { return err; } for (const auto& id : *ids) { - auto nodeInfo = MakeUnique(&mAllocator); + auto nodeInfo = MakeUnique(mAllocator); + if (!nodeInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mNodeInfoProvider->GetNodeInfo(id, *nodeInfo); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -249,7 +257,12 @@ NodeInfoCache* NodeInfoProvider::AddOrGetCacheItem(const String& nodeID) void NodeInfoProvider::NotifyListeners(const NodeInfoCache& info) { - auto unitNodeInfo = MakeUnique(&mAllocator); + auto unitNodeInfo = MakeUnique(mAllocator); + if (!unitNodeInfo) { + LOG_ERR() << "Can't allocate unit node info" << Log::Field(ErrorEnum::eNoMemory); + + return; + } info.GetUnitNodeInfo(*unitNodeInfo); diff --git a/src/core/cm/nodeinfoprovider/nodeinfoprovider.hpp b/src/core/cm/nodeinfoprovider/nodeinfoprovider.hpp index 4f869b49f..e30ae6347 100644 --- a/src/core/cm/nodeinfoprovider/nodeinfoprovider.hpp +++ b/src/core/cm/nodeinfoprovider/nodeinfoprovider.hpp @@ -7,7 +7,6 @@ #ifndef AOS_CORE_CM_NODEINFOPROVIDER_NODEINFOPROVIDER_HPP_ #define AOS_CORE_CM_NODEINFOPROVIDER_NODEINFOPROVIDER_HPP_ -#include #include #include #include @@ -34,11 +33,12 @@ class NodeInfoProvider : public NodeInfoProviderItf, public SMInfoReceiverItf, p /** * Initializes node info provider. * + * @param allocator allocator to use for temporary objects. * @param config configuration. * @param nodeInfoProvider IAM client node info provider. * @return Error. */ - Error Init(const Config& config, iamclient::NodeInfoProviderItf& nodeInfoProvider); + Error Init(AllocatorItf& allocator, const Config& config, iamclient::NodeInfoProviderItf& nodeInfoProvider); /** * Starts node info provider. @@ -112,8 +112,6 @@ class NodeInfoProvider : public NodeInfoProviderItf, public SMInfoReceiverItf, p private: static constexpr auto cListenersSize = 4; - static constexpr auto cAllocatorSize - = sizeof(UnitNodeInfo) + sizeof(StaticArray, cMaxNumNodes>); void OnNodeInfoChanged(const NodeInfo& info) override; @@ -124,7 +122,7 @@ class NodeInfoProvider : public NodeInfoProviderItf, public SMInfoReceiverItf, p void Run(); mutable Mutex mMutex; - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; Thread<> mThread; ConditionalVariable mCondVar; bool mRunning {}; diff --git a/src/core/cm/nodeinfoprovider/tests/nodeinfoprovider.cpp b/src/core/cm/nodeinfoprovider/tests/nodeinfoprovider.cpp index fb24bb6d8..3582188e5 100644 --- a/src/core/cm/nodeinfoprovider/tests/nodeinfoprovider.cpp +++ b/src/core/cm/nodeinfoprovider/tests/nodeinfoprovider.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -93,6 +94,10 @@ class CMNodeInfoProviderTest : public Test { protected: void SetUp() override { tests::utils::InitLog(); } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + iamclient::NodeInfoProviderStub mIAMNodeInfoProvider; NodeInfoListenerStub mListener; Config mConfig {Time::cMilliseconds * 100}; @@ -116,7 +121,7 @@ TEST_F(CMNodeInfoProviderTest, KnownInstancesAreProcessedOnStart) mIAMNodeInfoProvider.SetNodeInfo(*node); } - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.SubscribeListener(mListener); @@ -155,7 +160,7 @@ TEST_F(CMNodeInfoProviderTest, GetAllNodeIDs) mIAMNodeInfoProvider.SetNodeInfo(*CreateNodeInfo("node1", NodeStateEnum::eProvisioned, true)); mIAMNodeInfoProvider.SetNodeInfo(*CreateNodeInfo("node2", NodeStateEnum::eProvisioned, true)); - err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -177,7 +182,7 @@ TEST_F(CMNodeInfoProviderTest, GetNodeInfo) mIAMNodeInfoProvider.SetNodeInfo(*CreateNodeInfo("node1", NodeStateEnum::eProvisioned, true)); mIAMNodeInfoProvider.SetNodeInfo(*CreateNodeInfo("node2", NodeStateEnum::eProvisioned, true)); - auto err = mNodeInfoProvider.Init(Config {Time::cDay}, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, Config {Time::cDay}, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -208,7 +213,7 @@ TEST_F(CMNodeInfoProviderTest, NodeWithoutSMComponent) { auto nodeInfo = CreateNodeInfo("node1", NodeStateEnum::eProvisioned, true, false); - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -237,7 +242,7 @@ TEST_F(CMNodeInfoProviderTest, NodeWithSMComponent) CreateNodeInfo("node2", NodeStateEnum::eError, false), }; - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -300,7 +305,7 @@ TEST_F(CMNodeInfoProviderTest, NodeSetToOnlineIfBothIAMAndSMAreReceived) auto cNodeInfo = CreateNodeInfo("node1", NodeStateEnum::eProvisioned, false); - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -340,7 +345,7 @@ TEST_F(CMNodeInfoProviderTest, NodeSetToUnprovisionedIfSMNotConnectedButNodeInfo auto cNodeInfo = CreateNodeInfo("node1", NodeStateEnum::eUnprovisioned, true); - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); @@ -367,7 +372,7 @@ TEST_F(CMNodeInfoProviderTest, NotifySubscribersOnceSMInfoReceived) { const auto cNodeInfo = CreateNodeInfo("node1", NodeStateEnum::eProvisioned, false); - auto err = mNodeInfoProvider.Init(mConfig, mIAMNodeInfoProvider); + auto err = mNodeInfoProvider.Init(mAllocator, mConfig, mIAMNodeInfoProvider); ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); err = mNodeInfoProvider.Start(); diff --git a/src/core/cm/storagestate/storagestate.cpp b/src/core/cm/storagestate/storagestate.cpp index 4f1bf36e8..f09670780 100644 --- a/src/core/cm/storagestate/storagestate.cpp +++ b/src/core/cm/storagestate/storagestate.cpp @@ -44,11 +44,12 @@ Error ToRelativePath(const String& base, const String& full, String& result) * Public **********************************************************************************************************************/ -Error StorageState::Init(const Config& config, StorageItf& storage, SenderItf& sender, fs::FSPlatformItf& fsPlatform, - fs::FSWatcherItf& fsWatcher, crypto::HasherItf& hasher) +Error StorageState::Init(AllocatorItf& allocator, const Config& config, StorageItf& storage, SenderItf& sender, + fs::FSPlatformItf& fsPlatform, fs::FSWatcherItf& fsWatcher, crypto::HasherItf& hasher) { LOG_DBG() << "Init storage state"; + mAllocator = &allocator; mConfig = config; mStorage = &storage; mMessageSender = &sender; @@ -136,7 +137,10 @@ Error StorageState::UpdateState(const aos::UpdateState& state) return AOS_ERROR_WRAP(err); } - auto storageStateInfo = MakeUnique(&mAllocator); + auto storageStateInfo = MakeUnique(mAllocator); + if (!storageStateInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mStorage->GetStorageStateInfo(state, *storageStateInfo); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -188,7 +192,10 @@ Error StorageState::AcceptState(const StateAcceptance& state) return mMessageSender->SendStateRequest(request); } - auto storageStateInfo = MakeUnique(&mAllocator); + auto storageStateInfo = MakeUnique(mAllocator); + if (!storageStateInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mStorage->GetStorageStateInfo(state, *storageStateInfo); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -212,11 +219,17 @@ Error StorageState::Setup( LOG_DBG() << "Setup storage and state" << setupParams; - auto storageData = MakeUnique(&mAllocator); + auto storageData = MakeUnique(mAllocator); + if (!storageData) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto err = mStorage->GetStorageStateInfo(instanceIdent, *storageData); if (err.Is(ErrorEnum::eNotFound)) { - storageData = MakeUnique(&mAllocator); + storageData = MakeUnique(mAllocator); + if (!storageData) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } storageData->mInstanceIdent = instanceIdent; @@ -352,7 +365,10 @@ Error StorageState::InitStateWatching() { LOG_DBG() << "Init state watching"; - auto infos = MakeUnique(&mAllocator); + auto infos = MakeUnique(mAllocator); + if (!infos) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mStorage->GetAllStorageStateInfo(*infos); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -473,7 +489,10 @@ Error StorageState::CheckChecksumAndSendUpdateRequest(const State& state) { LOG_DBG() << "Check checksum and send update request" << state; - auto stateContent = MakeUnique>(&mAllocator); + auto stateContent = MakeUnique>(mAllocator); + if (!stateContent) { + return ErrorEnum::eNoMemory; + } if (auto err = fs::ReadFileToString(state.mFilePath, *stateContent); !err.IsNone()) { return err; @@ -574,7 +593,10 @@ Error StorageState::SetQuotas(const SetupParams& setupParams) Error StorageState::SendNewStateIfFileChanged(State& state) { - auto newState = MakeUnique(&mAllocator); + auto newState = MakeUnique(mAllocator); + if (!newState) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } static_cast(*newState) = state.mInstanceIdent; diff --git a/src/core/cm/storagestate/storagestate.hpp b/src/core/cm/storagestate/storagestate.hpp index 906ae936c..8cb0ed09e 100644 --- a/src/core/cm/storagestate/storagestate.hpp +++ b/src/core/cm/storagestate/storagestate.hpp @@ -35,6 +35,7 @@ class StorageState : public StateHandlerItf, /** * Initializes storage state instance. * + * @param allocator allocator to use for temporary objects. * @param config config object. * @param storage storage instance. * @param sender sender instance. @@ -43,8 +44,8 @@ class StorageState : public StateHandlerItf, * @param cryptoProvider crypto provider instance. * @return Error. */ - Error Init(const Config& config, StorageItf& storage, SenderItf& sender, fs::FSPlatformItf& fsPlatform, - fs::FSWatcherItf& fsWatcher, crypto::HasherItf& hasher); + Error Init(AllocatorItf& allocator, const Config& config, StorageItf& storage, SenderItf& sender, + fs::FSPlatformItf& fsPlatform, fs::FSWatcherItf& fsWatcher, crypto::HasherItf& hasher); /** * Starts storage state instance. @@ -145,7 +146,6 @@ class StorageState : public StateHandlerItf, static constexpr auto cHashAlgorithm = crypto::HashEnum::eSHA3_224; static constexpr auto cNumSendNewStateThreads = 1; static constexpr auto cInstanceStringLen = 8; - static constexpr auto cAllocatorSize = sizeof(InstanceInfoArray) + sizeof(NewState); struct State { State(const InstanceIdent& instanceIdent, const String& filePath, size_t quota) @@ -189,7 +189,7 @@ class StorageState : public StateHandlerItf, StaticString GetStoragePath(const InstanceIdent& instanceIdent) const; Error CalculateChecksum(const String& data, Array& checksum); - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; ThreadPool mThreadPool; Mutex mMutex; Config mConfig; diff --git a/src/core/cm/storagestate/tests/storagestate.cpp b/src/core/cm/storagestate/tests/storagestate.cpp index 051620036..c5c0c0ab4 100644 --- a/src/core/cm/storagestate/tests/storagestate.cpp +++ b/src/core/cm/storagestate/tests/storagestate.cpp @@ -17,6 +17,7 @@ #include #include #include +#include using namespace testing; @@ -203,7 +204,7 @@ class StorageStateTests : public Test { tests::utils::InitLog(); - EXPECT_TRUE(mCryptoProvider.Init().IsNone()) << "Failed to initialize crypto provider"; + EXPECT_TRUE(mCryptoProvider.Init(mAllocator).IsNone()) << "Failed to initialize crypto provider"; EXPECT_CALL(mFSPlatformMock, GetMountPoint) .WillRepeatedly(Return(RetWithError>(cTestDir.c_str()))); @@ -254,6 +255,10 @@ class StorageStateTests : public Test { return ErrorEnum::eNone; } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + crypto::DefaultCryptoProvider mCryptoProvider; StorageStub mStorageStub; StrictMock mFSPlatformMock; @@ -269,7 +274,8 @@ class StorageStateTests : public Test { TEST_F(StorageStateTests, StartStop) { - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -291,7 +297,8 @@ TEST_F(StorageStateTests, StorageQuotaNotSet) EXPECT_CALL(mFSPlatformMock, SetUserQuota(_, setupParams.mUID, setupParams.mStateQuota)).Times(1); - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mSenderMock, SendStateRequest).WillOnce(Return(ErrorEnum::eNone)); @@ -317,7 +324,8 @@ TEST_F(StorageStateTests, StateQuotaNotSet) StaticString storagePath, statePath; - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -352,7 +360,8 @@ TEST_F(StorageStateTests, StorageAndStateQuotaNotSet) StaticString storagePath, statePath; - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSPlatformMock, SetUserQuota).Times(0); @@ -375,7 +384,8 @@ TEST_F(StorageStateTests, SetupOnDifferentPartitions) .WillOnce(Return(RetWithError>("partition1"))) .WillOnce(Return(RetWithError>("partition2"))); - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -406,7 +416,8 @@ TEST_F(StorageStateTests, SetupFailsOnSetUserQuotaError) { constexpr auto cSetQuotaError = ErrorEnum::eOutOfRange; - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -426,7 +437,8 @@ TEST_F(StorageStateTests, SetupFailsOnSetUserQuotaError) TEST_F(StorageStateTests, SetupSameInstance) { - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -494,7 +506,8 @@ TEST_F(StorageStateTests, GetInstanceCheckSum) auto err = AddInstanceIdent(cInstanceIdent, "getchecksum-content"); EXPECT_TRUE(err.IsNone()) << "Failed to add instance ident: " << tests::utils::ErrorToStr(err); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSWatcherMock, Subscribe).WillOnce(Return(ErrorEnum::eNone)); @@ -521,7 +534,8 @@ TEST_F(StorageStateTests, Cleanup) { auto err = AddInstanceIdent(cInstanceIdent, "cleanup-content"); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSWatcherMock, Subscribe).WillOnce(Return(ErrorEnum::eNone)); @@ -553,7 +567,8 @@ TEST_F(StorageStateTests, Remove) { auto err = AddInstanceIdent(cInstanceIdent, "remove-content"); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSWatcherMock, Subscribe).WillOnce(Return(ErrorEnum::eNone)); @@ -585,7 +600,8 @@ TEST_F(StorageStateTests, UpdateState) auto err = AddInstanceIdent(cInstanceIdent, "outdated state content"); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSWatcherMock, Subscribe).WillOnce(Return(ErrorEnum::eNone)); @@ -624,7 +640,8 @@ TEST_F(StorageStateTests, UpdateState) TEST_F(StorageStateTests, AcceptStateUnknownInstance) { - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -647,7 +664,8 @@ TEST_F(StorageStateTests, AcceptStateChecksumMismatch) { auto err = AddInstanceIdent(cInstanceIdent, "initial state content"); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); EXPECT_CALL(mFSWatcherMock, Subscribe).WillOnce(Return(ErrorEnum::eNone)); @@ -674,7 +692,8 @@ TEST_F(StorageStateTests, AcceptStateChecksumMismatch) TEST_F(StorageStateTests, AcceptStateWithRejectedStatus) { - auto err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + auto err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); @@ -733,7 +752,8 @@ TEST_F(StorageStateTests, UpdateAndAcceptStateFlow) err = CalculateChecksum(cUpdateStateContent, updateStateContentChecksum); EXPECT_TRUE(err.IsNone()) << "Failed to calculate checksum: " << tests::utils::ErrorToStr(err); - err = mStorageState.Init(mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); + err = mStorageState.Init( + mAllocator, mConfig, mStorageStub, mSenderMock, mFSPlatformMock, mFSWatcherMock, mCryptoProvider); EXPECT_TRUE(err.IsNone()) << "Failed to initialize storage state: " << tests::utils::ErrorToStr(err); err = mStorageState.Start(); diff --git a/src/core/cm/unitconfig/tests/unitconfig.cpp b/src/core/cm/unitconfig/tests/unitconfig.cpp index 658a5bd34..13d0fab83 100644 --- a/src/core/cm/unitconfig/tests/unitconfig.cpp +++ b/src/core/cm/unitconfig/tests/unitconfig.cpp @@ -13,6 +13,7 @@ #include #include #include +#include using namespace testing; @@ -119,6 +120,10 @@ class UnitConfigTest : public Test { ASSERT_TRUE(err.IsNone()) << "Failed to create test config file"; } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + StrictMock mNodeInfoProvider; StrictMock mNodeConfigHandler; StrictMock mJSONProvider; @@ -135,7 +140,7 @@ TEST_F(UnitConfigTest, InitWithValidConfig) SetupValidUnitConfig(); - auto err = mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); + auto err = mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); EXPECT_TRUE(err.IsNone()); } @@ -146,14 +151,14 @@ TEST_F(UnitConfigTest, InitWithInvalidConfig) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)).WillOnce(Return(ErrorEnum::eInvalidArgument)); - auto err = mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); + auto err = mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); EXPECT_TRUE(err.IsNone()); } TEST_F(UnitConfigTest, InitWithMissingConfigFile) { - auto err = mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); + auto err = mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); EXPECT_TRUE(err.IsNone()); @@ -171,7 +176,8 @@ TEST_F(UnitConfigTest, GetUnitConfigStatusValid) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); UnitConfigStatus status; auto err = mUnitConfig.GetUnitConfigStatus(status); @@ -188,7 +194,7 @@ TEST_F(UnitConfigTest, GetUnitConfigStatusWithError) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)).WillOnce(Return(ErrorEnum::eInvalidArgument)); - auto err = mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); + auto err = mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider); EXPECT_TRUE(err.IsNone()); @@ -206,7 +212,8 @@ TEST_F(UnitConfigTest, GetNodeConfigByType) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); NodeConfig nodeConfig; auto err = mUnitConfig.GetNodeConfig("", cTestNodeType, nodeConfig); @@ -226,7 +233,8 @@ TEST_F(UnitConfigTest, GetNodeConfigByID) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)) .WillOnce(DoAll(SetArgReferee<1>(config), Return(ErrorEnum::eNone))); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); NodeConfig nodeConfig; auto err = mUnitConfig.GetNodeConfig(cTestNodeID, "", nodeConfig); @@ -243,7 +251,8 @@ TEST_F(UnitConfigTest, GetNodeConfigNotFound) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); NodeConfig nodeConfig; auto err = mUnitConfig.GetNodeConfig("nonexistent", "unknown", nodeConfig); @@ -263,7 +272,8 @@ TEST_F(UnitConfigTest, CheckUnitConfigValidVersion) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)) .WillOnce(DoAll(SetArgReferee<1>(config), Return(ErrorEnum::eNone))); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto newUnitConfig = CreateTestUnitConfigWithNodeID("2.0.0"); @@ -293,7 +303,8 @@ TEST_F(UnitConfigTest, CheckUnitConfigSameVersion) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto sameVersionConfig = CreateTestUnitConfig("1.0.0"); @@ -307,7 +318,8 @@ TEST_F(UnitConfigTest, CheckUnitConfigLowerVersion) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto lowerVersionConfig = CreateTestUnitConfig("0.9.0"); @@ -321,7 +333,8 @@ TEST_F(UnitConfigTest, UpdateUnitConfigSuccess) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto newUnitConfig = CreateTestUnitConfig("2.0.0"); @@ -347,7 +360,8 @@ TEST_F(UnitConfigTest, UpdateUnitConfigSameVersion) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto sameVersionConfig = CreateTestUnitConfig("1.0.0"); @@ -361,7 +375,8 @@ TEST_F(UnitConfigTest, OnNodeInfoChangedUpdatesConfig) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); UnitNodeInfo nodeInfo = CreateTestNodeInfo(); @@ -382,7 +397,8 @@ TEST_F(UnitConfigTest, OnNodeInfoChangedSkipsIfVersionMatches) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); UnitNodeInfo nodeInfo = CreateTestNodeInfo(); @@ -397,7 +413,8 @@ TEST_F(UnitConfigTest, OnNodeInfoChangedSkipsIfVersionMatches) TEST_F(UnitConfigTest, OnNodeInfoChangedSkipsWhenUnitConfigAbsent) { - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); UnitConfigStatus status; ASSERT_TRUE(mUnitConfig.GetUnitConfigStatus(status).IsNone()); @@ -415,7 +432,8 @@ TEST_F(UnitConfigTest, OnNodeInfoChangedWithUnitConfigError) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)).WillOnce(Return(ErrorEnum::eInvalidArgument)); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); UnitNodeInfo nodeInfo = CreateTestNodeInfo(); @@ -428,7 +446,8 @@ TEST_F(UnitConfigTest, VersionComparisonPrerelease) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto prereleaseVersion = CreateTestUnitConfig("1.0.0-alpha"); auto err = mUnitConfig.CheckUnitConfig(prereleaseVersion); @@ -445,7 +464,8 @@ TEST_F(UnitConfigTest, UpdateUnitConfigSkipsOfflineNodeThenSendsOnConnect) SetupValidUnitConfig(); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); auto newUnitConfig = CreateTestUnitConfig("2.0.0"); @@ -504,7 +524,8 @@ TEST_F(UnitConfigTest, CheckUnitConfigMultipleNodes) EXPECT_CALL(mJSONProvider, UnitConfigFromJSON(_, _)) .WillOnce(DoAll(SetArgReferee<1>(config), Return(ErrorEnum::eNone))); - ASSERT_TRUE(mUnitConfig.Init({cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); + ASSERT_TRUE( + mUnitConfig.Init(mAllocator, {cTestConfigFile}, mNodeInfoProvider, mNodeConfigHandler, mJSONProvider).IsNone()); aos::UnitConfig newUnitConfig; newUnitConfig.mVersion = "2.0.0"; diff --git a/src/core/cm/unitconfig/unitconfig.cpp b/src/core/cm/unitconfig/unitconfig.cpp index 98aee4419..937179637 100644 --- a/src/core/cm/unitconfig/unitconfig.cpp +++ b/src/core/cm/unitconfig/unitconfig.cpp @@ -16,11 +16,13 @@ namespace aos::cm::unitconfig { * Public **********************************************************************************************************************/ -Error UnitConfig::Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, - NodeConfigHandlerItf& nodeConfigHandler, JSONProviderItf& jsonProvider) +Error UnitConfig::Init(AllocatorItf& allocator, const Config& config, + nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, NodeConfigHandlerItf& nodeConfigHandler, + JSONProviderItf& jsonProvider) { LOG_DBG() << "Init unit config"; + mAllocator = &allocator; mUnitConfigFile = config.mUnitConfigFile; mNodeInfoProvider = &nodeInfoProvider; mNodeConfigHandler = &nodeConfigHandler; @@ -83,7 +85,10 @@ Error UnitConfig::CheckUnitConfig(const aos::UnitConfig& config) } for (const auto& id : nodeIds) { - auto nodeInfo = MakeUnique(&mAllocator); + auto nodeInfo = MakeUnique(mAllocator); + if (!nodeInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mNodeInfoProvider->GetNodeInfo(id, *nodeInfo); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -102,7 +107,10 @@ Error UnitConfig::CheckUnitConfig(const aos::UnitConfig& config) } if (nodeConfigStatus.mVersion != config.mVersion || !nodeConfigStatus.mError.IsNone()) { - auto nodeConfig = MakeUnique(&mAllocator); + auto nodeConfig = MakeUnique(mAllocator); + if (!nodeConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = FindNodeConfig(nodeInfo->mNodeID, nodeInfo->mNodeType, config, *nodeConfig); !err.IsNone()) { return err; @@ -143,7 +151,10 @@ Error UnitConfig::UpdateUnitConfig(const aos::UnitConfig& unitConfig) mUnitConfig = unitConfig; - auto unitConfigJSON = MakeUnique>(&mAllocator); + auto unitConfigJSON = MakeUnique>(mAllocator); + if (!unitConfigJSON) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mJSONProvider->UnitConfigToJSON(unitConfig, *unitConfigJSON); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -163,8 +174,15 @@ Error UnitConfig::UpdateUnitConfig(const aos::UnitConfig& unitConfig) } for (const auto& id : nodeIds) { - auto nodeConfig = MakeUnique(&mAllocator); - auto nodeInfo = MakeUnique(&mAllocator); + auto nodeConfig = MakeUnique(mAllocator); + if (!nodeConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto nodeInfo = MakeUnique(mAllocator); + if (!nodeInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mNodeInfoProvider->GetNodeInfo(id, *nodeInfo); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -227,7 +245,12 @@ void UnitConfig::OnNodeInfoChanged(const UnitNodeInfo& info) return; } - auto nodeConfig = MakeUnique(&mAllocator); + auto nodeConfig = MakeUnique(mAllocator); + if (!nodeConfig) { + LOG_ERR() << "Can't allocate node config" << Log::Field(ErrorEnum::eNoMemory); + + return; + } if (auto err = FindNodeConfig(info.mNodeID, info.mNodeType, mUnitConfig, *nodeConfig); !err.IsNone()) { LOG_ERR() << "Error finding node config" << Log::Field(err); @@ -250,7 +273,10 @@ Error UnitConfig::LoadConfig() { LOG_DBG() << "Load config"; - auto unitConfig = MakeUnique>(&mAllocator); + auto unitConfig = MakeUnique>(mAllocator); + if (!unitConfig) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto err = fs::ReadFileToString(mUnitConfigFile, *unitConfig); if (!err.IsNone()) { diff --git a/src/core/cm/unitconfig/unitconfig.hpp b/src/core/cm/unitconfig/unitconfig.hpp index 0d1c02622..16ad28965 100644 --- a/src/core/cm/unitconfig/unitconfig.hpp +++ b/src/core/cm/unitconfig/unitconfig.hpp @@ -33,13 +33,14 @@ class UnitConfig : public UnitConfigItf, public NodeConfigProviderItf, public no /** * Initializes unit config. * + * @param allocator allocator to use for temporary objects. * @param config configuration. * @param nodeInfoProvider node info provider. * @param nodeConfigHandler node config handler. * @param jsonProvider JSON provider. * @return Error. */ - Error Init(const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, + Error Init(AllocatorItf& allocator, const Config& config, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, NodeConfigHandlerItf& nodeConfigHandler, JSONProviderItf& jsonProvider); /** @@ -114,9 +115,7 @@ class UnitConfig : public UnitConfigItf, public NodeConfigProviderItf, public no Error mUnitConfigError; UnitConfigState mUnitConfigState {UnitConfigStateEnum::eAbsent}; - StaticAllocator) + sizeof(NodeConfig) - + sizeof(UnitNodeInfo)> - mAllocator; + AllocatorItf* mAllocator {}; StaticArray mNodeConfigStatuses; diff --git a/src/core/cm/updatemanager/desiredstatushandler.cpp b/src/core/cm/updatemanager/desiredstatushandler.cpp index 7c8ce5f33..de5feab69 100644 --- a/src/core/cm/updatemanager/desiredstatushandler.cpp +++ b/src/core/cm/updatemanager/desiredstatushandler.cpp @@ -14,12 +14,13 @@ namespace aos::cm::updatemanager { * Public **********************************************************************************************************************/ -Error DesiredStatusHandler::Init(iamclient::NodeHandlerItf& nodeHandler, unitconfig::UnitConfigItf& unitConfig, - imagemanager::ImageManagerItf& imageManager, launcher::LauncherItf& launcher, UnitStatusHandler& unitStatusHandler, - StorageItf& storage) +Error DesiredStatusHandler::Init(AllocatorItf& allocator, iamclient::NodeHandlerItf& nodeHandler, + unitconfig::UnitConfigItf& unitConfig, imagemanager::ImageManagerItf& imageManager, launcher::LauncherItf& launcher, + UnitStatusHandler& unitStatusHandler, StorageItf& storage) { LOG_DBG() << "Init desired status handler"; + mAllocator = &allocator; mNodeHandler = &nodeHandler; mUnitConfig = &unitConfig; mUnitStatusHandler = &unitStatusHandler; @@ -331,7 +332,10 @@ void DesiredStatusHandler::SetState(UpdateState state) Error DesiredStatusHandler::DownloadUpdateItems() { - auto itemsStatuses = MakeUnique>(&mAllocator); + auto itemsStatuses = MakeUnique>(mAllocator); + if (!itemsStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } LOG_DBG() << "Download update items" << Log::Field("count", mCurrentDesiredStatus.mUpdateItems.Size()); @@ -402,8 +406,15 @@ Error DesiredStatusHandler::InstallDesiredStatus() Error DesiredStatusHandler::LaunchInstances() { - auto runRequest = MakeUnique>(&mAllocator); - auto instancesStatuses = MakeUnique>(&mAllocator); + auto runRequest = MakeUnique>(mAllocator); + if (!runRequest) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto instancesStatuses = MakeUnique>(mAllocator); + if (!instancesStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } LOG_DBG() << "Launch instances" << Log::Field("count", mCurrentDesiredStatus.mInstances.Size()); @@ -458,7 +469,10 @@ Error DesiredStatusHandler::LaunchInstances() Error DesiredStatusHandler::WaitInstancesActive() { - auto instancesStatuses = MakeUnique>(&mAllocator); + auto instancesStatuses = MakeUnique>(mAllocator); + if (!instancesStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } while (mIsRunning) { if (auto err = mLauncher->GetInstancesStatuses(*instancesStatuses); !err.IsNone()) { @@ -491,7 +505,10 @@ Error DesiredStatusHandler::WaitInstancesActive() Error DesiredStatusHandler::FinalizeUpdate() { - auto itemsStatuses = MakeUnique>(&mAllocator); + auto itemsStatuses = MakeUnique>(mAllocator); + if (!itemsStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } LOG_DBG() << "Install update items" << Log::Field("count", mCurrentDesiredStatus.mUpdateItems.Size()); @@ -534,7 +551,12 @@ bool DesiredStatusHandler::IsUpdateRequired(const DesiredStatus& desiredStatus) bool DesiredStatusHandler::IsUpdateItemsRequired(const DesiredStatus& desiredStatus) const { - auto itemsStatuses = MakeUnique>(&mAllocator); + auto itemsStatuses = MakeUnique>(mAllocator); + if (!itemsStatuses) { + LOG_ERR() << "Failed to allocate update items statuses" << Log::Field(ErrorEnum::eNoMemory); + + return true; + } if (auto err = mImageManager->GetUpdateItemsStatuses(*itemsStatuses); !err.IsNone()) { LOG_ERR() << "Failed to get update items statuses" << Log::Field(err); @@ -594,7 +616,12 @@ bool DesiredStatusHandler::IsSameUpdate(const DesiredStatus& desiredStatus) cons bool DesiredStatusHandler::IsUpdateInstancesRequired(const DesiredStatus& desiredStatus) const { - auto instancesStatuses = MakeUnique>(&mAllocator); + auto instancesStatuses = MakeUnique>(mAllocator); + if (!instancesStatuses) { + LOG_ERR() << "Failed to allocate instances statuses" << Log::Field(ErrorEnum::eNoMemory); + + return true; + } if (auto err = mLauncher->GetInstancesStatuses(*instancesStatuses); !err.IsNone()) { LOG_ERR() << "Failed to get instances statuses" << Log::Field(err); diff --git a/src/core/cm/updatemanager/desiredstatushandler.hpp b/src/core/cm/updatemanager/desiredstatushandler.hpp index 7fbc831de..eaba0dcc4 100644 --- a/src/core/cm/updatemanager/desiredstatushandler.hpp +++ b/src/core/cm/updatemanager/desiredstatushandler.hpp @@ -31,6 +31,7 @@ class DesiredStatusHandler : private instancestatusprovider::ListenerItf { /** * Initializes desired status handler. * + * @param allocator allocator to use for temporary objects. * @param nodeHandler node handler. * @param unitConfig unit config interface. * @param imageManager image manager. @@ -39,7 +40,7 @@ class DesiredStatusHandler : private instancestatusprovider::ListenerItf { * @param storage storage interface. * @return Error. */ - Error Init(iamclient::NodeHandlerItf& nodeHandler, unitconfig::UnitConfigItf& unitConfig, + Error Init(AllocatorItf& allocator, iamclient::NodeHandlerItf& nodeHandler, unitconfig::UnitConfigItf& unitConfig, imagemanager::ImageManagerItf& imageManager, launcher::LauncherItf& launcher, UnitStatusHandler& unitStatusHandler, StorageItf& storage); @@ -67,11 +68,6 @@ class DesiredStatusHandler : private instancestatusprovider::ListenerItf { private: static constexpr auto cWaitActiveTimeout = Time::cMinutes * 10; - static constexpr auto cAllocatorSize = Max(sizeof(StaticArray), - sizeof(StaticArray) - + sizeof(StaticArray)) - + Max(sizeof(StaticArray), - sizeof(StaticArray)); // instancestatusprovider::ListenerItf implementation void OnInstancesStatusesChanged(const Array& statuses) override; @@ -109,7 +105,7 @@ class DesiredStatusHandler : private instancestatusprovider::ListenerItf { bool mCancelCurrentUpdate {}; UpdateState mUpdateState {}; - mutable StaticAllocator mAllocator {}; + AllocatorItf* mAllocator {}; }; /** @}*/ diff --git a/src/core/cm/updatemanager/tests/updatemanager.cpp b/src/core/cm/updatemanager/tests/updatemanager.cpp index b0d38895a..4969ccccb 100644 --- a/src/core/cm/updatemanager/tests/updatemanager.cpp +++ b/src/core/cm/updatemanager/tests/updatemanager.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -343,7 +344,7 @@ class UpdateManagerTest : public Test { Config config {cUnitStatusSendTimeout}; - auto err = mUpdateManager.Init(config, mIdentProviderMock, mNodeHandlerMock, mUnitConfigMock, + auto err = mUpdateManager.Init(mAllocator, config, mIdentProviderMock, mNodeHandlerMock, mUnitConfigMock, mNodeInfoProviderMock, mImageManagerMock, mLauncherMock, mCloudConnectionMock, mSenderStub, mStorageStub); EXPECT_TRUE(err.IsNone()) << "Failed to initialize update manager: " << tests::utils::ErrorToStr(err); @@ -429,6 +430,10 @@ class UpdateManagerTest : public Test { EXPECT_TRUE(err.IsNone()) << "Failed to stop update manager: " << tests::utils::ErrorToStr(err); } + // mAllocator must be declared (and therefore destroyed) after any member that allocates from it, since + // members are destroyed in reverse declaration order. + HeapAllocator mAllocator; + UpdateManager mUpdateManager; NiceMock mIdentProviderMock; NiceMock mNodeHandlerMock; diff --git a/src/core/cm/updatemanager/unitstatushandler.cpp b/src/core/cm/updatemanager/unitstatushandler.cpp index 14c0a61d0..3bd0f6e65 100644 --- a/src/core/cm/updatemanager/unitstatushandler.cpp +++ b/src/core/cm/updatemanager/unitstatushandler.cpp @@ -14,12 +14,13 @@ namespace aos::cm::updatemanager { * Public **********************************************************************************************************************/ -Error UnitStatusHandler::Init(const Config& config, iamclient::IdentProviderItf& identProvider, +Error UnitStatusHandler::Init(AllocatorItf& allocator, const Config& config, iamclient::IdentProviderItf& identProvider, unitconfig::UnitConfigItf& unitConfig, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, imagemanager::ItemStatusProviderItf& itemStatusProvider, instancestatusprovider::ProviderItf& instanceStatusProvider, cloudconnection::CloudConnectionItf& cloudConnection, SenderItf& sender) { + mAllocator = &allocator; mIdentProvider = &identProvider; mUnitConfig = &unitConfig; mNodeInfoProvider = &nodeInfoProvider; @@ -443,7 +444,10 @@ Error UnitStatusHandler::SetNodesInfo() Error UnitStatusHandler::SetUpdateItemsStatus() { - auto itemsStatuses = MakeUnique(&mAllocator); + auto itemsStatuses = MakeUnique(mAllocator); + if (!itemsStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } mItemStatusProvider->GetUpdateItemsStatuses(*itemsStatuses); @@ -467,7 +471,10 @@ Error UnitStatusHandler::SetInstancesStatus() mUnitStatus.mInstances.EmplaceValue(); mUnitInstancesStatuses.Clear(); - auto instancesStatuses = MakeUnique>(&mAllocator); + auto instancesStatuses = MakeUnique>(mAllocator); + if (!instancesStatuses) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mInstanceStatusProvider->GetInstancesStatuses(*instancesStatuses); !err.IsNone()) { return AOS_ERROR_WRAP(err); diff --git a/src/core/cm/updatemanager/unitstatushandler.hpp b/src/core/cm/updatemanager/unitstatushandler.hpp index 7f07d8831..50692f498 100644 --- a/src/core/cm/updatemanager/unitstatushandler.hpp +++ b/src/core/cm/updatemanager/unitstatushandler.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -38,6 +39,7 @@ class UnitStatusHandler : private nodeinfoprovider::NodeInfoListenerItf, /** * Initializes unit status handler. * + * @param allocator allocator to use for temporary objects. * @param config update manager configuration. * @param identProvider identity provider. * @param unitConfig unit config interface. @@ -48,10 +50,10 @@ class UnitStatusHandler : private nodeinfoprovider::NodeInfoListenerItf, * @param sender unit status sender. * @return Error. */ - Error Init(const Config& config, iamclient::IdentProviderItf& identProvider, unitconfig::UnitConfigItf& unitConfig, - nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, - imagemanager::ItemStatusProviderItf& itemStatusProvider, - instancestatusprovider::ProviderItf& instanceStatusProvider, + Error Init(AllocatorItf& allocator, const Config& config, iamclient::IdentProviderItf& identProvider, + unitconfig::UnitConfigItf& unitConfig, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, + imagemanager::ItemStatusProviderItf& itemStatusProvider, + instancestatusprovider::ProviderItf& instanceStatusProvider, cloudconnection::CloudConnectionItf& cloudConnection, SenderItf& sender); /** @@ -91,9 +93,6 @@ class UnitStatusHandler : private nodeinfoprovider::NodeInfoListenerItf, Error SetUpdateNodeStatus(const String& nodeID, const Error& updateErr); private: - static constexpr auto cAllocatorSize - = Max(sizeof(StaticArray), sizeof(UpdateItemStatusArray)); - // nodeinfoprovider::NodeInfoListenerItf implementation void OnNodeInfoChanged(const UnitNodeInfo& info) override; @@ -134,7 +133,7 @@ class UnitStatusHandler : private nodeinfoprovider::NodeInfoListenerItf, Mutex mMutex; UnitStatus mUnitStatus; StaticArray mUnitInstancesStatuses; - StaticAllocator mAllocator; + AllocatorItf* mAllocator {}; bool mCloudConnected {}; bool mIsStatusProcessing {}; diff --git a/src/core/cm/updatemanager/updatemanager.cpp b/src/core/cm/updatemanager/updatemanager.cpp index de7b5b95d..00ad0de46 100644 --- a/src/core/cm/updatemanager/updatemanager.cpp +++ b/src/core/cm/updatemanager/updatemanager.cpp @@ -14,7 +14,7 @@ namespace aos::cm::updatemanager { * Public **********************************************************************************************************************/ -Error UpdateManager::Init(const Config& config, iamclient::IdentProviderItf& identProvider, +Error UpdateManager::Init(AllocatorItf& allocator, const Config& config, iamclient::IdentProviderItf& identProvider, iamclient::NodeHandlerItf& nodeHandler, unitconfig::UnitConfigItf& unitConfig, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, imagemanager::ImageManagerItf& imageManager, launcher::LauncherItf& launcher, cloudconnection::CloudConnectionItf& cloudConnection, SenderItf& sender, @@ -22,14 +22,14 @@ Error UpdateManager::Init(const Config& config, iamclient::IdentProviderItf& ide { LOG_DBG() << "Init update manager"; - if (auto err - = mDesiredStatusHandler.Init(nodeHandler, unitConfig, imageManager, launcher, mUnitStatusHandler, storage); + if (auto err = mDesiredStatusHandler.Init( + allocator, nodeHandler, unitConfig, imageManager, launcher, mUnitStatusHandler, storage); !err.IsNone()) { return AOS_ERROR_WRAP(err); } - if (auto err = mUnitStatusHandler.Init( - config, identProvider, unitConfig, nodeInfoProvider, imageManager, launcher, cloudConnection, sender); + if (auto err = mUnitStatusHandler.Init(allocator, config, identProvider, unitConfig, nodeInfoProvider, imageManager, + launcher, cloudConnection, sender); !err.IsNone()) { return AOS_ERROR_WRAP(err); } diff --git a/src/core/cm/updatemanager/updatemanager.hpp b/src/core/cm/updatemanager/updatemanager.hpp index 5f0a02e29..140efbf09 100644 --- a/src/core/cm/updatemanager/updatemanager.hpp +++ b/src/core/cm/updatemanager/updatemanager.hpp @@ -27,6 +27,7 @@ class UpdateManager : public UpdateManagerItf { /** * Initializes update manager. * + * @param allocator allocator to use for temporary objects. * @param config update manager configuration. * @param identProvider identity provider. * @param nodeHandler node handler. @@ -39,10 +40,11 @@ class UpdateManager : public UpdateManagerItf { * @param storage storage interface. * @return Error. */ - Error Init(const Config& config, iamclient::IdentProviderItf& identProvider, iamclient::NodeHandlerItf& nodeHandler, - unitconfig::UnitConfigItf& unitConfig, nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, - imagemanager::ImageManagerItf& imageManager, launcher::LauncherItf& launcher, - cloudconnection::CloudConnectionItf& cloudConnection, SenderItf& sender, StorageItf& storage); + Error Init(AllocatorItf& allocator, const Config& config, iamclient::IdentProviderItf& identProvider, + iamclient::NodeHandlerItf& nodeHandler, unitconfig::UnitConfigItf& unitConfig, + nodeinfoprovider::NodeInfoProviderItf& nodeInfoProvider, imagemanager::ImageManagerItf& imageManager, + launcher::LauncherItf& launcher, cloudconnection::CloudConnectionItf& cloudConnection, SenderItf& sender, + StorageItf& storage); /** * Starts update manager. diff --git a/src/core/common/crypto/certloader.cpp b/src/core/common/crypto/certloader.cpp index c64e32d37..bcfe966fe 100644 --- a/src/core/common/crypto/certloader.cpp +++ b/src/core/common/crypto/certloader.cpp @@ -24,10 +24,11 @@ constexpr auto cSchemeMaxLength = Max(sizeof(cSchemeFile), sizeof(cSchemePKCS11) * CertLoader **********************************************************************************************************************/ -Error CertLoader::Init(x509::ProviderItf& cryptoProvider, pkcs11::PKCS11Manager& pkcs11Manager) +Error CertLoader::Init(AllocatorItf& allocator, x509::ProviderItf& cryptoProvider, pkcs11::PKCS11Manager& pkcs11Manager) { LOG_DBG() << "Init cert loader"; + mAllocator = &allocator; mCryptoProvider = &cryptoProvider; mPKCS11 = &pkcs11Manager; @@ -73,7 +74,7 @@ RetWithError> CertLoader::LoadCertsChainByURL( return {nullptr, err}; } - return pkcs11::Utils(session, *mCryptoProvider, mAllocator).FindCertificateChain(id, label); + return pkcs11::Utils(*mAllocator, session, *mCryptoProvider).FindCertificateChain(id, label); } return {nullptr, ErrorEnum::eInvalidArgument}; @@ -118,7 +119,7 @@ RetWithError> CertLoader::LoadPrivKeyByURL(const String return {nullptr, err}; } - auto key = pkcs11::Utils(session, *mCryptoProvider, mAllocator).FindPrivateKey(id, label); + auto key = pkcs11::Utils(*mAllocator, session, *mCryptoProvider).FindPrivateKey(id, label); return {key.mValue.GetPrivKey(), key.mError}; } @@ -165,7 +166,11 @@ RetWithError> CertLoader::OpenSession( RetWithError CertLoader::FindToken(const pkcs11::LibraryContext& library, const String& token) { StaticArray slotList; - auto tokenInfo = MakeUnique(&mAllocator); + + auto tokenInfo = MakeUnique(mAllocator); + if (!tokenInfo) { + return {0, ErrorEnum::eNoMemory}; + } auto err = library.GetSlotList(true, slotList); if (!err.IsNone()) { @@ -190,14 +195,20 @@ RetWithError> CertLoader::LoadCertsFromFile(co { LOG_DBG() << "Load certs chain from file: fileName=" << fileName; - auto buff = MakeUnique(&mAllocator); + auto buff = MakeUnique(mAllocator); + if (!buff) { + return {nullptr, ErrorEnum::eNoMemory}; + } auto err = fs::ReadFileToString(fileName, *buff); if (!err.IsNone()) { return {nullptr, err}; } - auto certificates = MakeShared(&mAllocator); + auto certificates = MakeShared(mAllocator); + if (!certificates) { + return {nullptr, ErrorEnum::eNoMemory}; + } err = mCryptoProvider->PEMToX509Certs(*buff, *certificates); @@ -208,7 +219,10 @@ RetWithError> CertLoader::LoadPrivKeyFromFile(const Str { LOG_DBG() << "Load private key from file: fileName=" << fileName; - auto buff = MakeUnique>(&mAllocator); + auto buff = MakeUnique>(mAllocator); + if (!buff) { + return {nullptr, ErrorEnum::eNoMemory}; + } auto err = fs::ReadFileToString(fileName, *buff); if (!err.IsNone()) { diff --git a/src/core/common/crypto/certloader.hpp b/src/core/common/crypto/certloader.hpp index fa63febe6..cd2a58d4a 100644 --- a/src/core/common/crypto/certloader.hpp +++ b/src/core/common/crypto/certloader.hpp @@ -24,11 +24,12 @@ class CertLoader : public CertLoaderItf { /** * Initializes object instance. * + * @param allocator allocator to use for certificates/keys. * @param cryptoProvider crypto provider interface. * @param pkcs11Manager PKCS11 library manager. * @return Error. */ - Error Init(x509::ProviderItf& cryptoProvider, pkcs11::PKCS11Manager& pkcs11Manager); + Error Init(AllocatorItf& allocator, x509::ProviderItf& cryptoProvider, pkcs11::PKCS11Manager& pkcs11Manager); /** * Loads certificate chain by URL. @@ -49,12 +50,6 @@ class CertLoader : public CertLoaderItf { private: using PEMCertChainBlob = StaticString; - static constexpr auto cCertAllocatorSize - = cCertChainsCount * cCertChainSize * sizeof(x509::Certificate) + sizeof(PEMCertChainBlob); - static constexpr auto cKeyAllocatorSize - = AOS_CONFIG_CRYPTO_PRIV_KEYS_COUNT * pkcs11::cPrivateKeyMaxSize + sizeof(cPrivKeyPEMLen); - static constexpr auto cNumAllocation = AOS_CONFIG_CRYPTO_NUM_ALLOCATIONS; - static constexpr auto cDefaultPKCS11Library = AOS_CONFIG_CRYPTO_DEFAULT_PKCS11_LIB; RetWithError> OpenSession( @@ -66,9 +61,7 @@ class CertLoader : public CertLoaderItf { x509::ProviderItf* mCryptoProvider = nullptr; pkcs11::PKCS11Manager* mPKCS11 = nullptr; - - StaticAllocator - mAllocator; + AllocatorItf* mAllocator {}; }; } // namespace aos::crypto diff --git a/src/core/common/crypto/cryptohelper.cpp b/src/core/common/crypto/cryptohelper.cpp index 0c1ccccd2..0be0cc313 100644 --- a/src/core/common/crypto/cryptohelper.cpp +++ b/src/core/common/crypto/cryptohelper.cpp @@ -21,15 +21,20 @@ CryptoHelper::CryptoHelper() { } -Error CryptoHelper::Init(iamclient::CertProviderItf& certProvider, CryptoProviderItf& cryptoProvider, - CertLoaderItf& certLoader, const String& serviceDiscoveryURL, const String& caCert) +Error CryptoHelper::Init(AllocatorItf& allocator, iamclient::CertProviderItf& certProvider, + CryptoProviderItf& cryptoProvider, CertLoaderItf& certLoader, const String& serviceDiscoveryURL, + const String& caCert) { + mAllocator = &allocator; mCertProvider = &certProvider; mCryptoProvider = &cryptoProvider; mCertLoader = &certLoader; mServiceDiscoveryURL = serviceDiscoveryURL; - auto caCertsPEM = MakeUnique>(&mAllocator); + auto caCertsPEM = MakeUnique>(mAllocator); + if (!caCertsPEM) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = fs::ReadFileToString(caCert, *caCertsPEM); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -110,7 +115,10 @@ Error CryptoHelper::ValidateSigns(const String& decryptedPath, const SignInfo& s { LockGuard lock {mSemaphore}; - auto signCtx = MakeUnique(&mAllocator); + auto signCtx = MakeUnique(mAllocator); + if (!signCtx) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = AddCertificates(certs, *signCtx); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -131,8 +139,15 @@ Error CryptoHelper::DecryptMetadata(const Array& input, Array& { LockGuard lock {mSemaphore}; - auto contentInfo = MakeUnique(&mAllocator); - auto symKey = MakeUnique>(&mAllocator); + auto contentInfo = MakeUnique(mAllocator); + if (!contentInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto symKey = MakeUnique>(mAllocator); + if (!symKey) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto err = UnmarshalCMS(input, *contentInfo); if (!err.IsNone()) { @@ -164,7 +179,11 @@ Error CryptoHelper::DecryptMetadata(const Array& input, Array& RetWithError> CryptoHelper::GetOnlineCert() { - auto certInfo = MakeUnique(&mAllocator); + auto certInfo = MakeUnique(mAllocator); + if (!certInfo) { + return {{}, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)}; + } + if (auto err = mCertProvider->GetCert(cOnlineCert, {}, {}, *certInfo); !err.IsNone()) { return {{}, AOS_ERROR_WRAP(err)}; } @@ -205,7 +224,10 @@ Error CryptoHelper::GetServiceDiscoveryFromExtensions(const x509::Certificate& c Error CryptoHelper::GetServiceDiscoveryFromOrganization( const x509::Certificate& cert, Array>& urls) { - auto subject = MakeUnique>(&mAllocator); + auto subject = MakeUnique>(mAllocator); + if (!subject) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mCryptoProvider->ASN1DecodeDN(cert.mSubject, *subject); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -221,8 +243,15 @@ Error CryptoHelper::GetServiceDiscoveryFromOrganization( auto valueStart = orgPos + orgKey.Size(); auto [valueEnd, _] = subject->FindSubstr(valueStart, ","); - auto orgName = MakeUnique>(&mAllocator); - auto url = MakeUnique>(&mAllocator); + auto orgName = MakeUnique>(mAllocator); + if (!orgName) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto url = MakeUnique>(mAllocator); + if (!url) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto assignErr = orgName->Insert(orgName->begin(), subject->begin() + valueStart, subject->begin() + valueEnd); if (!assignErr.IsNone()) { @@ -331,8 +360,15 @@ Error CryptoHelper::CheckSessionKey( Error CryptoHelper::DecodeFile(const String& encryptedFile, const String& decryptedFile, AESCipherItf& decoder) { - auto inBlock = MakeUnique>(&mAllocator); - auto outBlock = MakeUnique>(&mAllocator); + auto inBlock = MakeUnique>(mAllocator); + if (!inBlock) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + auto outBlock = MakeUnique>(mAllocator); + if (!outBlock) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } fs::File inputFile, outputFile; @@ -410,7 +446,10 @@ Error CryptoHelper::AddCertificates(const Array& certs, SignCon continue; } - auto cert = MakeUnique(&mAllocator); + auto cert = MakeUnique(mAllocator); + if (!cert) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = mCryptoProvider->DERToX509Cert(certInfo.mCertificate, *cert); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -475,7 +514,10 @@ Error CryptoHelper::VerifySigns(const String& file, const SignInfo& signs, SignC } // Verify sign - auto hashSum = MakeUnique>(&mAllocator); + auto hashSum = MakeUnique>(mAllocator); + if (!hashSum) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = CalculateFileHash(file, hash, *mCryptoProvider, *hashSum); !err.IsNone()) { return AOS_ERROR_WRAP(err); @@ -500,7 +542,10 @@ Error CryptoHelper::VerifySigns(const String& file, const SignInfo& signs, SignC } // Verify certs - auto intermCertPool = MakeUnique>(&mAllocator); + auto intermCertPool = MakeUnique>(mAllocator); + if (!intermCertPool) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (auto err = CreateIntermCertPool(signCtx, *chain, *intermCertPool); !err.IsNone()) { return err; @@ -856,7 +901,10 @@ Error CryptoHelper::ParseEncryptedContentInfo(const Array& data, Encryp Error CryptoHelper::GetKeyForEnvelope(const TransRecipientInfo& info, Array& symmetricKey) { - auto certInfo = MakeUnique(&mAllocator); + auto certInfo = MakeUnique(mAllocator); + if (!certInfo) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } auto err = mCertProvider->GetCert(cOfflineCert, info.mRID.mIssuer, info.mRID.mSerial, *certInfo); if (!err.IsNone()) { @@ -920,7 +968,10 @@ Error CryptoHelper::DecryptMessage( Error CryptoHelper::DecodeMessage(AESCipherItf& decoder, const Array& input, Array& message) { - auto outBlock = MakeUnique>(&mAllocator); + auto outBlock = MakeUnique>(mAllocator); + if (!outBlock) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } if (input.Size() % AESCipherItf::cBlockSize != 0) { return AOS_ERROR_WRAP(Error(ErrorEnum::eInvalidArgument, "message should be a multiple of CBC block size")); diff --git a/src/core/common/crypto/cryptohelper.hpp b/src/core/common/crypto/cryptohelper.hpp index a569e6f5b..df8e6b4ae 100644 --- a/src/core/common/crypto/cryptohelper.hpp +++ b/src/core/common/crypto/cryptohelper.hpp @@ -8,6 +8,7 @@ #define AOS_AOS_COMMON_CRYPTO_CRYPTOHELPER_HPP_ #include +#include #include "itf/certloader.hpp" #include "itf/crypto.hpp" @@ -97,6 +98,7 @@ class CryptoHelper : public CryptoHelperItf { /** * Initializes crypto helper. * + * @param allocator allocator to use for temporary objects. * @param certProvider certificate provider interface. * @param cryptoProvider cryptographic provider interface. * @param certLoader certificate loader interface. @@ -104,8 +106,8 @@ class CryptoHelper : public CryptoHelperItf { * @param caCert root certificate path. * @return Error. */ - Error Init(iamclient::CertProviderItf& certProvider, CryptoProviderItf& cryptoProvider, CertLoaderItf& certLoader, - const String& serviceDiscoveryURL, const String& caCert); + Error Init(AllocatorItf& allocator, iamclient::CertProviderItf& certProvider, CryptoProviderItf& cryptoProvider, + CertLoaderItf& certLoader, const String& serviceDiscoveryURL, const String& caCert); /** * Retrieves available service discovery URLs. @@ -157,11 +159,6 @@ class CryptoHelper : public CryptoHelperItf { static constexpr auto cRSAEncryptionOid = "1.2.840.113549.1.1.1"; static constexpr auto cAES256CBCOid = "2.16.840.1.101.3.4.1.42"; - static constexpr auto cThreadHeapUsage = 2 * sizeof(CertInfo) + sizeof(StaticString) - + sizeof(StaticArray) + sizeof(SignContext) + sizeof(x509::Certificate) - + sizeof(StaticArray) + sizeof(StaticArray) - + sizeof(StaticArray) * 2 + sizeof(StaticString) * 2; - RetWithError> GetOnlineCert(); Error SetDefaultServiceDiscoveryURL(Array>& urls); Error GetServiceDiscoveryFromExtensions(const x509::Certificate& cert, Array>& urls); @@ -201,8 +198,8 @@ class CryptoHelper : public CryptoHelperItf { StaticString mServiceDiscoveryURL; x509::CertificateChain mCACerts; - Semaphore mSemaphore; - StaticAllocator mAllocator; + Semaphore mSemaphore; + AllocatorItf* mAllocator {}; }; } // namespace aos::crypto diff --git a/src/core/common/crypto/cryptoutils.cpp b/src/core/common/crypto/cryptoutils.cpp index be1ee173b..243f89c31 100644 --- a/src/core/common/crypto/cryptoutils.cpp +++ b/src/core/common/crypto/cryptoutils.cpp @@ -6,6 +6,7 @@ #include #include +#include #include "cryptoutils.hpp" diff --git a/src/core/common/crypto/mbedtls/cryptoprovider.cpp b/src/core/common/crypto/mbedtls/cryptoprovider.cpp index ec3980795..3bd3e92f3 100644 --- a/src/core/common/crypto/mbedtls/cryptoprovider.cpp +++ b/src/core/common/crypto/mbedtls/cryptoprovider.cpp @@ -574,10 +574,12 @@ Error VerifyECDSASignature(const ECDSAPublicKey& pubKey, const Array& d * Public **********************************************************************************************************************/ -Error MbedTLSCryptoProvider::Init() +Error MbedTLSCryptoProvider::Init(AllocatorItf& allocator) { LOG_DBG() << "Init mbedTLS crypto provider"; + mAllocator = &allocator; + auto ret = psa_crypto_init(); return ret != PSA_SUCCESS ? AOS_ERROR_WRAP(ret) : ErrorEnum::eNone; @@ -827,7 +829,10 @@ RetWithError> MbedTLSCryptoProvider::PEMToX509PrivKey(c { LOG_ERR() << "Create private key from PEM"; - auto res = MakeShared(&mAllocator); + auto res = MakeShared(mAllocator); + if (!res) { + return {{}, ErrorEnum::eNoMemory}; + } auto err = res->Init(pemBlob); if (!err.IsNone()) { @@ -941,7 +946,11 @@ RetWithError> MbedTLSCryptoProvider::CreateHash(Hash algorith return {nullptr, ErrorEnum::eNotSupported}; } - auto hasher = MakeUnique(&mAllocator, alg); + auto hasher = MakeUnique(mAllocator, alg); + if (!hasher) { + return {nullptr, ErrorEnum::eNoMemory}; + } + if (auto err = hasher->Init(); !err.IsNone()) { return {nullptr, AOS_ERROR_WRAP(err)}; } @@ -1055,7 +1064,10 @@ RetWithError> MbedTLSCryptoProvider::CreateAESEncoder( return {{}, AOS_ERROR_WRAP(ErrorEnum::eNotSupported)}; } - auto cipher = MakeUnique(&mAllocator); + auto cipher = MakeUnique(mAllocator); + if (!cipher) { + return {{}, ErrorEnum::eNoMemory}; + } auto err = cipher->Init(key, iv, true); if (!err.IsNone()) { @@ -1072,7 +1084,10 @@ RetWithError> MbedTLSCryptoProvider::CreateAESDecoder( return {{}, AOS_ERROR_WRAP(ErrorEnum::eNotSupported)}; } - auto cipher = MakeUnique(&mAllocator); + auto cipher = MakeUnique(mAllocator); + if (!cipher) { + return {{}, ErrorEnum::eNoMemory}; + } auto err = cipher->Init(key, iv, false); if (!err.IsNone()) { diff --git a/src/core/common/crypto/mbedtls/cryptoprovider.hpp b/src/core/common/crypto/mbedtls/cryptoprovider.hpp index 88ec9d269..7b1f43b61 100644 --- a/src/core/common/crypto/mbedtls/cryptoprovider.hpp +++ b/src/core/common/crypto/mbedtls/cryptoprovider.hpp @@ -27,9 +27,10 @@ class MbedTLSCryptoProvider : public CryptoProviderItf { /** * Initializes the object. * + * @param allocator allocator to use for temporary and key objects. * @result Error. */ - Error Init(); + Error Init(AllocatorItf& allocator); /** * Creates a new certificate based on a template. @@ -396,11 +397,6 @@ class MbedTLSCryptoProvider : public CryptoProviderItf { mutable mbedtls_pk_context mPrivKey; }; - static constexpr auto cAllocatorSize - = AOS_CONFIG_CRYPTO_PUB_KEYS_COUNT * Max(sizeof(RSAPublicKey), sizeof(ECDSAPublicKey)) - + AOS_CONFIG_CRYPTO_HASHER_COUNT * sizeof(MBedTLSHash) - + AOS_CONFIG_CRYPTO_PRIV_KEYS_COUNT * sizeof(MbedTLSRSAPrivKey); - static int VerifyTime(void* data, mbedtls_x509_crt* crt, int depth, uint32_t* flags); static RetWithError