Skip to content

Redesign allocator #636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
al1img merged 13 commits into from
Aug 4, 2026
Merged

Redesign allocator #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6103885
memory: check allocation before constructing objects
al1img Jul 30, 2026
c110472
tools: check MakeUnique/MakeShared allocation failures everywhere
al1img Jul 30, 2026
afda74d
tools: replace static allocator with AllocatorItf/HeapAllocator
al1img Jul 30, 2026
c958f60
common: migrate to injected AllocatorItf
al1img Jul 30, 2026
d9b4272
cm: migrate to injected AllocatorItf
al1img Jul 30, 2026
7b59fe2
iam: migrate to injected AllocatorItf
al1img Jul 30, 2026
863ed40
sm: migrate to injected AllocatorItf
al1img Jul 30, 2026
56dc6f1
sm: networkmanager: fix mInstanceNetworkInfos and mBatchEntries capacity
al1img Jul 31, 2026
8073279
common: tools: remove unmatched suppression: templateRecursion
al1img Aug 3, 2026
34c5c12
sm: launcher: fix missed FinishLaunch on early return
al1img Aug 3, 2026
425187f
common: iam: tests: suppress unusedStructMember for mAllocator
al1img Aug 3, 2026
15f6b2f
tools: make SharedPtr control block ref count thread safe
al1img Aug 3, 2026
485e1b2
common: pkcs11: close session on SessionContext alloc failure
al1img Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/core/cm/alerts/alerts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,12 @@ class SetTimestamp : public StaticVisitor<void> {
* 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;
Expand Down Expand Up @@ -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());

Expand All @@ -257,7 +261,12 @@ Error Alerts::SendAlerts()

bool Alerts::IsDuplicated(const AlertVariant& alert)
{
auto alertCopy = MakeUnique<AlertVariant>(&mAllocator, alert);
auto alertCopy = MakeUnique<AlertVariant>(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())));
Expand All @@ -268,7 +277,12 @@ bool Alerts::IsDuplicated(const AlertVariant& alert)

UniquePtr<aos::Alerts> Alerts::CreatePackage()
{
auto package = MakeUnique<aos::Alerts>(&mAllocator);
auto package = MakeUnique<aos::Alerts>(mAllocator);
if (!package) {
LOG_ERR() << "Can't allocate alerts package" << Log::Field(ErrorEnum::eNoMemory);

return package;
}

const auto count = Min<size_t>(cAlertItemsCount, mAlerts.Size());

Expand Down
6 changes: 3 additions & 3 deletions src/core/cm/alerts/alerts.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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<size_t>(AlertTagEnum::eNumAlertTags);

Expand All @@ -111,7 +111,7 @@ class Alerts : public ReceiverItf,
void ShrinkCache(size_t count);
void NotifyListeners(const AlertVariant& alert);

StaticAllocator<cAllocatorSize> mAllocator;
AllocatorItf* mAllocator {};
alerts::Config mConfig;
cm::alerts::SenderItf* mSender {};
cloudconnection::CloudConnectionItf* mCloudConnection {};
Expand Down
21 changes: 13 additions & 8 deletions src/core/cm/alerts/tests/alerts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <core/common/tests/mocks/cloudconnectionmock.hpp>
#include <core/common/tests/utils/log.hpp>
#include <core/common/tests/utils/utils.hpp>
#include <core/common/tools/heapallocator.hpp>

#include <core/cm/alerts/alerts.hpp>

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -311,7 +316,7 @@ TEST_F(AlertsTest, PackagesAreSentOnReconnect)

std::vector<aos::Alerts> 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();
Expand Down Expand Up @@ -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(
Expand Down
55 changes: 30 additions & 25 deletions src/core/cm/imagemanager/imagemanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@
* Public
**********************************************************************************************************************/

Error ImageManager::Init(const Config& config, StorageItf& storage, BlobInfoProviderItf& blobInfoProvider,
spaceallocator::SpaceAllocatorItf& downloadingSpaceAllocator,
Error ImageManager::Init(AllocatorItf& allocator, const Config& config, StorageItf& storage,

Check warning on line 17 in src/core/cm/imagemanager/imagemanager.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 11 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=aosedge_aos_core_lib_cpp&issues=AZ-4pgHoSOjhs3Jv3RoZ&open=AZ-4pgHoSOjhs3Jv3RoZ&pullRequest=636
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;
Expand All @@ -33,7 +34,7 @@
mFileInfoProvider = &fileInfoProvider;
mOCISpec = &ociSpec;

auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!items) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -130,7 +131,7 @@
statuses[i].mError = ErrorEnum::eNone;
}

auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!storedItems) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -220,7 +221,7 @@
statuses[i].mError = ErrorEnum::eNone;
}

auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!storedItems) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -300,7 +301,7 @@

LOG_DBG() << "Get update items statuses";

auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!items) {
return ErrorEnum::eNoMemory;
}
Expand Down Expand Up @@ -358,7 +359,7 @@

