diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index 1b63153d2..3270cc37f 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -179,6 +179,9 @@ void cClientHandle::Destroy(void) if (player != nullptr) { + // Atomically decrement player count (in world or server thread) + cRoot::Get()->GetServer()->PlayerDestroyed(); + auto world = player->GetWorld(); if (world != nullptr) { @@ -322,6 +325,9 @@ void cClientHandle::Kick(const AString & a_Reason) void cClientHandle::Authenticate(const AString & a_Name, const AString & a_UUID, const Json::Value & a_Properties) { + // Atomically increment player count (in server thread) + cRoot::Get()->GetServer()->PlayerCreated(); + cWorld * World; { cCSLock lock(m_CSState); @@ -662,11 +668,14 @@ void cClientHandle::HandleNPCTrade(int a_SlotNum) void cClientHandle::HandlePing(void) { + /* TODO: unused function, handles Legacy Server List Ping + http://wiki.vg/Protocol#Legacy_Server_List_Ping suggests that servers SHOULD handle this packet */ + // Somebody tries to retrieve information about the server AString Reply; const cServer & Server = *cRoot::Get()->GetServer(); - Printf(Reply, "%s%s%i%s%i", + Printf(Reply, "%s%s" SIZE_T_FMT "%s" SIZE_T_FMT, Server.GetDescription().c_str(), cChatColor::Delimiter, Server.GetNumPlayers(), @@ -1860,17 +1869,14 @@ bool cClientHandle::HandleHandshake(const AString & a_Username) { if (a_Username.length() > 16) { - Kick("Your username is too long(>16 characters)"); + Kick("Your username is too long (>16 characters)"); return false; } - if (!cRoot::Get()->GetPluginManager()->CallHookHandshake(*this, a_Username)) + if (cRoot::Get()->GetPluginManager()->CallHookHandshake(*this, a_Username)) { - if (cRoot::Get()->GetServer()->GetNumPlayers() >= cRoot::Get()->GetServer()->GetMaxPlayers()) - { - Kick("The server is currently full :(-- Try again later"); - return false; - } + Kick("Entry denied by plugin"); + return false; } return CheckMultiLogin(a_Username); diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp index 77ce378da..afeb8fa04 100644 --- a/src/Entities/Player.cpp +++ b/src/Entities/Player.cpp @@ -146,8 +146,6 @@ cPlayer::cPlayer(cClientHandlePtr a_Client, const AString & a_PlayerName) : m_IsFlying = true; m_bVisible = false; } - - cRoot::Get()->GetServer()->PlayerCreated(this); } @@ -183,9 +181,6 @@ cPlayer::~cPlayer(void) LOGD("Deleting cPlayer \"%s\" at %p, ID %d", GetName().c_str(), static_cast(this), GetUniqueID()); - // Notify the server that the player is being destroyed - cRoot::Get()->GetServer()->PlayerDestroying(this); - SaveToDisk(); m_ClientHandle = nullptr; diff --git a/src/FastRandom.h b/src/FastRandom.h index 7c3048118..7ae5949d4 100644 --- a/src/FastRandom.h +++ b/src/FastRandom.h @@ -15,9 +15,6 @@ prefer calls to GetRandomProvider over creating new instances. #pragma once -#include -#include -#include diff --git a/src/Globals.h b/src/Globals.h index a48b48d9c..854ddcc62 100644 --- a/src/Globals.h +++ b/src/Globals.h @@ -251,7 +251,9 @@ template class SizeChecker; #include #include #include -#include +#include +#include +#include diff --git a/src/Protocol/ProtocolRecognizer.cpp b/src/Protocol/ProtocolRecognizer.cpp index 187a73a6e..43facde72 100644 --- a/src/Protocol/ProtocolRecognizer.cpp +++ b/src/Protocol/ProtocolRecognizer.cpp @@ -1157,8 +1157,8 @@ void cProtocolRecognizer::HandlePacketStatusRequest(void) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); diff --git a/src/Protocol/Protocol_1_10.cpp b/src/Protocol/Protocol_1_10.cpp index 16c1b3277..bc6b89635 100644 --- a/src/Protocol/Protocol_1_10.cpp +++ b/src/Protocol/Protocol_1_10.cpp @@ -321,8 +321,8 @@ void cProtocol_1_10_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); diff --git a/src/Protocol/Protocol_1_11.cpp b/src/Protocol/Protocol_1_11.cpp index 3376e8f88..4a4006390 100644 --- a/src/Protocol/Protocol_1_11.cpp +++ b/src/Protocol/Protocol_1_11.cpp @@ -561,8 +561,8 @@ void cProtocol_1_11_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); @@ -1184,8 +1184,8 @@ void cProtocol_1_11_1::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); diff --git a/src/Protocol/Protocol_1_12.cpp b/src/Protocol/Protocol_1_12.cpp index a25ac1c0d..278c063cc 100644 --- a/src/Protocol/Protocol_1_12.cpp +++ b/src/Protocol/Protocol_1_12.cpp @@ -374,8 +374,8 @@ void cProtocol_1_12::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); @@ -1262,7 +1262,7 @@ void cProtocol_1_12::SendLogin(const cPlayer & a_Player, const cWorld & a_World) Pkt.WriteBEUInt8(static_cast(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4 Pkt.WriteBEInt32(static_cast(a_World.GetDimension())); Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal) - Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); + Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); Pkt.WriteString("default"); // Level type - wtf? Pkt.WriteBool(false); // Reduced Debug Info - wtf? } diff --git a/src/Protocol/Protocol_1_8.cpp b/src/Protocol/Protocol_1_8.cpp index aa5af83e1..895dde3ed 100644 --- a/src/Protocol/Protocol_1_8.cpp +++ b/src/Protocol/Protocol_1_8.cpp @@ -639,7 +639,7 @@ void cProtocol_1_8_0::SendLogin(const cPlayer & a_Player, const cWorld & a_World Pkt.WriteBEUInt8(static_cast(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4 Pkt.WriteBEInt8(static_cast(a_World.GetDimension())); Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal) - Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); + Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); Pkt.WriteString("default"); // Level type - wtf? Pkt.WriteBool(false); // Reduced Debug Info - wtf? } @@ -2110,8 +2110,8 @@ void cProtocol_1_8_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); diff --git a/src/Protocol/Protocol_1_9.cpp b/src/Protocol/Protocol_1_9.cpp index 665a1f457..3b47e2b8a 100644 --- a/src/Protocol/Protocol_1_9.cpp +++ b/src/Protocol/Protocol_1_9.cpp @@ -656,7 +656,7 @@ void cProtocol_1_9_0::SendLogin(const cPlayer & a_Player, const cWorld & a_World Pkt.WriteBEUInt8(static_cast(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4 Pkt.WriteBEInt8(static_cast(a_World.GetDimension())); Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal) - Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); + Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); Pkt.WriteString("default"); // Level type - wtf? Pkt.WriteBool(false); // Reduced Debug Info - wtf? } @@ -2135,8 +2135,8 @@ void cProtocol_1_9_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); @@ -4110,7 +4110,7 @@ void cProtocol_1_9_1::SendLogin(const cPlayer & a_Player, const cWorld & a_World Pkt.WriteBEUInt8(static_cast(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4 Pkt.WriteBEInt32(static_cast(a_World.GetDimension())); Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal) - Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); + Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); Pkt.WriteString("default"); // Level type - wtf? Pkt.WriteBool(false); // Reduced Debug Info - wtf? } @@ -4139,8 +4139,8 @@ void cProtocol_1_9_1::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); @@ -4196,8 +4196,8 @@ void cProtocol_1_9_2::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); @@ -4253,8 +4253,8 @@ void cProtocol_1_9_4::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) { cServer * Server = cRoot::Get()->GetServer(); AString ServerDescription = Server->GetDescription(); - int NumPlayers = Server->GetNumPlayers(); - int MaxPlayers = Server->GetMaxPlayers(); + auto NumPlayers = static_cast(Server->GetNumPlayers()); + auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); 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) diff --git a/src/Server.h b/src/Server.h index 68741b144..88f91120c 100644 --- a/src/Server.h +++ b/src/Server.h @@ -67,9 +67,9 @@ public: const AString & GetShutdownMessage(void) const { return m_ShutdownMessage; } // Player counts: - int GetMaxPlayers(void) const { return m_MaxPlayers; } - int GetNumPlayers(void) const; - void SetMaxPlayers(int a_MaxPlayers) { m_MaxPlayers = a_MaxPlayers; } + size_t GetMaxPlayers(void) const { return m_MaxPlayers; } + size_t GetNumPlayers(void) const { return m_PlayerCount; } + void SetMaxPlayers(size_t a_MaxPlayers) { m_MaxPlayers = a_MaxPlayers; } /** Check if the player is queued to be transferred to a World. Returns true is Player is found in queue. */ @@ -106,17 +106,14 @@ public: const AString & GetServerID(void) const { return m_ServerID; } // tolua_export - /** Called by cClientHandle's destructor; stop m_SocketThreads from calling back into a_Client */ - void ClientDestroying(const cClientHandle * a_Client); - /** Don't tick a_Client anymore, it will be ticked from its cPlayer instead */ void ClientMovedToWorld(const cClientHandle * a_Client); /** Notifies the server that a player was created; the server uses this to adjust the number of players */ - void PlayerCreated(const cPlayer * a_Player); + void PlayerCreated(); /** Notifies the server that a player is being destroyed; the server uses this to adjust the number of players */ - void PlayerDestroying(const cPlayer * a_Player); + void PlayerDestroyed(); /** Returns base64 encoded favicon data (obtained from favicon.png) */ const AString & GetFaviconData(void) const { return m_FaviconData; } @@ -182,17 +179,8 @@ private: /** Clients that have just been moved into a world and are to be removed from m_Clients in the next Tick(). */ cClientHandles m_ClientsToRemove; - /** Protects m_PlayerCount against multithreaded access. */ - mutable cCriticalSection m_CSPlayerCount; - /** Number of players currently playing in the server. */ - int m_PlayerCount; - - /** Protects m_PlayerCountDiff against multithreaded access. */ - cCriticalSection m_CSPlayerCountDiff; - - /** Adjustment to m_PlayerCount to be applied in the Tick thread. */ - int m_PlayerCountDiff; + std::atomic_size_t m_PlayerCount; int m_ClientViewDistance; // The default view distance for clients; settable in Settings.ini @@ -211,7 +199,7 @@ private: AString m_Description; AString m_ShutdownMessage; AString m_FaviconData; - int m_MaxPlayers; + size_t m_MaxPlayers; bool m_bIsHardcore; /** True - allow same username to login more than once False - only once */