From be862b6418ac2067a0298e9394165efd38385725 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 18 Jun 2026 16:51:43 +0300 Subject: [PATCH 1/4] spaceallocator: fix mAllocationCount leak in ResizeSpace ResizeSpace called Partition::Free + Partition::Allocate which toggled mAllocationCount on every resize. Since mAllocationCount > 0 suppresses disk re-reads, mAvailableSize became stale and lazy eviction stopped triggering, leading to ENOSPC on subsequent allocations. Add Partition::AdjustSize that adjusts mAvailableSize and triggers eviction when needed without touching mAllocationCount. ResizeSpace now calls AdjustSize instead of the Free/Allocate pair on the partition, keeping mAllocationCount as a pure count of live Space objects. Signed-off-by: Mykola Solianko --- .../common/spaceallocator/spaceallocator.hpp | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/core/common/spaceallocator/spaceallocator.hpp b/src/core/common/spaceallocator/spaceallocator.hpp index 0aa69f56a..35cf8008d 100644 --- a/src/core/common/spaceallocator/spaceallocator.hpp +++ b/src/core/common/spaceallocator/spaceallocator.hpp @@ -157,6 +157,42 @@ class Partition { return ErrorEnum::eNone; } + /** + * Adjusts allocated size without changing allocation count. + * Used by resize operations so that mAllocationCount tracks live Space objects only. + * + * @param oldSize previously allocated size to return. + * @param newSize new size to reserve. + * @return Error. + */ + Error AdjustSize(size_t oldSize, size_t newSize) + { + LockGuard lock {mMutex}; + + mAvailableSize += oldSize; + + if (newSize > mAvailableSize) { + if (mOutdatedItems.Size() == 0) { + return Error(ErrorEnum::eNoMemory, "not enough space"); + } + + auto [freedSize, err] = RemoveOutdatedItems(newSize - mAvailableSize); + if (!err.IsNone()) { + return err; + } + + mAvailableSize += freedSize; + + if (newSize > mAvailableSize) { + return Error(ErrorEnum::eNoMemory, "not enough space"); + } + } + + mAvailableSize -= newSize; + + return ErrorEnum::eNone; + } + /** * Add outdated item. * @@ -501,13 +537,12 @@ class SpaceAllocator : public SpaceAllocatorItf, public SpaceAllocatorStorage { } Free(oldSize); - mPartition->Free(oldSize); if (auto err = Allocate(newSize); !err.IsNone()) { return err; } - if (auto err = mPartition->Allocate(newSize); !err.IsNone()) { + if (auto err = mPartition->AdjustSize(oldSize, newSize); !err.IsNone()) { Free(newSize); return err; From 2ad78eaf20db63c711ded39589192d1aedec3474 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 18 Jun 2026 16:56:57 +0300 Subject: [PATCH 2/4] imagemanager: protect in-progress blobs and layers in RemoveOrphans RemoveOrphans sweeps the disk and deletes everything not referenced by any DB item. Blobs and layer dirs for concurrently-installing items are on disk but not yet committed to DB, so they were deleted as orphans. Before calling RemoveOrphanBlobs and RemoveOrphanLayers, add the blob path and corresponding layer dir path for every digest in mInProgressBlobs to the respective used-sets. This prevents the sweep from touching files that belong to an in-progress installation. Signed-off-by: Mykola Solianko --- src/core/sm/imagemanager/imagemanager.cpp | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/core/sm/imagemanager/imagemanager.cpp b/src/core/sm/imagemanager/imagemanager.cpp index 5b9bc7d5d..8959acae3 100644 --- a/src/core/sm/imagemanager/imagemanager.cpp +++ b/src/core/sm/imagemanager/imagemanager.cpp @@ -1292,6 +1292,30 @@ RetWithError ImageManager::RemoveOrphans() } } + for (const auto& digest : mInProgressBlobs) { + StaticString blobPath; + + if (auto err = CreateBlobPath(digest, blobPath); !err.IsNone()) { + LOG_ERR() << "Failed to create path for in-progress blob" << Log::Field("digest", digest) + << Log::Field(err); + continue; + } + + if (auto err = usedBlobs->PushBack(blobPath); !err.IsNone()) { + LOG_ERR() << "Failed to protect in-progress blob" << Log::Field(err); + } + + StaticString layerPath; + + if (auto err = CreateLayerPath(digest, layerPath); !err.IsNone()) { + continue; + } + + if (auto err = usedLayers->PushBack(layerPath); !err.IsNone()) { + LOG_ERR() << "Failed to protect in-progress layer" << Log::Field(err); + } + } + size_t removedSize = 0; size_t size = 0; Error err; From 86ef3800ebd2514b4df861da4a1bfc7bdb2ba9c0 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 18 Jun 2026 17:03:37 +0300 Subject: [PATCH 3/4] imagemanager: replace broad RemoveOrphans in RemoveItem with targeted deletion RemoveOrphans sweeps the entire disk and deletes everything not referenced by the DB. When called from RemoveItem during eviction, it raced with concurrent installs: files on disk but not yet in DB were deleted as orphans, causing "Failed to unpack layer" errors. Replace the RemoveOrphans call in RemoveItem with targeted deletion: 1. Collect the blobs/layers still needed by all remaining DB items (CalcRemainingBlobsAndLayers, skipping the evicted item). 2. Load the evicted item's own manifest to enumerate exactly what it owns (FindUpdateItemData + DeleteEvictedItemFiles). 3. Delete only the evicted item's exclusive files, blobs and layers not present in the remaining set. RemoveOrphans is retained as a fallback for the allocation-failure path where no item data is available. Signed-off-by: Mykola Solianko --- src/core/sm/imagemanager/imagemanager.cpp | 186 +++++++++++++++++++++- src/core/sm/imagemanager/imagemanager.hpp | 6 + 2 files changed, 188 insertions(+), 4 deletions(-) diff --git a/src/core/sm/imagemanager/imagemanager.cpp b/src/core/sm/imagemanager/imagemanager.cpp index 8959acae3..ad7c42a30 100644 --- a/src/core/sm/imagemanager/imagemanager.cpp +++ b/src/core/sm/imagemanager/imagemanager.cpp @@ -339,17 +339,195 @@ RetWithError ImageManager::RemoveItem(const String& id, const String& ve LOG_DBG() << "Remove item" << Log::Field("id", id) << Log::Field("version", version); + UpdateItemData evictedData; + + if (!FindUpdateItemData(id, version, evictedData)) { + if (auto err = mStorage->RemoveUpdateItem(id, version); !err.IsNone()) { + LOG_ERR() << "Failed to remove update item" << Log::Field("itemID", id) << Log::Field("version", version) + << Log::Field(err); + } + + auto [size, err] = RemoveOrphans(); + if (!err.IsNone()) { + return {0, AOS_ERROR_WRAP(err)}; + } + + return size; + } + + // Memory budget: remainingBlobs + remainingLayers + itemsData (freed in CalcRemainingBlobsAndLayers scope) + // + 1 manifest/config pair per CalcItemBlobsAndLayers call — fits existing cAllocatorSize. + auto remainingBlobs = MakeUnique, cMaxNumInstalledBlobs>>(&mAllocator); + auto remainingLayers = MakeUnique, cMaxNumInstalledLayers>>(&mAllocator); + + if (!remainingBlobs || !remainingLayers) { + if (auto err = mStorage->RemoveUpdateItem(id, version); !err.IsNone()) { + LOG_ERR() << "Failed to remove update item" << Log::Field("itemID", id) << Log::Field("version", version) + << Log::Field(err); + } + + auto [size, err] = RemoveOrphans(); + if (!err.IsNone()) { + return {0, AOS_ERROR_WRAP(err)}; + } + + return size; + } + + if (auto err = CalcRemainingBlobsAndLayers(id, version, *remainingBlobs, *remainingLayers); !err.IsNone()) { + LOG_ERR() << "Failed to calculate remaining blobs and layers" << Log::Field(err); + } + if (auto err = mStorage->RemoveUpdateItem(id, version); !err.IsNone()) { LOG_ERR() << "Failed to remove update item" << Log::Field("itemID", id) << Log::Field("version", version) << Log::Field(err); } - auto [size, err] = RemoveOrphans(); - if (!err.IsNone()) { - return RetWithError(0, AOS_ERROR_WRAP(err)); + return DeleteEvictedItemFiles(evictedData, *remainingBlobs, *remainingLayers); +} + +bool ImageManager::FindUpdateItemData(const String& id, const String& version, UpdateItemData& data) +{ + auto itemsData = MakeUnique(&mAllocator); + if (!itemsData) { + return false; + } + + if (auto err = mStorage->GetAllUpdateItems(*itemsData); !err.IsNone()) { + return false; + } + + for (const auto& item : *itemsData) { + if (item.mID == id && item.mVersion == version) { + data = item; + + return true; + } + } + + return false; +} + +Error ImageManager::CalcRemainingBlobsAndLayers(const String& skipID, const String& skipVersion, + Array>& blobs, Array>& layers) +{ + auto itemsData = MakeUnique(&mAllocator); + if (!itemsData) { + return AOS_ERROR_WRAP(ErrorEnum::eNoMemory); + } + + if (auto err = mStorage->GetAllUpdateItems(*itemsData); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + for (const auto& item : *itemsData) { + if (item.mID == skipID && item.mVersion == skipVersion) { + continue; + } + + if (auto err = CalcItemBlobsAndLayers(item, blobs, layers); !err.IsNone()) { + LOG_ERR() << "Failed to calculate item blobs and layers" << Log::Field("itemID", item.mID) + << Log::Field("version", item.mVersion) << Log::Field(err); + } + } + + return ErrorEnum::eNone; +} + +RetWithError ImageManager::DeleteEvictedItemFiles(const UpdateItemData& evictedData, + const Array>& remainingBlobs, const Array>& remainingLayers) +{ + // Only delete files exclusively owned by the evicted item (not referenced by any remaining DB item). + // Files for concurrent in-progress installs are never listed in evictedData's manifest, + // so they are safe from deletion here. + size_t freedSize = 0; + StaticString path; + StaticString manifestPath; + + auto manifest = MakeUnique(&mAllocator); + if (!manifest) { + return {0, ErrorEnum::eNone}; + } + + if (auto err = CreateBlobPath(evictedData.mManifestDigest, manifestPath); !err.IsNone()) { + return {0, ErrorEnum::eNone}; + } + + if (auto err = mOCISpec->LoadImageManifest(manifestPath, *manifest); !err.IsNone()) { + LOG_ERR() << "Failed to load manifest for evicted item" << Log::Field(err); + } else { + if (manifest->mItemConfig.HasValue()) { + if (auto err = CreateBlobPath(manifest->mItemConfig->mDigest, path); err.IsNone()) { + if (remainingBlobs.Find(path) == remainingBlobs.end()) { + auto [sz, szErr] = fs::CalculateSize(path); + freedSize += sz; + + if (auto removeErr = fs::RemoveAll(path); !removeErr.IsNone()) { + LOG_ERR() << "Failed to remove orphaned item config" << Log::Field(removeErr); + } + } + } + } + + if (evictedData.mType == UpdateItemTypeEnum::eService) { + if (auto err = CreateBlobPath(manifest->mConfig.mDigest, path); err.IsNone()) { + auto imageConfigPath = path; + + auto imageConfig = MakeUnique(&mAllocator); + if (imageConfig) { + if (auto err = mOCISpec->LoadImageConfig(imageConfigPath, *imageConfig); err.IsNone()) { + for (const auto& diffID : imageConfig->mRootfs.mDiffIDs) { + if (auto err = CreateLayerPath(diffID, path); err.IsNone()) { + if (remainingLayers.Find(path) == remainingLayers.end()) { + auto [sz, szErr] = fs::CalculateSize(path); + freedSize += sz; + + if (auto removeErr = fs::RemoveAll(path); !removeErr.IsNone()) { + LOG_ERR() << "Failed to remove orphaned layer" << Log::Field(removeErr); + } + } + } + } + } else { + LOG_ERR() << "Failed to load image config for evicted item" << Log::Field(err); + } + } + + if (remainingBlobs.Find(imageConfigPath) == remainingBlobs.end()) { + auto [sz, szErr] = fs::CalculateSize(imageConfigPath); + freedSize += sz; + + if (auto removeErr = fs::RemoveAll(imageConfigPath); !removeErr.IsNone()) { + LOG_ERR() << "Failed to remove orphaned image config" << Log::Field(removeErr); + } + } + } + } + + for (const auto& layer : manifest->mLayers) { + if (auto err = CreateBlobPath(layer.mDigest, path); err.IsNone()) { + if (remainingBlobs.Find(path) == remainingBlobs.end()) { + auto [sz, szErr] = fs::CalculateSize(path); + freedSize += sz; + + if (auto removeErr = fs::RemoveAll(path); !removeErr.IsNone()) { + LOG_ERR() << "Failed to remove orphaned layer blob" << Log::Field(removeErr); + } + } + } + } + } + + if (remainingBlobs.Find(manifestPath) == remainingBlobs.end()) { + auto [sz, szErr] = fs::CalculateSize(manifestPath); + freedSize += sz; + + if (auto removeErr = fs::RemoveAll(manifestPath); !removeErr.IsNone()) { + LOG_ERR() << "Failed to remove orphaned manifest" << Log::Field(removeErr); + } } - return size; + return {freedSize, ErrorEnum::eNone}; } Error ImageManager::CreateBlobPath(const String& digest, String& path) const diff --git a/src/core/sm/imagemanager/imagemanager.hpp b/src/core/sm/imagemanager/imagemanager.hpp index 60c65cc90..96b463687 100644 --- a/src/core/sm/imagemanager/imagemanager.hpp +++ b/src/core/sm/imagemanager/imagemanager.hpp @@ -146,6 +146,12 @@ class ImageManager : public ImageManagerItf, public ItemInfoProviderItf, public Error HandleItemsIntegrity(); Error CalcItemBlobsAndLayers(const UpdateItemData& itemData, Array>& itemBlobs, Array>& itemLayers); + Error CalcRemainingBlobsAndLayers(const String& skipID, const String& skipVersion, + Array>& blobs, Array>& layers); + bool FindUpdateItemData(const String& id, const String& version, UpdateItemData& data); + RetWithError DeleteEvictedItemFiles(const UpdateItemData& evictedData, + const Array>& remainingBlobs, + const Array>& remainingLayers); RetWithError RemoveOrphanBlobs(const Array>& usedBlobs); RetWithError RemoveOrphanLayers(const Array>& usedLayers); RetWithError RemoveOrphans(); From 3979a4356abe99c33c567389a44301c86ebfbfb7 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Mon, 22 Jun 2026 18:47:44 +0300 Subject: [PATCH 4/4] imagemanager: fix stale cancel flag blocking new download after cancellation When a download is cancelled to process a new desired status, mCancel is set to true. Once the cancelled download finishes and mInProgress becomes false, a subsequent DownloadUpdateItems call would enter StartAction and immediately return false because mCancel was still set, causing the new download to fail with eCanceled without attempting any network activity. Only reject starting a new action when mCancel is true and mInProgress is also true, meaning there is an active download being cancelled. A stale mCancel with no in-progress action should not block the next download. Signed-off-by: Mykola Solianko --- src/core/cm/imagemanager/imagemanager.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/cm/imagemanager/imagemanager.cpp b/src/core/cm/imagemanager/imagemanager.cpp index b66eb8146..3c6ff7bff 100644 --- a/src/core/cm/imagemanager/imagemanager.cpp +++ b/src/core/cm/imagemanager/imagemanager.cpp @@ -1417,9 +1417,11 @@ bool ImageManager::StartAction() mCondVar.Wait(lock, [this]() { return !mInProgress || mCancel; }); - if (mCancel) { - mCancel = false; + const bool cancelledWhileRunning = mCancel && mInProgress; + + mCancel = false; + if (cancelledWhileRunning) { return false; }