fix index out of bounds in CacheBucket::CompactState (#532)

* use PayloadIndex for indexing into payload array
* naming cleanup
* fix metadata index in CacheBucket::CompactState

1 files changed, 24 insertions, 25 deletions

diff --git a/src/zenserver/cache/cachedisklayer.cpp b/src/zenserver/cache/cachedisklayer.cpp
index 9bb75480e..a6cb54444 100644
--- a/src/zenserver/cache/cachedisklayer.cpp
+++ b/src/zenserver/cache/cachedisklayer.cpp
@@ -246,12 +246,10 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir, bo
 		const auto _ =
 			MakeGuard([&] { ZEN_INFO("read store manifest '{}' in {}", ManifestPath, NiceTimeSpanMs(Timer.GetElapsedTimeMs())); });
 
-		const uint64_t kInvalidIndex = ~(0ull);
-
 		const uint64_t Count = Manifest["Count"sv].AsUInt64(0);
 		if (Count != 0)
 		{
-			std::vector<size_t> KeysIndexes;
+			std::vector<PayloadIndex> KeysIndexes;
 			KeysIndexes.reserve(Count);
 			CbArrayView KeyArray = Manifest["Keys"sv].AsArrayView();
 			for (CbFieldView& KeyView : KeyArray)
@@ -262,15 +260,15 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir, bo
 				}
 				else
 				{
-					KeysIndexes.push_back(kInvalidIndex);
+					KeysIndexes.push_back(PayloadIndex());
 				}
 			}
 			size_t		KeyIndexOffset = 0;
 			CbArrayView TimeStampArray = Manifest["Timestamps"].AsArrayView();
 			for (CbFieldView& TimeStampView : TimeStampArray)
 			{
-				const size_t KeyIndex = KeysIndexes[KeyIndexOffset++];
-				if (KeyIndex != kInvalidIndex)
+				const PayloadIndex KeyIndex = KeysIndexes[KeyIndexOffset++];
+				if (KeyIndex)
 				{
 					m_AccessTimes[KeyIndex] = TimeStampView.AsInt64();
 				}
@@ -284,9 +282,9 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir, bo
 				auto RawSizeIt = RawSizeArray.CreateViewIterator();
 				while (RawHashIt != CbFieldViewIterator())
 				{
-					const size_t KeyIndex = KeysIndexes[KeyIndexOffset++];
+					const PayloadIndex KeyIndex = KeysIndexes[KeyIndexOffset++];
 
-					if (KeyIndex != kInvalidIndex)
+					if (KeyIndex)
 					{
 						uint64_t RawSize = RawSizeIt.AsUInt64();
 						IoHash	 RawHash = RawHashIt.AsHash();
@@ -1729,8 +1727,8 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 	std::unordered_set<IoHash, IoHash::Hasher> ExpiredCacheKeys(ExpiredCacheKeySpan.begin(), ExpiredCacheKeySpan.end());
 
 	std::vector<DiskIndexEntry>		 ExpiredStandaloneEntries;
-	IndexMap						 Index;
-	std::vector<BucketPayload>		 Payloads;
+	IndexMap						 IndexSnapshot;
+	std::vector<BucketPayload>		 PayloadsSnapshot;
 	BlockStore::ReclaimSnapshotState BlockStoreState;
 	{
 		bool Expected = false;
@@ -1741,7 +1739,6 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 		}
 		auto FlushingGuard = MakeGuard([&] { m_IsFlushing.store(false); });
 
-		std::vector<AccessTime> AccessTimes;
 		{
 			ZEN_TRACE_CPU("Z$::Disk::Bucket::CollectGarbage::State");
 			RwLock::SharedLockScope IndexLock(m_IndexLock);
@@ -1755,23 +1752,23 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 
 			BlockStoreState = m_BlockStore.GetReclaimSnapshotState();
 
-			Payloads	= m_Payloads;
-			AccessTimes = m_AccessTimes;
-			Index		= m_Index;
-
 			for (const IoHash& Key : ExpiredCacheKeys)
 			{
-				if (auto It = Index.find(Key); It != Index.end())
+				if (auto It = m_Index.find(Key); It != m_Index.end())
 				{
-					const BucketPayload& Payload = Payloads[It->second];
-					DiskIndexEntry		 Entry	 = {.Key = It->first, .Location = Payload.Location};
-					if (Entry.Location.Flags & DiskLocation::kStandaloneFile)
+					const BucketPayload& Payload = m_Payloads[It->second];
+					if (Payload.Location.Flags & DiskLocation::kStandaloneFile)
 					{
+						DiskIndexEntry Entry = {.Key = Key, .Location = Payload.Location};
 						Entry.Location.Flags |= DiskLocation::kTombStone;
 						ExpiredStandaloneEntries.push_back(Entry);
 					}
 				}
 			}
+
+			PayloadsSnapshot = m_Payloads;
+			IndexSnapshot	 = m_Index;
+
 			if (GcCtx.IsDeletionMode())
 			{
 				IndexLock.ReleaseNow();
@@ -1836,7 +1833,7 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 		}
 	}
 
-	TotalChunkCount = Index.size();
+	TotalChunkCount = IndexSnapshot.size();
 
 	std::vector<BlockStoreLocation> ChunkLocations;
 	BlockStore::ChunkIndexArray		KeepChunkIndexes;
@@ -1846,10 +1843,10 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 	ChunkIndexToChunkHash.reserve(TotalChunkCount);
 	{
 		TotalChunkCount = 0;
-		for (const auto& Entry : Index)
+		for (const auto& Entry : IndexSnapshot)
 		{
 			size_t				EntryIndex	 = Entry.second;
-			const DiskLocation& DiskLocation = Payloads[EntryIndex].Location;
+			const DiskLocation& DiskLocation = PayloadsSnapshot[EntryIndex].Location;
 
 			if (DiskLocation.Flags & DiskLocation::kStandaloneFile)
 			{
@@ -1908,7 +1905,7 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 					const IoHash&			  ChunkHash	  = ChunkIndexToChunkHash[ChunkIndex];
 					size_t					  EntryIndex  = m_Index[ChunkHash];
 					BucketPayload&			  Payload	  = m_Payloads[EntryIndex];
-					if (Payloads[Index[ChunkHash]].Location != m_Payloads[EntryIndex].Location)
+					if (PayloadsSnapshot[IndexSnapshot[ChunkHash]].Location != m_Payloads[EntryIndex].Location)
 					{
 						// Entry has been updated while GC was running, ignore the move
 						continue;
@@ -1921,7 +1918,7 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 					const IoHash&  ChunkHash  = ChunkIndexToChunkHash[ChunkIndex];
 					size_t		   EntryIndex = m_Index[ChunkHash];
 					BucketPayload& Payload	  = m_Payloads[EntryIndex];
-					if (Payloads[Index[ChunkHash]].Location != Payload.Location)
+					if (PayloadsSnapshot[IndexSnapshot[ChunkHash]].Location != Payload.Location)
 					{
 						// Entry has been updated while GC was running, ignore the delete
 						continue;
@@ -2813,7 +2810,9 @@ ZenCacheDiskLayer::CacheBucket::CompactReferences(RwLock::ExclusiveLockScope&)
 	}
 	m_FirstReferenceIndex.swap(FirstReferenceIndex);
 	m_ReferenceHashes.swap(NewReferenceHashes);
+	m_ReferenceHashes.shrink_to_fit();
 	m_NextReferenceHashesIndexes.swap(NewNextReferenceHashesIndexes);
+	m_NextReferenceHashesIndexes.shrink_to_fit();
 	m_ReferenceCount = m_ReferenceHashes.size();
 }
 
@@ -2975,7 +2974,7 @@ ZenCacheDiskLayer::CacheBucket::CompactState(std::vector<BucketPayload>&  Payloa
 		if (Payload.MetaData)
 		{
 			MetaDatas.push_back(m_MetaDatas[Payload.MetaData]);
-			Payload.MetaData = MetaDataIndex(m_MetaDatas.size() - 1);
+			Payload.MetaData = MetaDataIndex(MetaDatas.size() - 1);
 		}
 		if (Payload.MemCached)
 		{