[FIXED] std::list asserted in cChunkMap::RemoveClientFromChunks()

[FakeTruth]

I'm kinda confused why it asserted


As you can see it didn't like the ++itr part in cChunkMap::RemoveClientFromChunks(), which is strange, because a_Chunks is empty



To me this means:
1. cClientHandle::m_LoadedChunks has been altered by another thread, but the criticalsection for m_LoadedChunks was locked and owned by the current thread.

2. The line GetChunkNoGen(...)->RemoveClient(a_Client) in cChunkMap::RemoveClientFromChunks() changed cClientHandle::m_LoadedChunks. But upon inspection this does not happen

So, wtf?

By the way, I seem to have a shitload of SendThreads (hundreds) ?!?

368 threads yo~

Using revision 326

[xoft]

What was the assert message? That's the most important diagnostic.

SendThreads should be eliminated now (rev 328+) since sending uses cSocketThreads now.

I think I remember fixing one place where the m_LoadedChunks list was accessed without proper locking, though I don't really recall where or when. It would explain a lot

[FakeTruth]

Actually I now think the destructor cleared it

Code:

void cClientHandle::Destroy()
{
    m_bDestroyed = true;
    
    if ((m_Player != NULL) && (m_Player->GetWorld() != NULL))
    {
        RemoveFromAllChunks();
    }
}

m_bDestroyed is set first, but it's done from the SendThread. The tick thread comes along, sees the flag m_bDestroyed is true, and deletes the client.

Thoughts?

[xoft]

This might actually be the case. And it might also be the cause for the crashes we get reported "from russia".

Setting m_bDestroyed to true as the last thing Destroy() does should fix that.

---

I'll do it.

---

Yeah, destructor didn't lock m_CSChunkLists, so it could have done that. Fixing both.