1
0
Fork 0

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.
This commit is contained in:
Mattes D 2015-10-04 14:06:37 +02:00
parent d8127aa228
commit 9da404ea2d
10 changed files with 45 additions and 65 deletions

View File

@ -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:

View File

@ -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;
} ;

View File

@ -2396,10 +2396,10 @@ void cChunkMap::PrepareChunk(int a_ChunkX, int a_ChunkZ, std::unique_ptr<cChunkC
return;
}
// The chunk is present and lit, just call the callback:
// The chunk is present and lit, just call the callback, report as success:
if (a_Callback != nullptr)
{
a_Callback->Call(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;
}

View File

@ -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(

View File

@ -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);

View File

@ -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);
}
}

View File

@ -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<cChunkCoordCallback> a_CallbackAfter);
/** Blocks until the queue is empty or the thread is terminated */

View File

@ -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);
}

View File

@ -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;
}

View File

@ -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);