From f323d50e34bb7c003a35299e9a4607fd346bc6d5 Mon Sep 17 00:00:00 2001 From: Anonymous-275 Date: Wed, 31 Mar 2021 00:05:05 +0300 Subject: [PATCH] Decreased the scope of read mutex --- include/TServer.h | 2 +- src/THeartbeatThread.cpp | 1 + src/TLuaFile.cpp | 11 ++-- src/TNetwork.cpp | 113 +++++++++++++++++++++++---------------- src/TPPSMonitor.cpp | 33 +++++++----- src/TServer.cpp | 1 - 6 files changed, 97 insertions(+), 64 deletions(-) diff --git a/include/TServer.h b/include/TServer.h index 19249a8..426ec33 100644 --- a/include/TServer.h +++ b/include/TServer.h @@ -26,7 +26,7 @@ public: static void GlobalParser(const std::weak_ptr& Client, std::string Packet, TPPSMonitor& PPSMonitor, TNetwork& Network); static void HandleEvent(TClient& c, const std::string& Data); - + RWMutex& GetClientMutex() const { return mClientsMutex; } private: TClientSet mClients; mutable RWMutex mClientsMutex; diff --git a/src/THeartbeatThread.cpp b/src/THeartbeatThread.cpp index a9bdcbe..f6910e3 100644 --- a/src/THeartbeatThread.cpp +++ b/src/THeartbeatThread.cpp @@ -99,6 +99,7 @@ THeartbeatThread::THeartbeatThread(TResourceManager& ResourceManager, TServer& S std::string THeartbeatThread::GetPlayers() { std::string Return; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { + ReadLock Lock(mServer.GetClientMutex()); if (!ClientPtr.expired()) { Return += ClientPtr.lock()->GetName() + ";"; } diff --git a/src/TLuaFile.cpp b/src/TLuaFile.cpp index 4a24086..3af050c 100644 --- a/src/TLuaFile.cpp +++ b/src/TLuaFile.cpp @@ -233,6 +233,7 @@ int lua_Sleep(lua_State* L) { std::optional> GetClient(TServer& Server, int ID) { std::optional> MaybeClient { std::nullopt }; Server.ForEachClient([&](std::weak_ptr CPtr) -> bool { + ReadLock Lock(Server.GetClientMutex()); if (!CPtr.expired()) { auto C = CPtr.lock(); if (C->GetID() == ID) { @@ -294,9 +295,13 @@ int lua_GetGuest(lua_State* L) { int lua_GetAllPlayers(lua_State* L) { lua_newtable(L); Engine().Server().ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { - if (ClientPtr.expired()) - return true; - auto Client = ClientPtr.lock(); + std::shared_ptr Client; + { + ReadLock Lock(Engine().Server().GetClientMutex()); + if (ClientPtr.expired()) + return true; + Client = ClientPtr.lock(); + } lua_pushinteger(L, Client->GetID()); lua_pushstring(L, Client->GetName().c_str()); lua_settable(L, -3); diff --git a/src/TNetwork.cpp b/src/TNetwork.cpp index 6397db4..a2b9e57 100644 --- a/src/TNetwork.cpp +++ b/src/TNetwork.cpp @@ -83,15 +83,21 @@ void TNetwork::UDPServerMain() { inet_ntop(AF_INET, &client.sin_addr, clientIp, 256);*/ uint8_t ID = uint8_t(Data.at(0)) - 1; mServer.ForEachClient([&](std::weak_ptr ClientPtr) -> bool { - if (!ClientPtr.expired()) { - auto Client = ClientPtr.lock(); - if (Client->GetID() == ID) { - Client->UpdatePingTime(); - Client->SetUDPAddr(client); - Client->SetIsConnected(true); - TServer::GlobalParser(ClientPtr, Data.substr(2), mPPSMonitor, *this); - } + std::shared_ptr Client; + { + ReadLock Lock(mServer.GetClientMutex()); + if (!ClientPtr.expired()) { + Client = ClientPtr.lock(); + }else return true; } + + if (Client->GetID() == ID) { + Client->UpdatePingTime(); + Client->SetUDPAddr(client); + Client->SetIsConnected(true); + TServer::GlobalParser(ClientPtr, Data.substr(2), mPPSMonitor, *this); + } + return true; }); } catch (const std::exception& e) { @@ -227,6 +233,7 @@ void TNetwork::HandleDownload(SOCKET TCPSock) { } auto ID = uint8_t(D); mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { + ReadLock Lock(mServer.GetClientMutex()); if (!ClientPtr.expired()) { auto c = ClientPtr.lock(); if (c->GetID() == ID) { @@ -300,17 +307,22 @@ void TNetwork::Authentication(SOCKET TCPSock) { debug("Name -> " + Client->GetName() + ", Guest -> " + std::to_string(Client->IsGuest()) + ", Roles -> " + Client->GetRoles()); debug("There are " + std::to_string(mServer.ClientCount()) + " known clients"); mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { - if (!ClientPtr.expired()) { - auto Cl = ClientPtr.lock(); - info("Client Iteration: Name -> " + Cl->GetName() + ", Guest -> " + std::to_string(Cl->IsGuest()) + ", Roles -> " + Cl->GetRoles()); - if (Cl->GetName() == Client->GetName() && Cl->IsGuest() == Client->IsGuest()) { - info("New client matched with current iteration"); - info("Old client (" + Cl->GetName() + ") kicked: Reconnecting"); - CloseSocketProper(Cl->GetTCPSock()); - Cl->SetStatus(-2); - return false; - } + std::shared_ptr Cl; + { + ReadLock Lock(mServer.GetClientMutex()); + if (!ClientPtr.expired()) { + Cl = ClientPtr.lock(); + } else return true; } + info("Client Iteration: Name -> " + Cl->GetName() + ", Guest -> " + std::to_string(Cl->IsGuest()) + ", Roles -> " + Cl->GetRoles()); + if (Cl->GetName() == Client->GetName() && Cl->IsGuest() == Client->IsGuest()) { + info("New client matched with current iteration"); + info("Old client (" + Cl->GetName() + ") kicked: Reconnecting"); + CloseSocketProper(Cl->GetTCPSock()); + Cl->SetStatus(-2); + return false; + } + return true; }); @@ -531,6 +543,7 @@ void TNetwork::TCPClient(const std::weak_ptr& c) { void TNetwork::UpdatePlayer(TClient& Client) { std::string Packet = ("Ss") + std::to_string(mServer.ClientCount()) + "/" + std::to_string(Application::Settings.MaxPlayers) + ":"; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { + ReadLock Lock(mServer.GetClientMutex()); if (!ClientPtr.expired()) { auto c = ClientPtr.lock(); Packet += c->GetName() + ","; @@ -576,6 +589,7 @@ int TNetwork::OpenID() { do { found = true; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { + ReadLock Lock(mServer.GetClientMutex()); if (!ClientPtr.expired()) { auto c = ClientPtr.lock(); if (c->GetID() == ID) { @@ -794,25 +808,30 @@ bool TNetwork::SyncClient(const std::weak_ptr& c) { bool Return = false; bool res = true; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { - if (!ClientPtr.expired()) { - auto client = ClientPtr.lock(); - TClient::TSetOfVehicleData VehicleData; - { // Vehicle Data Lock Scope - auto LockedData = client->GetAllCars(); - VehicleData = LockedData.VehicleData; - } // End Vehicle Data Lock Scope - if (client != LockedClient) { - for (auto& v : VehicleData) { - if (LockedClient->GetStatus() < 0) { - Return = true; - res = false; - return false; - } - res = Respond(*LockedClient, v.Data(), true, true); - std::this_thread::sleep_for(std::chrono::seconds(2)); + std::shared_ptr client; + { + ReadLock Lock(mServer.GetClientMutex()); + if (!ClientPtr.expired()) { + client = ClientPtr.lock(); + } else return true; + } + TClient::TSetOfVehicleData VehicleData; + { // Vehicle Data Lock Scope + auto LockedData = client->GetAllCars(); + VehicleData = LockedData.VehicleData; + } // End Vehicle Data Lock Scope + if (client != LockedClient) { + for (auto& v : VehicleData) { + if (LockedClient->GetStatus() < 0) { + Return = true; + res = false; + return false; } + res = Respond(*LockedClient, v.Data(), true, true); + std::this_thread::sleep_for(std::chrono::seconds(2)); } } + return true; }); LockedClient->SetIsSyncing(false); @@ -830,19 +849,23 @@ void TNetwork::SendToAll(TClient* c, const std::string& Data, bool Self, bool Re char C = Data.at(0); bool ret = true; mServer.ForEachClient([&](std::weak_ptr ClientPtr) -> bool { - if (!ClientPtr.expired()) { - auto Client = ClientPtr.lock(); - if (Self || Client.get() != c) { - if (Client->IsSynced() || Client->IsSyncing()) { - if (Rel || C == 'W' || C == 'Y' || C == 'V' || C == 'E') { - if (C == 'O' || C == 'T' || Data.length() > 1000) { - ret = SendLarge(*Client, Data); - } else { - ret = TCPSend(*Client, Data); - } + std::shared_ptr Client; + { + ReadLock Lock(mServer.GetClientMutex()); + if (!ClientPtr.expired()) { + Client = ClientPtr.lock(); + }else return true; + } + if (Self || Client.get() != c) { + if (Client->IsSynced() || Client->IsSyncing()) { + if (Rel || C == 'W' || C == 'Y' || C == 'V' || C == 'E') { + if (C == 'O' || C == 'T' || Data.length() > 1000) { + ret = SendLarge(*Client, Data); } else { - ret = UDPSend(*Client, Data); + ret = TCPSend(*Client, Data); } + } else { + ret = UDPSend(*Client, Data); } } } diff --git a/src/TPPSMonitor.cpp b/src/TPPSMonitor.cpp index d286677..2712802 100644 --- a/src/TPPSMonitor.cpp +++ b/src/TPPSMonitor.cpp @@ -31,21 +31,26 @@ void TPPSMonitor::operator()() { continue; } mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { - if (!ClientPtr.expired()) { - auto c = ClientPtr.lock(); - if (c->GetCarCount() > 0) { - C++; - V += c->GetCarCount(); - } - if (!c->IsSynced() || c->IsSyncing()) { - c->UpdatePingTime(); - } - // kick on "no ping" - if (c->SecondsSinceLastPing() > 60 && c->IsSynced() && !c->IsSyncing()) { - debug("client " + std::string("(") + std::to_string(c->GetID()) + ")" + c->GetName() + " timing out: " + std::to_string(c->SecondsSinceLastPing()) + ", pps: " + Application::PPS()); - TimedOutClients.push_back(c); - } + std::shared_ptr c; + { + ReadLock Lock(mServer.GetClientMutex()); + if (!ClientPtr.expired()) { + c = ClientPtr.lock(); + } else return true; } + if (c->GetCarCount() > 0) { + C++; + V += c->GetCarCount(); + } + if (!c->IsSynced() || c->IsSyncing()) { + c->UpdatePingTime(); + } + // kick on "no ping" + if (c->SecondsSinceLastPing() > 60 && c->IsSynced() && !c->IsSyncing()) { + debug("client " + std::string("(") + std::to_string(c->GetID()) + ")" + c->GetName() + " timing out: " + std::to_string(c->SecondsSinceLastPing()) + ", pps: " + Application::PPS()); + TimedOutClients.push_back(c); + } + return true; }); for (auto& ClientToKick : TimedOutClients) { diff --git a/src/TServer.cpp b/src/TServer.cpp index 6111508..bbc3a6d 100644 --- a/src/TServer.cpp +++ b/src/TServer.cpp @@ -48,7 +48,6 @@ std::weak_ptr TServer::InsertNewClient() { } void TServer::ForEachClient(const std::function)>& Fn) { - ReadLock Lock(mClientsMutex); for (auto& Client : mClients) { if (!Fn(Client)) { break;