De/oplog gc lifetime (#178)

* Make sure we don't use invalidated iterators in projectstore.cpp
* project store keeps track of project file and will garbage collect data for a project if the project file no longer exist
* Implement GC of projects in project store - still need to fix lifetime issues for Project instances
* Add INFO log if project file path is empty in projectstore
* changelog

1 files changed, 131 insertions, 43 deletions

diff --git a/zenserver/projectstore.cpp b/zenserver/projectstore.cpp
index 19978c3e9..2789bff42 100644
--- a/zenserver/projectstore.cpp
+++ b/zenserver/projectstore.cpp
@@ -52,8 +52,10 @@ namespace rocksdb = ROCKSDB_NAMESPACE;
 Oid
 OpKeyStringAsOId(std::string_view OpKey)
 {
+	using namespace std::literals;
+
 	CbObjectWriter Writer;
-	Writer << "key" << OpKey;
+	Writer << "key"sv << OpKey;
 
 	XXH3_128Stream KeyHasher;
 	Writer.Save()["key"].WriteToStream([&](const void* Data, size_t Size) { KeyHasher.Append(Data, Size); });
@@ -107,6 +109,8 @@ struct ProjectStore::OplogStorage : public RefCounted
 
 	void Open(bool IsCreate)
 	{
+		using namespace std::literals;
+
 		ZEN_INFO("initializing oplog storage at '{}'", m_OplogStoragePath);
 
 		if (IsCreate)
@@ -115,10 +119,10 @@ struct ProjectStore::OplogStorage : public RefCounted
 			CreateDirectories(m_OplogStoragePath);
 		}
 
-		m_Oplog.Open(m_OplogStoragePath / "ops.zlog", IsCreate ? CasLogFile::Mode::kTruncate : CasLogFile::Mode::kWrite);
+		m_Oplog.Open(m_OplogStoragePath / "ops.zlog"sv, IsCreate ? CasLogFile::Mode::kTruncate : CasLogFile::Mode::kWrite);
 		m_Oplog.Initialize();
 
-		m_OpBlobs.Open(m_OplogStoragePath / "ops.zops", IsCreate ? BasicFile::Mode::kTruncate : BasicFile::Mode::kWrite);
+		m_OpBlobs.Open(m_OplogStoragePath / "ops.zops"sv, IsCreate ? BasicFile::Mode::kTruncate : BasicFile::Mode::kWrite);
 
 		ZEN_ASSERT(IsPow2(m_OpsAlign));
 		ZEN_ASSERT(!(m_NextOpsOffset & (m_OpsAlign - 1)));
@@ -312,11 +316,13 @@ ProjectStore::Oplog::Oplog(std::string_view Id, Project* Project, CidStore& Stor
 , m_BasePath(BasePath)
 , m_OplogId(Id)
 {
+	using namespace std::literals;
+
 	m_Storage			   = new OplogStorage(this, m_BasePath);
 	const bool StoreExists = m_Storage->Exists();
 	m_Storage->Open(/* IsCreate */ !StoreExists);
 
-	m_TempPath = m_BasePath / "temp";
+	m_TempPath = m_BasePath / "temp"sv;
 
 	zen::CleanDirectory(m_TempPath);
 }
@@ -381,9 +387,10 @@ ProjectStore::Oplog::FindChunk(Oid ChunkId)
 
 	if (auto ChunkIt = m_ChunkMap.find(ChunkId); ChunkIt != m_ChunkMap.end())
 	{
+		IoHash ChunkHash = ChunkIt->second;
 		_.ReleaseNow();
 
-		IoBuffer Chunk = m_CidStore.FindChunkByCid(ChunkIt->second);
+		IoBuffer Chunk = m_CidStore.FindChunkByCid(ChunkHash);
 		Chunk.SetContentType(ZenContentType::kCompressedBinary);
 
 		return Chunk;
@@ -391,10 +398,10 @@ ProjectStore::Oplog::FindChunk(Oid ChunkId)
 
 	if (auto FileIt = m_FileMap.find(ChunkId); FileIt != m_FileMap.end())
 	{
-		_.ReleaseNow();
-
 		std::filesystem::path FilePath = m_OuterProject->RootDir / FileIt->second.ServerPath;
 
+		_.ReleaseNow();
+
 		IoBuffer FileChunk = IoBufferBuilder::MakeFromFile(FilePath);
 		FileChunk.SetContentType(ZenContentType::kBinary);
 
@@ -403,9 +410,10 @@ ProjectStore::Oplog::FindChunk(Oid ChunkId)
 
 	if (auto MetaIt = m_MetaMap.find(ChunkId); MetaIt != m_MetaMap.end())
 	{
+		IoHash ChunkHash = MetaIt->second;
 		_.ReleaseNow();
 
-		IoBuffer Chunk = m_CidStore.FindChunkByCid(MetaIt->second);
+		IoBuffer Chunk = m_CidStore.FindChunkByCid(ChunkHash);
 		Chunk.SetContentType(ZenContentType::kCompressedBinary);
 
 		return Chunk;
@@ -665,7 +673,9 @@ ProjectStore::Project::Exists(std::filesystem::path BasePath)
 void
 ProjectStore::Project::Read()
 {
-	std::filesystem::path ProjectStateFilePath = m_OplogStoragePath / "Project.zcb";
+	using namespace std::literals;
+
+	std::filesystem::path ProjectStateFilePath = m_OplogStoragePath / "Project.zcb"sv;
 
 	ZEN_INFO("reading config for project '{}' from {}", Identifier, ProjectStateFilePath);
 
@@ -679,10 +689,11 @@ ProjectStore::Project::Read()
 	{
 		CbObject Cfg = LoadCompactBinaryObject(Obj);
 
-		Identifier	   = Cfg["id"].AsString();
-		RootDir		   = Cfg["root"].AsString();
-		ProjectRootDir = Cfg["project"].AsString();
-		EngineRootDir  = Cfg["engine"].AsString();
+		Identifier		= Cfg["id"].AsString();
+		RootDir			= Cfg["root"].AsString();
+		ProjectRootDir	= Cfg["project"].AsString();
+		EngineRootDir	= Cfg["engine"].AsString();
+		ProjectFilePath = Cfg["projectfile"].AsString();
 	}
 	else
 	{
@@ -693,19 +704,22 @@ ProjectStore::Project::Read()
 void
 ProjectStore::Project::Write()
 {
+	using namespace std::literals;
+
 	BinaryWriter Mem;
 
 	CbObjectWriter Cfg;
-	Cfg << "id" << Identifier;
-	Cfg << "root" << PathToUtf8(RootDir);
-	Cfg << "project" << ProjectRootDir;
-	Cfg << "engine" << EngineRootDir;
+	Cfg << "id"sv << Identifier;
+	Cfg << "root"sv << PathToUtf8(RootDir);
+	Cfg << "project"sv << ProjectRootDir;
+	Cfg << "engine"sv << EngineRootDir;
+	Cfg << "projectfile"sv << ProjectFilePath;
 
 	Cfg.Save(Mem);
 
 	CreateDirectories(m_OplogStoragePath);
 
-	std::filesystem::path ProjectStateFilePath = m_OplogStoragePath / "Project.zcb";
+	std::filesystem::path ProjectStateFilePath = m_OplogStoragePath / "Project.zcb"sv;
 
 	ZEN_INFO("persisting config for project '{}' to {}", Identifier, ProjectStateFilePath);
 
@@ -867,13 +881,36 @@ ProjectStore::Project::Scrub(ScrubContext& Ctx)
 void
 ProjectStore::Project::GatherReferences(GcContext& GcCtx)
 {
+	if (IsExpired())
+	{
+		ZEN_DEBUG("Skipping reference gathering for '{}', project file '{}' no longer exist", Identifier, ProjectFilePath);
+		return;
+	}
 	IterateOplogs([&](Oplog& Ops) { Ops.GatherReferences(GcCtx); });
 }
 
+void
+ProjectStore::Project::Delete()
+{
+	RwLock::ExclusiveLockScope _(m_ProjectLock);
+	DeleteDirectories(m_OplogStoragePath);
+}
+
+bool
+ProjectStore::Project::IsExpired() const
+{
+	if (ProjectFilePath.empty())
+	{
+		return false;
+	}
+	return !std::filesystem::exists(ProjectFilePath);
+}
+
 //////////////////////////////////////////////////////////////////////////
 
 ProjectStore::ProjectStore(CidStore& Store, std::filesystem::path BasePath, GcManager& Gc)
-: GcContributor(Gc)
+: GcStorage(Gc)
+, GcContributor(Gc)
 , m_Log(zen::logging::Get("project"))
 , m_CidStore(Store)
 , m_ProjectBasePath(BasePath)
@@ -966,6 +1003,39 @@ ProjectStore::GatherReferences(GcContext& GcCtx)
 	}
 }
 
+void
+ProjectStore::CollectGarbage(GcContext& GcCtx)
+{
+	std::vector<std::string> ExpiredProjects;
+
+	{
+		RwLock::SharedLockScope _(m_ProjectsLock);
+		for (auto& Kv : m_Projects)
+		{
+			if (Kv.second.IsExpired())
+			{
+				ExpiredProjects.push_back(Kv.first);
+			}
+		}
+	}
+
+	if (!GcCtx.IsDeletionMode())
+	{
+		return;
+	}
+
+	for (const std::string& ProjectId : ExpiredProjects)
+	{
+		DeleteProject(ProjectId, true);
+	}
+}
+
+GcStorageSize
+ProjectStore::StorageSize() const
+{
+	return {0, 0};
+}
+
 ProjectStore::Project*
 ProjectStore::OpenProject(std::string_view ProjectId)
 {
@@ -1010,7 +1080,8 @@ ProjectStore::NewProject(std::filesystem::path BasePath,
 						 std::string_view	   ProjectId,
 						 std::string_view	   RootDir,
 						 std::string_view	   EngineRootDir,
-						 std::string_view	   ProjectRootDir)
+						 std::string_view	   ProjectRootDir,
+						 std::string_view	   ProjectFilePath)
 {
 	RwLock::ExclusiveLockScope _(m_ProjectsLock);
 
@@ -1019,21 +1090,34 @@ ProjectStore::NewProject(std::filesystem::path BasePath,
 	Prj.RootDir				   = RootDir;
 	Prj.EngineRootDir		   = EngineRootDir;
 	Prj.ProjectRootDir		   = ProjectRootDir;
+	Prj.ProjectFilePath		   = ProjectFilePath;
 	Prj.Write();
 
 	return &Prj;
 }
 
 void
-ProjectStore::DeleteProject(std::string_view ProjectId)
+ProjectStore::DeleteProject(std::string_view ProjectId, bool OnlyDeleteIfExpired)
 {
-	std::filesystem::path ProjectBasePath = BasePathForProject(ProjectId);
+	ZEN_INFO("deleting project {}", ProjectId);
+
+	RwLock::SharedLockScope _(m_ProjectsLock);
 
-	ZEN_INFO("deleting project {} @ {}", ProjectId, ProjectBasePath);
+	auto ProjIt = m_Projects.find(std::string{ProjectId});
 
-	m_Projects.erase(std::string{ProjectId});
+	if (ProjIt == m_Projects.end())
+	{
+		return;
+	}
+	if (OnlyDeleteIfExpired && !ProjIt->second.IsExpired())
+	{
+		return;
+	}
+	// _.ReleaseNow(); TODO: We can't release here since any changes to m_Projects map will invalidate our ProjIt pointer
+	ProjIt->second.Delete();
 
-	DeleteDirectories(ProjectBasePath);
+	m_Projects.erase(
+		ProjIt);  // TODO: We need to keep the project pointer alive, somebody else may use it after fetching it from m_Projects
 }
 
 bool
@@ -1084,6 +1168,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 				Response << "RootDir"sv << Prj.RootDir.string();
 				Response << "ProjectRootDir"sv << Prj.ProjectRootDir;
 				Response << "EngineRootDir"sv << Prj.EngineRootDir;
+				Response << "ProjectFilePath"sv << Prj.ProjectFilePath;
 				Response.EndObject();
 			});
 			Response.EndArray();
@@ -1239,18 +1324,18 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 
 			HttpServerRequest::QueryParams Params = HttpReq.GetQueryParams();
 
-			const bool FilterClient = Params.GetValue("filter") == "client";
+			const bool FilterClient = Params.GetValue("filter"sv) == "client"sv;
 
 			CbObjectWriter Response;
 			Response.BeginArray("files");
 
 			Log.IterateFileMap([&](const Oid& Id, const std::string_view& ServerPath, const std::string_view& ClientPath) {
 				Response.BeginObject();
-				Response << "id" << Id;
-				Response << "clientpath" << ClientPath;
+				Response << "id"sv << Id;
+				Response << "clientpath"sv << ClientPath;
 				if (!FilterClient)
 				{
-					Response << "serverpath" << ServerPath;
+					Response << "serverpath"sv << ServerPath;
 				}
 				Response.EndObject();
 			});
@@ -1297,7 +1382,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 			}
 
 			CbObjectWriter Response;
-			Response << "size" << ChunkSize;
+			Response << "size"sv << ChunkSize;
 			HttpReq.WriteResponse(HttpResponseCode::OK, Response.Save());
 		},
 		HttpVerb::kGet);
@@ -1580,7 +1665,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 			if (!legacy::TryLoadCbPackage(Package, Payload, &UniqueBuffer::Alloc, &Resolver))
 			{
 				std::filesystem::path BadPackagePath =
-					Oplog.TempPath() / "bad_packages" / fmt::format("session{}_request{}", HttpReq.SessionId(), HttpReq.RequestId());
+					Oplog.TempPath() / "bad_packages"sv / fmt::format("session{}_request{}"sv, HttpReq.SessionId(), HttpReq.RequestId());
 
 				ZEN_WARN("Received malformed package! Saving payload to '{}'", BadPackagePath);
 
@@ -1812,7 +1897,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 
 					if (Op.has_value())
 					{
-						Response << "entry" << Op.value();
+						Response << "entry"sv << Op.value();
 					}
 					else
 					{
@@ -1842,22 +1927,25 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 			{
 				case HttpVerb::kPost:
 					{
-						IoBuffer		 Payload	 = Req.ServerRequest().ReadPayload();
-						CbObject		 Params		 = LoadCompactBinaryObject(Payload);
-						std::string_view Id			 = Params["id"sv].AsString();
-						std::string_view Root		 = Params["root"sv].AsString();
-						std::string_view EngineRoot	 = Params["engine"sv].AsString();
-						std::string_view ProjectRoot = Params["project"sv].AsString();
+						IoBuffer		 Payload		 = Req.ServerRequest().ReadPayload();
+						CbObject		 Params			 = LoadCompactBinaryObject(Payload);
+						std::string_view Id				 = Params["id"sv].AsString();
+						std::string_view Root			 = Params["root"sv].AsString();
+						std::string_view EngineRoot		 = Params["engine"sv].AsString();
+						std::string_view ProjectRoot	 = Params["project"sv].AsString();
+						std::string_view ProjectFilePath = Params["projectfile"sv].AsString();
 
 						const std::filesystem::path BasePath = m_ProjectStore->BasePath() / ProjectId;
-						m_ProjectStore->NewProject(BasePath, ProjectId, Root, EngineRoot, ProjectRoot);
+						m_ProjectStore->NewProject(BasePath, ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath);
 
-						ZEN_INFO("established project - {} (id: '{}', roots: '{}', '{}', '{}')",
+						ZEN_INFO("established project - {} (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
 								 ProjectId,
 								 Id,
 								 Root,
 								 EngineRoot,
-								 ProjectRoot);
+								 ProjectRoot,
+								 ProjectFilePath,
+								 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
 
 						Req.ServerRequest().WriteResponse(HttpResponseCode::Created);
 					}
@@ -1877,7 +1965,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 						const ProjectStore::Project& Prj = *ProjectIt;
 
 						CbObjectWriter Response;
-						Response << "id" << Prj.Identifier << "root" << PathToUtf8(Prj.RootDir);
+						Response << "id"sv << Prj.Identifier << "root"sv << PathToUtf8(Prj.RootDir);
 
 						Response.BeginArray("oplogs"sv);
 						Prj.IterateOplogs([&](const ProjectStore::Oplog& I) {
@@ -1902,7 +1990,7 @@ HttpProjectService::HttpProjectService(CidStore& Store, ProjectStore* Projects)
 																	 fmt::format("project {} not found", ProjectId));
 						}
 
-						m_ProjectStore->DeleteProject(ProjectId);
+						m_ProjectStore->DeleteProject(ProjectId, false);
 
 						return Req.ServerRequest().WriteResponse(HttpResponseCode::NoContent);
 					}