LOG_DBG() << "Get index digest" << Log::Field("itemID", itemID) << Log::Field("version", version);

auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumItemVersions>>(&mAllocator);
auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumItemVersions>>(mAllocator);
if (!items) {
return ErrorEnum::eNoMemory;
}
Expand Down Expand Up @@ -429,7 +430,7 @@

LOG_DBG() << "Get item current version" << Log::Field("itemID", itemID);

auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumItemVersions>>(&mAllocator);
auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumItemVersions>>(mAllocator);
if (!items) {
return ErrorEnum::eNoMemory;
}
Expand Down Expand Up @@ -458,7 +459,7 @@

LOG_DBG() << "Remove item" << Log::Field("id", id) << Log::Field("version", version);

auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!storedItems) {
return {0, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)};
}
Expand Down Expand Up @@ -508,7 +509,7 @@

LOG_DBG() << "Remove outdated items";

auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto items = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!items) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -586,7 +587,7 @@
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);
Expand Down Expand Up @@ -884,7 +885,7 @@
return AOS_ERROR_WRAP(err);
}

auto imageIndex = MakeUnique<oci::ImageIndex>(&mAllocator);
auto imageIndex = MakeUnique<oci::ImageIndex>(mAllocator);
if (!imageIndex) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand All @@ -898,7 +899,7 @@
LOG_DBG() << "Processing manifests" << Log::Field("count", imageIndex->mManifests.Size());

for (const auto& manifestDescriptor : imageIndex->mManifests) {
auto manifest = MakeUnique<oci::ImageManifest>(&mAllocator);
auto manifest = MakeUnique<oci::ImageManifest>(mAllocator);
if (!manifest) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -1080,7 +1081,11 @@
{
LOG_DBG() << "Ensure blob" << Log::Field("digest", digest);

auto blobInfo = MakeUnique<BlobInfo>(&mAllocator);
auto blobInfo = MakeUnique<BlobInfo>(mAllocator);
if (!blobInfo) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}

UniquePtr<spaceallocator::SpaceItf> downloadingSpace;

do {
Expand Down Expand Up @@ -1132,7 +1137,7 @@
return AOS_ERROR_WRAP(err);
}

auto blobsInfo = MakeUnique<StaticArray<BlobInfo, 1>>(&mAllocator);
auto blobsInfo = MakeUnique<StaticArray<BlobInfo, 1>>(mAllocator);
if (!blobsInfo) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -1190,7 +1195,7 @@
return AOS_ERROR_WRAP(err);
}

auto expectedSHA256 = MakeUnique<StaticArray<uint8_t, crypto::cSHA256Size>>(&mAllocator);
auto expectedSHA256 = MakeUnique<StaticArray<uint8_t, crypto::cSHA256Size>>(mAllocator);
if (!expectedSHA256) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -1223,7 +1228,7 @@
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);
}
Expand Down Expand Up @@ -1275,7 +1280,7 @@
<< 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);
Expand Down Expand Up @@ -1514,7 +1519,7 @@
return AOS_ERROR_WRAP(err);
}

auto expectedSHA256 = MakeUnique<StaticArray<uint8_t, crypto::cSHA256Size>>(&mAllocator);
auto expectedSHA256 = MakeUnique<StaticArray<uint8_t, crypto::cSHA256Size>>(mAllocator);
if (!expectedSHA256) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -1543,7 +1548,7 @@
return AOS_ERROR_WRAP(err);
}

auto imageIndex = MakeUnique<oci::ImageIndex>(&mAllocator);
auto imageIndex = MakeUnique<oci::ImageIndex>(mAllocator);
if (!imageIndex) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand All @@ -1562,7 +1567,7 @@
return AOS_ERROR_WRAP(err);
}

auto manifest = MakeUnique<oci::ImageManifest>(&mAllocator);
auto manifest = MakeUnique<oci::ImageManifest>(mAllocator);
if (!manifest) {
return AOS_ERROR_WRAP(ErrorEnum::eNoMemory);
}
Expand Down Expand Up @@ -1605,7 +1610,7 @@
continue;
}

auto imageIndex = MakeUnique<oci::ImageIndex>(&mAllocator);
auto imageIndex = MakeUnique<oci::ImageIndex>(mAllocator);
if (!imageIndex) {
continue;
}
Expand All @@ -1625,7 +1630,7 @@
continue;
}

auto manifest = MakeUnique<oci::ImageManifest>(&mAllocator);
auto manifest = MakeUnique<oci::ImageManifest>(mAllocator);
if (!manifest) {
continue;
}
Expand Down Expand Up @@ -1659,7 +1664,7 @@

size_t totalSize = 0;

auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(&mAllocator);
auto storedItems = MakeUnique<StaticArray<ItemInfo, cMaxNumUpdateItems>>(mAllocator);
if (!storedItems) {
return {0, AOS_ERROR_WRAP(ErrorEnum::eNoMemory)};
}
Expand All @@ -1685,7 +1690,7 @@
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 {
Expand Down
Loading
Loading