fix various issues

include/Client.h

@@ -13,13 +13,18 @@ class TServer;
 
 class TClient final {
 public:
-    using TSetOfVehicleData = std::unordered_set<std::unique_ptr<TVehicleData>>;
+    using TSetOfVehicleData = std::unordered_set<TVehicleData>;
+
+    struct TVehicleDataLockPair {
+        TSetOfVehicleData& VehicleData;
+        std::unique_lock<std::mutex> Lock;
+    };
 
     explicit TClient(TServer& Server);
 
     void AddNewCar(int Ident, const std::string& Data);
     void SetCarData(int Ident, const std::string& Data);
-    TSetOfVehicleData& GetAllCars();
+    TVehicleDataLockPair GetAllCars();
     void SetName(const std::string& Name) { mName = Name; }
     void SetRoles(const std::string& Role) { mRole = Role; }
     std::string GetCarData(int Ident);
@@ -54,6 +59,7 @@ private:
     bool mIsConnected = false;
     bool mIsSynced = false;
     bool mIsGuest = false;
+    std::mutex mVehicleDataMutex;
     TSetOfVehicleData mVehicleData;
     std::string mName = "Unknown Client";
     SOCKET mSocket[2] { SOCKET(-1) };

include/Common.h

@@ -56,6 +56,9 @@ private:
     static inline std::deque<TShutdownHandler> mShutdownHandlers {};
 };
 
+#define KB 1024
+#define MB (KB * 1024)
+
 #ifndef DEBUG
 static inline void warn(const std::string& str) {
     Application::Console().Write(std::string("[WARN] ") + str);

include/VehicleData.h

@@ -7,13 +7,24 @@ public:
     TVehicleData(int ID, const std::string& Data);
     ~TVehicleData();
 
-    bool IsInvalid() const { return _ID == -1; }
+    [[nodiscard]] bool IsInvalid() const { return mID == -1; }
-    int ID() const { return _ID; }
+    [[nodiscard]] int ID() const { return mID; }
 
-    std::string Data() const { return _Data; }
+    [[nodiscard]] std::string Data() const { return mData; }
-    void SetData(const std::string& Data) { _Data = Data; }
+    void SetData(const std::string& Data) { mData = Data; }
+
+    bool operator==(const TVehicleData& v) const { return mID == v.mID; }
 
 private:
-    int _ID { -1 };
+    int mID { -1 };
-    std::string _Data;
+    std::string mData;
 };
+
+namespace std {
+template <>
+struct hash<TVehicleData> {
+    std::size_t operator()(const TVehicleData& s) const noexcept {
+        return s.ID();
+    }
+};
+}

src/Client.cpp

