Move MarkAsDeleteOnClose to BasicFile

3 files changed, 58 insertions, 45 deletions

diff --git a/zenstore/basicfile.cpp b/zenstore/basicfile.cpp
index 17a57309d..efa91f107 100644
--- a/zenstore/basicfile.cpp
+++ b/zenstore/basicfile.cpp
@@ -296,6 +296,31 @@ BasicFile::SetFileSize(uint64_t FileSize)
 #endif
 }
 
+void
+BasicFile::MarkAsDeleteOnClose(std::error_code& Ec)
+{
+	Ec.clear();
+#if ZEN_PLATFORM_WINDOWS
+	FILE_DISPOSITION_INFO Fdi{};
+	Fdi.DeleteFile = TRUE;
+	BOOL Success   = SetFileInformationByHandle(m_FileHandle, FileDispositionInfo, &Fdi, sizeof Fdi);
+	if (!Success)
+	{
+		Ec = MakeErrorCodeFromLastError();
+	}
+#elif ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC
+	std::filesystem::path SourcePath = PathFromHandle(FileRef.FileHandle);
+	if (unlink(SourcePath.c_str()) < 0)
+	{
+		int UnlinkError = zen::GetLastError();
+		if (UnlinkError != ENOENT)
+		{
+			Ec = MakeErrorCode(UnlinkError);
+		}
+	}
+#endif
+}
+
 //////////////////////////////////////////////////////////////////////////
 
 TemporaryFile::~TemporaryFile()
diff --git a/zenstore/compactcas.cpp b/zenstore/compactcas.cpp
index 99f00f5af..82e0d49bf 100644
--- a/zenstore/compactcas.cpp
+++ b/zenstore/compactcas.cpp
@@ -82,44 +82,21 @@ namespace {
 		return Path.ToPath();
 	}
 
-	void MarkFileAsDeleteOnClose(void* FileHandle)
-	{
-#if ZEN_PLATFORM_WINDOWS
-		FILE_DISPOSITION_INFO Fdi{};
-		Fdi.DeleteFile = TRUE;
-		BOOL Success   = SetFileInformationByHandle(FileHandle, FileDispositionInfo, &Fdi, sizeof Fdi);
-		if (!Success)
-		{
-			ZEN_WARN("Failed to flag temporary payload file '{}' for deletion: '{}'", PathFromHandle(FileHandle), GetLastErrorAsString());
-		}
-#elif ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC
-		std::filesystem::path SourcePath = PathFromHandle(FileRef.FileHandle);
-		if (unlink(SourcePath.c_str()) < 0)
-		{
-			int UnlinkError = zen::GetLastError();
-			if (UnlinkError != ENOENT)
-			{
-				ZEN_WARN("unlink of CAS payload file failed ('{}')", GetSystemErrorAsString(UnlinkError));
-			}
-		}
-#endif
-	}
-
 }  // namespace
 
 struct CasContainerStrategy::ChunkBlock
 {
 	explicit ChunkBlock(const std::filesystem::path& BlockPath);
 	~ChunkBlock();
-	const std::filesystem::path GetPath() const;
-	void						Open();
-	void						Create(uint64_t InitialSize);
-	void						MarkAsDeleteOnClose();
-	uint64_t					FileSize();
-	IoBuffer					GetRange(uint64_t Offset, uint64_t Size);
-	void						Read(void* Data, uint64_t Size, uint64_t FileOffset);
-	void						Write(const void* Data, uint64_t Size, uint64_t FileOffset);
-	void						Flush();
+	const std::filesystem::path& GetPath() const;
+	void						 Open();
+	void						 Create(uint64_t InitialSize);
+	void						 MarkAsDeleteOnClose(std::error_code& Ec);
+	uint64_t					 FileSize();
+	IoBuffer					 GetRange(uint64_t Offset, uint64_t Size);
+	void						 Read(void* Data, uint64_t Size, uint64_t FileOffset);
+	void						 Write(const void* Data, uint64_t Size, uint64_t FileOffset);
+	void						 Flush();
 	void StreamByteRange(uint64_t FileOffset, uint64_t Size, std::function<void(const void* Data, uint64_t Size)>&& ChunkFun);
 
 private:
@@ -142,7 +119,7 @@ CasContainerStrategy::ChunkBlock::~ChunkBlock()
 	}
 }
 
