Decreased the scope of read mutex

include/TServer.h

@@ -26,7 +26,7 @@ public:
 
     static void GlobalParser(const std::weak_ptr<TClient>& 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;

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<TClient>& ClientPtr) -> bool {
+        ReadLock Lock(mServer.GetClientMutex());
         if (!ClientPtr.expired()) {
             Return += ClientPtr.lock()->GetName() + ";";
         }

src/TLuaFile.cpp

@@ -233,6 +233,7 @@ int lua_Sleep(lua_State* L) {
 std::optional<std::weak_ptr<TClient>> GetClient(TServer& Server, int ID) {
     std::optional<std::weak_ptr<TClient>> MaybeClient { std::nullopt };
     Server.ForEachClient([&](std::weak_ptr<TClient> 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<TClient>& ClientPtr) -> bool {
-        if (ClientPtr.expired())
+        std::shared_ptr<TClient> Client;
-            return true;
+        {
-        auto Client = ClientPtr.lock();
+            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);

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<TClient> ClientPtr) -> bool {
-                if (!ClientPtr.expired()) {
+                std::shared_ptr<TClient> Client;
-                    auto Client = ClientPtr.lock();
+                {
-                    if (Client->GetID() == ID) {
+                    ReadLock Lock(mServer.GetClientMutex());
-                        Client->UpdatePingTime();
+                    if (!ClientPtr.expired()) {
-                        Client->SetUDPAddr(client);
+                        Client = ClientPtr.lock();
-                        Client->SetIsConnected(true);
+                    }else return true;
-                        TServer::GlobalParser(ClientPtr, Data.substr(2), mPPSMonitor, *this);
-                    }
                 }
+
+                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<TClient>& 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<TClient>& ClientPtr) -> bool {
-        if (!ClientPtr.expired()) {
+        std::shared_ptr<TClient> Cl;
-            auto Cl = ClientPtr.lock();
+        {
-            info("Client Iteration: Name -> " + Cl->GetName() + ", Guest -> " + std::to_string(Cl->IsGuest()) + ", Roles -> " + Cl->GetRoles());
+            ReadLock Lock(mServer.GetClientMutex());
-            if (Cl->GetName() == Client->GetName() && Cl->IsGuest() == Client->IsGuest()) {
+            if (!ClientPtr.expired()) {
-                info("New client matched with current iteration");
+                Cl = ClientPtr.lock();
-                info("Old client (" + Cl->GetName() + ") kicked: Reconnecting");
+            } else return true;
-                CloseSocketProper(Cl->GetTCPSock());
-                Cl->SetStatus(-2);
-                return false;
-            }
         }
+        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<TClient>& 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<TClient>& 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<TClient>& 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<TClient>& c) {
     bool Return = false;
     bool res = true;
     mServer.ForEachClient([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
-        if (!ClientPtr.expired()) {
+        std::shared_ptr<TClient> client;
-            auto client = ClientPtr.lock();
+        {
-            TClient::TSetOfVehicleData VehicleData;
+            ReadLock Lock(mServer.GetClientMutex());
-            { // Vehicle Data Lock Scope
+            if (!ClientPtr.expired()) {
-                auto LockedData = client->GetAllCars();
+                client = ClientPtr.lock();
-                VehicleData = LockedData.VehicleData;
+            } else return true;
-            } // End Vehicle Data Lock Scope
+        }
-            if (client != LockedClient) {
+        TClient::TSetOfVehicleData VehicleData;
-                for (auto& v : VehicleData) {
+        { // Vehicle Data Lock Scope
-                    if (LockedClient->GetStatus() < 0) {
+            auto LockedData = client->GetAllCars();
-                        Return = true;
+            VehicleData = LockedData.VehicleData;
-                        res = false;
+        } // End Vehicle Data Lock Scope
-                        return false;
+        if (client != LockedClient) {
-                    }
+            for (auto& v : VehicleData) {
-                    res = Respond(*LockedClient, v.Data(), true, true);
+                if (LockedClient->GetStatus() < 0) {
-                    std::this_thread::sleep_for(std::chrono::seconds(2));
+                    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<TClient> ClientPtr) -> bool {
-        if (!ClientPtr.expired()) {
+        std::shared_ptr<TClient> Client;
-            auto Client = ClientPtr.lock();
+        {
-            if (Self || Client.get() != c) {
+            ReadLock Lock(mServer.GetClientMutex());
-                if (Client->IsSynced() || Client->IsSyncing()) {
+            if (!ClientPtr.expired()) {
-                    if (Rel || C == 'W' || C == 'Y' || C == 'V' || C == 'E') {
+                Client = ClientPtr.lock();
-                        if (C == 'O' || C == 'T' || Data.length() > 1000) {
+            }else return true;
-                            ret = SendLarge(*Client, Data);
+        }
-                        } else {
+        if (Self || Client.get() != c) {
-                            ret = TCPSend(*Client, Data);
+            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);
                 }
             }
         }

src/TPPSMonitor.cpp

@@ -31,21 +31,26 @@ void TPPSMonitor::operator()() {
             continue;
         }
         mServer.ForEachClient([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
-            if (!ClientPtr.expired()) {
+            std::shared_ptr<TClient> c;
-                auto c = ClientPtr.lock();
+            {
-                if (c->GetCarCount() > 0) {
+                ReadLock Lock(mServer.GetClientMutex());
-                    C++;
+                if (!ClientPtr.expired()) {
-                    V += c->GetCarCount();
+                    c = ClientPtr.lock();
-                }
+                } else return true;
-                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);
-                }
             }
+            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) {

src/TServer.cpp

@@ -48,7 +48,6 @@ std::weak_ptr<TClient> TServer::InsertNewClient() {
 }
 
 void TServer::ForEachClient(const std::function<bool(std::weak_ptr<TClient>)>& Fn) {
-    ReadLock Lock(mClientsMutex);
     for (auto& Client : mClients) {
         if (!Fn(Client)) {
             break;