From 2aecc7d7016009c331fdf5ddf889eaedeca41933 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sat, 19 Nov 2016 23:24:01 +0100 Subject: Fixed race conditions in cClientHandle's State. --- src/ClientHandle.h | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) (limited to 'src/ClientHandle.h') diff --git a/src/ClientHandle.h b/src/ClientHandle.h index a361a0fb5..e6982d546 100644 --- a/src/ClientHandle.h +++ b/src/ClientHandle.h @@ -465,22 +465,28 @@ private: enum eState { - csConnected, ///< The client has just connected, waiting for their handshake / login - csAuthenticating, ///< The client has logged in, waiting for external authentication - csAuthenticated, ///< The client has been authenticated, will start streaming chunks in the next tick - csDownloadingWorld, ///< The client is waiting for chunks, we're waiting for the loader to provide and send them - csConfirmingPos, ///< The client has been sent the position packet, waiting for them to repeat the position back - csPlaying, ///< Normal gameplay - csDestroying, ///< The client is being destroyed, don't queue any more packets / don't add to chunks - csDestroyed, ///< The client has been destroyed, the destructor is to be called from the owner thread + csConnected, ///< The client has just connected, waiting for their handshake / login + csAuthenticating, ///< The client has logged in, waiting for external authentication + csAuthenticated, ///< The client has been authenticated, will start streaming chunks in the next tick + csDownloadingWorld, ///< The client is waiting for chunks, we're waiting for the loader to provide and send them + csConfirmingPos, ///< The client has been sent the position packet, waiting for them to repeat the position back + csPlaying, ///< Normal gameplay + csQueuedForDestruction, ///< The client will be destroyed in the next tick (flag set when socket closed) + csDestroying, ///< The client is being destroyed, don't queue any more packets / don't add to chunks + csDestroyed, ///< The client has been destroyed, the destructor is to be called from the owner thread // TODO: Add Kicking here as well } ; - std::atomic m_State; + /* Mutex protecting m_State from concurrent writes. */ + cCriticalSection m_CSState; - /** m_State needs to be locked in the Destroy() function so that the destruction code doesn't run twice on two different threads */ - cCriticalSection m_CSDestroyingState; + /** The current (networking) state of the client. + Protected from concurrent writes by m_CSState; but may be read by other threads concurrently. + If a function depends on m_State or wants to change m_State, it needs to lock m_CSState. + However, if it only uses m_State for a quick bail out, or it doesn't break if the client disconnects in the middle of it, + it may just read m_State without locking m_CSState. */ + std::atomic m_State; /** If set to true during csDownloadingWorld, the tick thread calls CheckIfWorldDownloaded() */ bool m_ShouldCheckDownloaded; @@ -556,6 +562,10 @@ private: /** Called right after the instance is created to store its SharedPtr inside. */ void SetSelf(cClientHandlePtr a_Self); + /** Processes the data in the network input and output buffers. + Called by both Tick() and ServerTick(). */ + void ProcessProtocolInOut(void); + // cTCPLink::cCallbacks overrides: virtual void OnLinkCreated(cTCPLinkPtr a_Link) override; virtual void OnReceivedData(const char * a_Data, size_t a_Length) override; -- cgit v1.2.3