11-20-2016, 08:39 AM
I believe I've fixed one huge pain in our collective a**es - the cClientHandle race conditions. I did test this extensively, but still would appreciate more people going through the changes: https://github.com/cuberite/cuberite/pull/3439
Mostly, the change is about adding a cCriticalSection wrapped around cClientHandle's m_State, so that each write is protected. If such a write depends on a previous read, the read is included in the CS lock. Still, there are places where the value of m_State is not as important (say, when deciding what chunks to send next, it's not exactly important if the client has just disconnected - the chunk will be chosen and then the clienthandle destroyed anyway), so there are places where it makes sense to read the m_State value without holding the CS. For that reason, m_State is kept as a std::atomic.
Mostly, the change is about adding a cCriticalSection wrapped around cClientHandle's m_State, so that each write is protected. If such a write depends on a previous read, the read is included in the CS lock. Still, there are places where the value of m_State is not as important (say, when deciding what chunks to send next, it's not exactly important if the client has just disconnected - the chunk will be chosen and then the clienthandle destroyed anyway), so there are places where it makes sense to read the m_State value without holding the CS. For that reason, m_State is kept as a std::atomic.