@@ -7,7 +7,7 @@
 
 void TClient::DeleteCar(int Ident) {
     for (auto& v : mVehicleData) {
-        if (v != nullptr && v->ID() == Ident) {
+        if (v.ID() == Ident) {
             mVehicleData.erase(v);
             break;
         }
@@ -24,7 +24,7 @@ int TClient::GetOpenCarID() const {
     do {
         found = true;
         for (auto& v : mVehicleData) {
-            if (v != nullptr && v->ID() == OpenID) {
+            if (v.ID() == OpenID) {
                 OpenID++;
                 found = false;
             }
@@ -34,17 +34,17 @@ int TClient::GetOpenCarID() const {
 }
 
 void TClient::AddNewCar(int Ident, const std::string& Data) {
-    mVehicleData.insert(std::make_unique<TVehicleData>(Ident, Data));
+    mVehicleData.insert(TVehicleData(Ident, Data));
 }
 
-TClient::TSetOfVehicleData& TClient::GetAllCars() {
+TClient::TVehicleDataLockPair TClient::GetAllCars() {
-    return mVehicleData;
+    return { mVehicleData, std::unique_lock(mVehicleDataMutex) };
 }
 
 std::string TClient::GetCarData(int Ident) {
     for (auto& v : mVehicleData) {
-        if (v != nullptr && v->ID() == Ident) {
+        if (v.ID() == Ident) {
-            return v->Data();
+            return v.Data();
         }
     }
     DeleteCar(Ident);
@@ -53,8 +53,8 @@ std::string TClient::GetCarData(int Ident) {
 
 void TClient::SetCarData(int Ident, const std::string& Data) {
     for (auto& v : mVehicleData) {
-        if (v != nullptr && v->ID() == Ident) {
+        if (v.ID() == Ident) {
-            v->Data() = Data;
+            v.Data() = Data;
             return;
         }
     }

src/TLuaFile.cpp

@@ -179,8 +179,8 @@ void SafeExecution(TLuaEngine& Engine, TLuaFile* lua, const std::string& FuncNam
         char* Origin = ThreadOrigin(lua);
 #ifdef WIN32
         //__try {
-            int R = lua_pcall(luaState, 0, 0, 0);
+        int R = lua_pcall(luaState, 0, 0, 0);
-            CheckLua(luaState, R);
+        CheckLua(luaState, R);
         /*} __except (Handle(GetExceptionInformation(), Origin)) {
         }*/
 #else // unix
@@ -328,15 +328,18 @@ int lua_GetCars(lua_State* L) {
         if (MaybeClient && !MaybeClient.value().expired()) {
             auto Client = MaybeClient.value().lock();
             int i = 1;
-            for (auto& v : Client->GetAllCars()) {
+            TClient::TSetOfVehicleData VehicleData;
-                if (v != nullptr) {
+            { // Vehicle Data Lock Scope
-                    lua_pushinteger(L, v->ID());
+                auto LockedData = Client->GetAllCars();
-                    lua_pushstring(L, v->Data().substr(3).c_str());
+                VehicleData = LockedData.VehicleData;
-                    lua_settable(L, -3);
+            } // End Vehicle Data Lock Scope
-                    i++;
+            for (auto& v : VehicleData) {
-                }
+                lua_pushinteger(L, v.ID());
+                lua_pushstring(L, v.Data().substr(3).c_str());
+                lua_settable(L, -3);
+                i++;
             }
-            if (Client->GetAllCars().empty())
+            if (VehicleData.empty())
                 return 0;
         } else
             return 0;
@@ -575,8 +578,7 @@ int lua_Print(lua_State* L) {
 
 int lua_TempFix(lua_State* L);
 
-
+void TLuaFile::Init(const std::string& PluginName, const std::string& FileName, fs::file_time_type LastWrote) {
-void TLuaFile::Init(const std::string& PluginName, const std::string& FileName, fs::file_time_type LastWrote){
     // set global engine for lua_* functions
     if (!TheEngine) {
         TheEngine = &mEngine;
@@ -595,7 +597,7 @@ void TLuaFile::Init(const std::string& PluginName, const std::string& FileName,
 TLuaFile::TLuaFile(TLuaEngine& Engine, bool Console)
     : mEngine(Engine)
     , mLuaState(luaL_newstate()) {
-    if(Console) {
+    if (Console) {
         mConsole = Console;
         Load();
     }

src/TTCPServer.cpp

@@ -226,7 +226,13 @@ std::string TTCPServer::TCPRcv(TClient& c) {
 #endif // DEBUG
         return "";
     }
-    Data.resize(Header);
+    if (Header < 100 * MB) {
+        Data.resize(Header);
+    } else {
+        ClientKick(c, "Header size limit exceeded");
+        warn("Client " + c.GetName() + " (" + std::to_string(c.GetID()) + ") sent header of >100MB - assuming malicious intent and disconnecting the client.");
+        return "";
+    }
     BytesRcv = 0;
     do {
         Temp = recv(c.GetTCPSock(), &Data[BytesRcv], Header - BytesRcv, 0);
@@ -307,11 +313,14 @@ void TTCPServer::OnDisconnect(const std::weak_ptr<TClient>& ClientPtr, bool kick
     TClient& c = *LockedClientPtr;
     info(c.GetName() + (" Connection Terminated"));
     std::string Packet;
-    for (auto& v : c.GetAllCars()) {
+    TClient::TSetOfVehicleData VehicleData;
-        if (v != nullptr) {
+    { // Vehicle Data Lock Scope
-            Packet = "Od:" + std::to_string(c.GetID()) + "-" + std::to_string(v->ID());
+        auto LockedData = c.GetAllCars();
-            UDPServer().SendToAll(&c, Packet, false, true);
+        VehicleData = LockedData.VehicleData;
-        }
+    } // End Vehicle Data Lock Scope
+    for (auto& v : VehicleData) {
+        Packet = "Od:" + std::to_string(c.GetID()) + "-" + std::to_string(v.ID());
+        UDPServer().SendToAll(&c, Packet, false, true);
     }
     if (kicked)
         Packet = ("L") + c.GetName() + (" was kicked!");
@@ -538,16 +547,19 @@ void TTCPServer::SyncClient(const std::weak_ptr<TClient>& c) {
     mServer.ForEachClient([&](const std::weak_ptr<TClient>& 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 : client->GetAllCars()) {
+                for (auto& v : VehicleData) {
-                    if (v != nullptr) {
+                    if (LockedClient->GetStatus() < 0) {
-                        if (LockedClient->GetStatus() < 0) {
+                        Return = true;
-                            Return = true;
+                        return false;
-                            return false;
-                        }
-                        Respond(*LockedClient, v->Data(), true);
-                        std::this_thread::sleep_for(std::chrono::seconds(2));
                     }
+                    Respond(*LockedClient, v.Data(), true);
+                    std::this_thread::sleep_for(std::chrono::seconds(2));
                 }
             }
         }

src/TUDPServer.cpp

@@ -114,9 +114,10 @@ void TUDPServer::SendToAll(TClient* c, const std::string& Data, bool Self, bool
 
 void TUDPServer::UDPSend(TClient& Client, std::string Data) const {
     if (!Client.IsConnected() || Client.GetStatus() < 0) {
-#ifdef DEBUG
+        // this can happen if we try to send a packet to a client that is either
-        debug(Client.GetName() + ": !IsConnected() or GetStatus() < 0");
+        // 1. not yet fully connected, or
-#endif // DEBUG
+        // 2. disconnected and not yet fully removed
+        // this is fine can can be ignored :^)
         return;
     }
     sockaddr_in Addr = Client.GetUDPAddr();

src/VehicleData.cpp

@@ -2,11 +2,11 @@
 #include "Common.h"
 
 TVehicleData::TVehicleData(int ID, const std::string& Data)
-    : _ID(ID)
+    : mID(ID)
-    , _Data(Data) {
+    , mData(Data) {
-    debug("vehicle " + std::to_string(_ID) + " constructed");
+    debug("vehicle " + std::to_string(mID) + " constructed");
 }
 
 TVehicleData::~TVehicleData() {
-    debug("vehicle " + std::to_string(_ID) + " destroyed");
+    debug("vehicle " + std::to_string(mID) + " destroyed");
 }