flewkey/cuberite-2a
1
0
Fork 0
Code Releases Activity

Tentative fix for player-limit race condition (#3862)

Browse Source 
* Attempts to fix #2257

Derived from d233e9843148313c71fbaba96ccff660e47b07b1

* Changed player count type to int

* Clarified certain actions
This commit is contained in:
Tiger Wang 2017-07-28 17:54:40 +01:00 committed by GitHub
parent e0a44fb3bc
commit eb4432bb62
12 changed files with 61 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/ClientHandle.cpp
View File

@ -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);

5
src/Entities/Player.cpp
View File

@ -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<void *>(this), GetUniqueID());
// Notify the server that the player is being destroyed
cRoot::Get()->GetServer()->PlayerDestroying(this);
SaveToDisk();
m_ClientHandle = nullptr;

3
src/FastRandom.h
View File

@ -15,9 +15,6 @@ prefer calls to GetRandomProvider over creating new instances.
#pragma once
#include <random>
#include <type_traits>
#include <limits>

4
src/Globals.h
View File

@ -251,7 +251,9 @@ template class SizeChecker<UInt8, 1>;
#include <set>
#include <queue>
#include <limits>
#include <chrono>
#include <random>
#include <type_traits>
#include <atomic>

4
src/Protocol/ProtocolRecognizer.cpp
View File

@ -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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
AString Favicon = Server->GetFaviconData();
cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);

4
src/Protocol/Protocol_1_10.cpp
View File

@ -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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
AString Favicon = Server->GetFaviconData();
cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);

8
src/Protocol/Protocol_1_11.cpp
View File

@ -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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
AString Favicon = Server->GetFaviconData();
cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);

6
src/Protocol/Protocol_1_12.cpp
View File

@ -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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(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<UInt8>(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4
Pkt.WriteBEInt32(static_cast<Int32>(a_World.GetDimension()));
Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal)
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<int>(Server->GetMaxPlayers(), 0, 255)));
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<size_t>(Server->GetMaxPlayers(), 0, 255)));
Pkt.WriteString("default"); // Level type - wtf?
Pkt.WriteBool(false); // Reduced Debug Info - wtf?
}

6
src/Protocol/Protocol_1_8.cpp
View File

@ -639,7 +639,7 @@ void cProtocol_1_8_0::SendLogin(const cPlayer & a_Player, const cWorld & a_World
Pkt.WriteBEUInt8(static_cast<UInt8>(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4
Pkt.WriteBEInt8(static_cast<Int8>(a_World.GetDimension()));
Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal)
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<int>(Server->GetMaxPlayers(), 0, 255)));
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<size_t>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
AString Favicon = Server->GetFaviconData();
cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);

20
src/Protocol/Protocol_1_9.cpp
View File

@ -656,7 +656,7 @@ void cProtocol_1_9_0::SendLogin(const cPlayer & a_Player, const cWorld & a_World
Pkt.WriteBEUInt8(static_cast<UInt8>(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4
Pkt.WriteBEInt8(static_cast<Int8>(a_World.GetDimension()));
Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal)
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<int>(Server->GetMaxPlayers(), 0, 255)));
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<size_t>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(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<UInt8>(a_Player.GetEffectiveGameMode()) | (Server->IsHardcore() ? 0x08 : 0)); // Hardcore flag bit 4
Pkt.WriteBEInt32(static_cast<Int32>(a_World.GetDimension()));
Pkt.WriteBEUInt8(2); // TODO: Difficulty (set to Normal)
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<int>(Server->GetMaxPlayers(), 0, 255)));
Pkt.WriteBEUInt8(static_cast<UInt8>(Clamp<size_t>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(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<signed>(Server->GetNumPlayers());
auto MaxPlayers = static_cast<signed>(Server->GetMaxPlayers());
AString Favicon = Server->GetFaviconData();
cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon);

48
src/Server.cpp
View File

@ -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<size_t>(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)

26
src/Server.h
View File

@ -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 */