From 9da404ea2db52dd4a5d9d0a5ca1736fd7e2e10bf Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sun, 4 Oct 2015 14:06:37 +0200 Subject: Fixed a race condition between chunk loader and generator. When using ChunkWorx to generate multiple chunks, the server would sometimes fail an assert because it would generate a chunk even when it was successfully loaded. This was caused by chunks queued in cWorld's m_SetChunkDataQueue and thus being marked as "InQueue" although they were already loaded. Solved by adding a new parameter to chunk coord callbacks specifying whether the operation succeeded or failed, and using that instead of the chunk presence flag to decide whether to generate or not. --- src/Bindings/ManualBindings_World.cpp | 4 ++-- src/ChunkDef.h | 3 ++- src/ChunkMap.cpp | 36 +++++++++++++++++------------------ src/ChunkSender.cpp | 2 +- src/Generating/ChunkGenerator.cpp | 12 ++++++------ src/LightingThread.cpp | 6 +++--- src/LightingThread.h | 3 ++- src/SpawnPrepare.cpp | 2 +- src/WorldStorage/WorldStorage.cpp | 32 ++++--------------------------- src/WorldStorage/WorldStorage.h | 10 ++++++---- 10 files changed, 45 insertions(+), 65 deletions(-) diff --git a/src/Bindings/ManualBindings_World.cpp b/src/Bindings/ManualBindings_World.cpp index ba80d7130..c664329f9 100644 --- a/src/Bindings/ManualBindings_World.cpp +++ b/src/Bindings/ManualBindings_World.cpp @@ -348,11 +348,11 @@ static int tolua_cWorld_PrepareChunk(lua_State * tolua_S) } // cChunkCoordCallback override: - virtual void Call(int a_CBChunkX, int a_CBChunkZ) override + virtual void Call(int a_CBChunkX, int a_CBChunkZ, bool a_IsSuccess) override { if (m_Callback.IsValid()) { - m_LuaState.Call(m_Callback, a_CBChunkX, a_CBChunkZ); + m_LuaState.Call(m_Callback, a_CBChunkX, a_CBChunkZ, a_IsSuccess); } // This is the last reference of this object, we must delete it so that it doesn't leak: diff --git a/src/ChunkDef.h b/src/ChunkDef.h index f6c0381db..fcda9b5a6 100644 --- a/src/ChunkDef.h +++ b/src/ChunkDef.h @@ -464,7 +464,8 @@ public: virtual ~cChunkCoordCallback() {} - virtual void Call(int a_ChunkX, int a_ChunkZ) = 0; + /** Called with the chunk's coords, and an optional operation status flag for operations that support it. */ + virtual void Call(int a_ChunkX, int a_ChunkZ, bool a_IsSuccess) = 0; } ; diff --git a/src/ChunkMap.cpp b/src/ChunkMap.cpp index a4771ce52..59a743746 100644 --- a/src/ChunkMap.cpp +++ b/src/ChunkMap.cpp @@ -2396,10 +2396,10 @@ void cChunkMap::PrepareChunk(int a_ChunkX, int a_ChunkZ, std::unique_ptrCall(a_ChunkX, a_ChunkZ); + a_Callback->Call(a_ChunkX, a_ChunkZ, true); } } @@ -2432,9 +2432,19 @@ bool cChunkMap::GenerateChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallback * } // cChunkCoordCallback override: - virtual void Call(int a_CBChunkX, int a_CBChunkZ) override + virtual void Call(int a_CBChunkX, int a_CBChunkZ, bool a_CBIsSuccess) override { - // The chunk has been loaded or an error occurred, check if it's valid now: + // If success is reported, the chunk is already valid, no need to do anything else: + if (a_CBIsSuccess) + { + if (m_Callback != nullptr) + { + m_Callback->Call(a_CBChunkX, a_CBChunkZ, true); + } + return; + } + + // The chunk failed to load, generate it: cCSLock Lock(m_ChunkMap.m_CSLayers); cChunkPtr CBChunk = m_ChunkMap.GetChunkNoLoad(a_CBChunkX, a_CBChunkZ); @@ -2443,23 +2453,13 @@ bool cChunkMap::GenerateChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallback * // An error occurred, but we promised to call the callback, so call it even when there's no real chunk data: if (m_Callback != nullptr) { - m_Callback->Call(a_CBChunkX, a_CBChunkZ); + m_Callback->Call(a_CBChunkX, a_CBChunkZ, false); } return; } - // If the chunk is not valid, queue it in the generator: - if (!CBChunk->IsValid()) - { - m_World.GetGenerator().QueueGenerateChunk(a_CBChunkX, a_CBChunkZ, false, m_Callback); - return; - } - - // The chunk was loaded, call the callback: - if (m_Callback != nullptr) - { - m_Callback->Call(a_CBChunkX, a_CBChunkZ); - } + CBChunk->SetPresence(cChunk::cpQueued); + m_World.GetGenerator().QueueGenerateChunk(a_CBChunkX, a_CBChunkZ, false, m_Callback); } protected: @@ -2474,7 +2474,7 @@ bool cChunkMap::GenerateChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallback * // The chunk is valid, just call the callback: if (a_Callback != nullptr) { - a_Callback->Call(a_ChunkX, a_ChunkZ); + a_Callback->Call(a_ChunkX, a_ChunkZ, true); } return true; } diff --git a/src/ChunkSender.cpp b/src/ChunkSender.cpp index 674291c73..1d55cf743 100644 --- a/src/ChunkSender.cpp +++ b/src/ChunkSender.cpp @@ -27,7 +27,7 @@ class cNotifyChunkSender : public cChunkCoordCallback { - virtual void Call(int a_ChunkX, int a_ChunkZ) override + virtual void Call(int a_ChunkX, int a_ChunkZ, bool a_IsSuccess) override { cChunkSender & ChunkSender = m_ChunkSender; m_World.DoWithChunk( diff --git a/src/Generating/ChunkGenerator.cpp b/src/Generating/ChunkGenerator.cpp index 741cdd7ae..bfa3344b9 100644 --- a/src/Generating/ChunkGenerator.cpp +++ b/src/Generating/ChunkGenerator.cpp @@ -248,13 +248,13 @@ void cChunkGenerator::Execute(void) LastReportTick = clock(); } - // Skip the chunk if it's already generated and regeneration is not forced: + // Skip the chunk if it's already generated and regeneration is not forced. Report as success: if (!item.m_ForceGenerate && m_ChunkSink->IsChunkValid(item.m_ChunkX, item.m_ChunkZ)) { LOGD("Chunk [%d, %d] already generated, skipping generation", item.m_ChunkX, item.m_ChunkZ); if (item.m_Callback != nullptr) { - item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ); + item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ, true); } continue; } @@ -265,7 +265,7 @@ void cChunkGenerator::Execute(void) LOGWARNING("Chunk generator overloaded, skipping chunk [%d, %d]", item.m_ChunkX, item.m_ChunkZ); if (item.m_Callback != nullptr) { - item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ); + item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ, false); } continue; } @@ -275,7 +275,7 @@ void cChunkGenerator::Execute(void) DoGenerate(item.m_ChunkX, item.m_ChunkZ); if (item.m_Callback != nullptr) { - item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ); + item.m_Callback->Call(item.m_ChunkX, item.m_ChunkZ, true); } NumChunksGenerated++; } // while (!bStop) @@ -296,8 +296,8 @@ void cChunkGenerator::DoGenerate(int a_ChunkX, int a_ChunkZ) m_PluginInterface->CallHookChunkGenerated(ChunkDesc); #ifdef _DEBUG - // Verify that the generator has produced valid data: - ChunkDesc.VerifyHeightmap(); + // Verify that the generator has produced valid data: + ChunkDesc.VerifyHeightmap(); #endif m_ChunkSink->OnChunkGenerated(ChunkDesc); diff --git a/src/LightingThread.cpp b/src/LightingThread.cpp index b5102c11b..25956ae86 100644 --- a/src/LightingThread.cpp +++ b/src/LightingThread.cpp @@ -238,12 +238,12 @@ void cLightingThread::Execute(void) void cLightingThread::LightChunk(cLightingChunkStay & a_Item) { - // If the chunk is already lit, skip it: + // If the chunk is already lit, skip it (report as success): if (m_World->IsChunkLighted(a_Item.m_ChunkX, a_Item.m_ChunkZ)) { if (a_Item.m_CallbackAfter != nullptr) { - a_Item.m_CallbackAfter->Call(a_Item.m_ChunkX, a_Item.m_ChunkZ); + a_Item.m_CallbackAfter->Call(a_Item.m_ChunkX, a_Item.m_ChunkZ, true); } return; } @@ -324,7 +324,7 @@ void cLightingThread::LightChunk(cLightingChunkStay & a_Item) if (a_Item.m_CallbackAfter != nullptr) { - a_Item.m_CallbackAfter->Call(a_Item.m_ChunkX, a_Item.m_ChunkZ); + a_Item.m_CallbackAfter->Call(a_Item.m_ChunkX, a_Item.m_ChunkZ, true); } } diff --git a/src/LightingThread.h b/src/LightingThread.h index b18ac7799..138c40002 100644 --- a/src/LightingThread.h +++ b/src/LightingThread.h @@ -60,7 +60,8 @@ public: void Stop(void); - /** Queues the entire chunk for lighting */ + /** Queues the entire chunk for lighting. + The callback, if specified, is called after the lighting has been processed. */ void QueueChunk(int a_ChunkX, int a_ChunkZ, std::unique_ptr a_CallbackAfter); /** Blocks until the queue is empty or the thread is terminated */ diff --git a/src/SpawnPrepare.cpp b/src/SpawnPrepare.cpp index 74dcb3ecd..e332d3b1a 100644 --- a/src/SpawnPrepare.cpp +++ b/src/SpawnPrepare.cpp @@ -20,7 +20,7 @@ protected: cSpawnPrepare & m_SpawnPrepare; - virtual void Call(int a_ChunkX, int a_ChunkZ) override + virtual void Call(int a_ChunkX, int a_ChunkZ, bool a_IsSuccess) override { m_SpawnPrepare.PreparedChunkCallback(a_ChunkX, a_ChunkZ); } diff --git a/src/WorldStorage/WorldStorage.cpp b/src/WorldStorage/WorldStorage.cpp index 19fc2be56..3a2385848 100644 --- a/src/WorldStorage/WorldStorage.cpp +++ b/src/WorldStorage/WorldStorage.cpp @@ -164,32 +164,6 @@ void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallba -void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkZ) -{ - m_LoadQueue.RemoveIf([=](cChunkCoordsWithCallback & a_Item) - { - return (a_Item.m_ChunkX == a_ChunkX) && (a_Item.m_ChunkZ == a_ChunkZ); - } - ); -} - - - - - -void cWorldStorage::UnqueueSave(const cChunkCoords & a_Chunk) -{ - m_SaveQueue.RemoveIf([=](cChunkCoordsWithCallback & a_Item) - { - return (a_Item.m_ChunkX == a_Chunk.m_ChunkX) && (a_Item.m_ChunkZ == a_Chunk.m_ChunkZ); - } - ); -} - - - - - void cWorldStorage::InitSchemas(int a_StorageCompressionFactor) { // The first schema added is considered the default @@ -266,7 +240,7 @@ bool cWorldStorage::LoadOneChunk(void) // Call the callback, if specified: if (ToLoad.m_Callback != nullptr) { - ToLoad.m_Callback->Call(ToLoad.m_ChunkX, ToLoad.m_ChunkZ); + ToLoad.m_Callback->Call(ToLoad.m_ChunkX, ToLoad.m_ChunkZ, res); } return res; } @@ -286,19 +260,21 @@ bool cWorldStorage::SaveOneChunk(void) } // Save the chunk, if it's valid: + bool Status = false; if (m_World->IsChunkValid(ToSave.m_ChunkX, ToSave.m_ChunkZ)) { m_World->MarkChunkSaving(ToSave.m_ChunkX, ToSave.m_ChunkZ); if (m_SaveSchema->SaveChunk(cChunkCoords(ToSave.m_ChunkX, ToSave.m_ChunkZ))) { m_World->MarkChunkSaved(ToSave.m_ChunkX, ToSave.m_ChunkZ); + Status = true; } } // Call the callback, if specified: if (ToSave.m_Callback != nullptr) { - ToSave.m_Callback->Call(ToSave.m_ChunkX, ToSave.m_ChunkZ); + ToSave.m_Callback->Call(ToSave.m_ChunkX, ToSave.m_ChunkZ, Status); } return true; } diff --git a/src/WorldStorage/WorldStorage.h b/src/WorldStorage/WorldStorage.h index 70cca9031..ab8a7f44b 100644 --- a/src/WorldStorage/WorldStorage.h +++ b/src/WorldStorage/WorldStorage.h @@ -63,13 +63,15 @@ public: cWorldStorage(void); ~cWorldStorage(); - + + /** Queues a chunk to be loaded, asynchronously. + The callback, if specified, will be called with the result of the load operation. */ void QueueLoadChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallback * a_Callback = nullptr); + + /** Queues a chunk to be saved, asynchronously. + The callback, if specified, will be called with the result of the save operation. */ void QueueSaveChunk(int a_ChunkX, int a_ChunkZ, cChunkCoordCallback * a_Callback = nullptr); - void UnqueueLoad(int a_ChunkX, int a_ChunkZ); - void UnqueueSave(const cChunkCoords & a_Chunk); - bool Start(cWorld * a_World, const AString & a_StorageSchemaName, int a_StorageCompressionFactor); // Hide the cIsThread's Start() method, we need to provide args void Stop(void); // Hide the cIsThread's Stop() method, we need to signal the event void WaitForFinish(void); -- cgit v1.2.3