From eb4432bb6260eaadb41495149739244308e9e125 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Fri, 28 Jul 2017 17:54:40 +0100 Subject: Tentative fix for player-limit race condition (#3862) * Attempts to fix #2257 Derived from d233e9843148313c71fbaba96ccff660e47b07b1 * Changed player count type to int * Clarified certain actions --- src/Server.cpp | 48 +++++++++++++----------------------------------- 1 file changed, 13 insertions(+), 35 deletions(-) (limited to 'src/Server.cpp') diff --git a/src/Server.cpp b/src/Server.cpp index fd707e61a..947852775 100644 --- a/src/Server.cpp +++ b/src/Server.cpp @@ -118,7 +118,6 @@ void cServer::cTickThread::Execute(void) cServer::cServer(void) : m_PlayerCount(0), - m_PlayerCountDiff(0), m_ClientViewDistance(0), m_bIsConnected(false), m_bRestarting(false), @@ -148,24 +147,18 @@ void cServer::ClientMovedToWorld(const cClientHandle * a_Client) -void cServer::PlayerCreated(const cPlayer * a_Player) +void cServer::PlayerCreated() { - UNUSED(a_Player); - // To avoid deadlocks, the player count is not handled directly, but rather posted onto the tick thread - cCSLock Lock(m_CSPlayerCountDiff); - m_PlayerCountDiff += 1; + m_PlayerCount++; } -void cServer::PlayerDestroying(const cPlayer * a_Player) +void cServer::PlayerDestroyed() { - UNUSED(a_Player); - // To avoid deadlocks, the player count is not handled directly, but rather posted onto the tick thread - cCSLock Lock(m_CSPlayerCountDiff); - m_PlayerCountDiff -= 1; + m_PlayerCount--; } @@ -176,11 +169,9 @@ bool cServer::InitServer(cSettingsRepositoryInterface & a_Settings, bool a_Shoul { m_Description = a_Settings.GetValueSet("Server", "Description", "Cuberite - in C++!"); m_ShutdownMessage = a_Settings.GetValueSet("Server", "ShutdownMessage", "Server shutdown"); - m_MaxPlayers = a_Settings.GetValueSetI("Server", "MaxPlayers", 100); + m_MaxPlayers = static_cast(a_Settings.GetValueSetI("Server", "MaxPlayers", 100)); m_bIsHardcore = a_Settings.GetValueSetB("Server", "HardcoreEnabled", false); m_bAllowMultiLogin = a_Settings.GetValueSetB("Server", "AllowMultiLogin", false); - m_PlayerCount = 0; - m_PlayerCountDiff = 0; m_FaviconData = Base64Encode(cFile::ReadWholeFile(FILE_IO_PREFIX + AString("favicon.png"))); // Will return empty string if file nonexistant; client doesn't mind @@ -246,16 +237,6 @@ bool cServer::InitServer(cSettingsRepositoryInterface & a_Settings, bool a_Shoul -int cServer::GetNumPlayers(void) const -{ - cCSLock Lock(m_CSPlayerCount); - return m_PlayerCount; -} - - - - - bool cServer::IsPlayerInQueue(AString a_Username) { cCSLock Lock(m_CSClients); @@ -300,17 +281,6 @@ cTCPLink::cCallbacksPtr cServer::OnConnectionAccepted(const AString & a_RemoteIP bool cServer::Tick(float a_Dt) { - // Apply the queued playercount adjustments (postponed to avoid deadlocks) - int PlayerCountDiff = 0; - { - cCSLock Lock(m_CSPlayerCountDiff); - std::swap(PlayerCountDiff, m_PlayerCountDiff); - } - { - cCSLock Lock(m_CSPlayerCount); - m_PlayerCount += PlayerCountDiff; - } - // Send the tick to the plugins, as well as let the plugin manager reload, if asked to (issue #102): cPluginManager::Get()->Tick(a_Dt); @@ -657,6 +627,14 @@ void cServer::KickUser(int a_ClientID, const AString & a_Reason) void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const AString & a_UUID, const Json::Value & a_Properties) { cCSLock Lock(m_CSClients); + + // Check max players condition within lock (expect server and authenticator thread to both call here) + if (GetNumPlayers() >= GetMaxPlayers()) + { + KickUser(a_ClientID, "The server is currently full :(" "\n" "Try again later?"); + return; + } + for (auto itr = m_Clients.begin(); itr != m_Clients.end(); ++itr) { if ((*itr)->GetUniqueID() == a_ClientID) -- cgit v1.2.3