-const std::filesystem::path
+const std::filesystem::path&
 CasContainerStrategy::ChunkBlock::GetPath() const
 {
 	return m_Path;
@@ -194,19 +171,19 @@ CasContainerStrategy::ChunkBlock::FileSize()
 }
 
 void
-CasContainerStrategy::ChunkBlock::MarkAsDeleteOnClose()
+CasContainerStrategy::ChunkBlock::MarkAsDeleteOnClose(std::error_code& Ec)
 {
 	RwLock::ExclusiveLockScope _(m_OpenLock);
-	if (!m_IsOpened.load())
+	if (m_IsOpened.load())
 	{
-		if (std::filesystem::is_regular_file(m_Path))
-		{
-			std::filesystem::remove(m_Path);
-		}
+		m_SmallObjectFile.MarkAsDeleteOnClose(Ec);
 		return;
 	}
-	void* FileHandle = m_SmallObjectFile.Handle();
-	MarkFileAsDeleteOnClose(FileHandle);
+	if (std::filesystem::is_regular_file(m_Path))
+	{
+		Ec.clear();
+		std::filesystem::remove(m_Path, Ec);
+	}
 }
 
 IoBuffer
@@ -356,8 +333,8 @@ CasContainerStrategy::FindChunk(const IoHash& ChunkHash)
 
 		if (auto BlockIt = m_ChunkBlocks.find(Location.BlockIndex); BlockIt != m_ChunkBlocks.end())
 		{
-			if (BlockIt->second)  // This happens if the data associated with the block is not found - ie the ucas file is deleted but the
-								  // index not updated
+			if (BlockIt->second)  // This happens if the data associated with the block is not found - ie the ucas file is deleted but
+								  // the index not updated
 			{
 				return BlockIt->second->GetRange(Location.Offset, Location.Size);
 			}
@@ -655,7 +632,12 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 			RwLock::ExclusiveLockScope _i(m_LocationMapLock);
 			auto					   BlockFile = m_ChunkBlocks[BlockIndex];
 			ZEN_INFO("marking cas store file for delete {}, block {}", m_ContainerBaseName, std::to_string(BlockIndex));
-			BlockFile->MarkAsDeleteOnClose();
+			std::error_code Ec;
+			BlockFile->MarkAsDeleteOnClose(Ec);
+			if (Ec)
+			{
+				ZEN_WARN("Failed to flag file '{}' for deletion: '{}'", BlockFile->GetPath(), Ec.message());
+			}
 			BlockFile.reset();
 			continue;
 		}
@@ -766,7 +748,12 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 				}
 			}
 			ZEN_INFO("marking cas store file for delete {}, block index {}", m_ContainerBaseName, BlockIndex);
-			BlockFile->MarkAsDeleteOnClose();
+			std::error_code Ec;
+			BlockFile->MarkAsDeleteOnClose(Ec);
+			if (Ec)
+			{
+				ZEN_WARN("Failed to flag file '{}' for deletion: '{}'", BlockFile->GetPath(), Ec.message());
+			}
 			BlockFile.reset();
 		}
 	}
diff --git a/zenstore/include/zenstore/basicfile.h b/zenstore/include/zenstore/basicfile.h
index 9ee4ee512..0837412de 100644
--- a/zenstore/include/zenstore/basicfile.h
+++ b/zenstore/include/zenstore/basicfile.h
@@ -46,6 +46,7 @@ public:
 	void	 SetFileSize(uint64_t FileSize);
 	IoBuffer ReadAll();
 	void	 WriteAll(IoBuffer Data, std::error_code& Ec);
+	void	 MarkAsDeleteOnClose(std::error_code& Ec);
 
 	inline void* Handle() { return m_FileHandle; }
 	inline void* Detach()