From 9ca12fc7a683a46ac2fd574f74013484b8fafab0 Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sat, 18 Apr 2026 18:52:26 +0200 Subject: [PATCH] fix std::weak_ptr locking and expiry checks You're supposed to .lock() instead of TOCTOU checking, of course. Not sure what I was thinking when I built that. .lock() returns a default constructed std::shared_ptr on error, which is `false` via `operator bool`. --- src/Client.cpp | 3 +- src/LuaAPI.cpp | 171 ++++++++++++++++++++++----------------- src/TConsole.cpp | 36 ++++----- src/THeartbeatThread.cpp | 4 +- src/TLuaEngine.cpp | 163 +++++++++++++++++++------------------ src/TNetwork.cpp | 80 +++++++++--------- src/TPPSMonitor.cpp | 4 +- src/TServer.cpp | 28 +++---- 8 files changed, 260 insertions(+), 229 deletions(-) diff --git a/src/Client.cpp b/src/Client.cpp index f9ae40e..7d21594 100644 --- a/src/Client.cpp +++ b/src/Client.cpp @@ -178,8 +178,7 @@ 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 (auto C = CPtr.lock()) { if (C->GetID() == ID) { MaybeClient = CPtr; return false; diff --git a/src/LuaAPI.cpp b/src/LuaAPI.cpp index e7fab3f..bb16dda 100644 --- a/src/LuaAPI.cpp +++ b/src/LuaAPI.cpp @@ -143,22 +143,23 @@ static inline std::pair InternalTriggerClientEvent(int Player return { true, "" }; } else { auto MaybeClient = GetClient(LuaAPI::MP::Engine->Server(), PlayerID); - if (!MaybeClient || MaybeClient.value().expired()) { - beammp_lua_errorf("TriggerClientEvent invalid Player ID '{}'", PlayerID); - return { false, "Invalid Player ID" }; - } - auto c = MaybeClient.value().lock(); + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + if (!c->IsSyncing() && !c->IsSynced()) { + return { false, "Player hasn't joined yet" }; + } - if (!c->IsSyncing() && !c->IsSynced()) { - return { false, "Player hasn't joined yet" }; + if (!LuaAPI::MP::Engine->Network().Respond(*c, StringToVector(Packet), true)) { + beammp_lua_errorf("Respond failed, dropping client {}", PlayerID); + LuaAPI::MP::Engine->Network().ClientKick(*c, "Disconnected after failing to receive packets"); + return { false, "Respond failed, dropping client" }; + } + return { true, "" }; + } } - if (!LuaAPI::MP::Engine->Network().Respond(*c, StringToVector(Packet), true)) { - beammp_lua_errorf("Respond failed, dropping client {}", PlayerID); - LuaAPI::MP::Engine->Network().ClientKick(*c, "Disconnected after failing to receive packets"); - return { false, "Respond failed, dropping client" }; - } - return { true, "" }; + beammp_lua_errorf("TriggerClientEvent invalid Player ID '{}'", PlayerID); + return { false, "Invalid Player ID" }; } } @@ -169,13 +170,15 @@ std::pair LuaAPI::MP::TriggerClientEvent(int PlayerID, const std::pair LuaAPI::MP::DropPlayer(int ID, std::optional MaybeReason) { auto MaybeClient = GetClient(Engine->Server(), ID); - if (!MaybeClient || MaybeClient.value().expired()) { - beammp_lua_errorf("Tried to drop client with id {}, who doesn't exist", ID); - return { false, "Player does not exist" }; + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + LuaAPI::MP::Engine->Network().ClientKick(*c, MaybeReason.value_or("No reason")); + return { true, "" }; + } } - auto c = MaybeClient.value().lock(); - LuaAPI::MP::Engine->Network().ClientKick(*c, MaybeReason.value_or("No reason")); - return { true, "" }; + + beammp_lua_errorf("Tried to drop client with id {}, who doesn't exist", ID); + return { false, "Player does not exist" }; } std::pair LuaAPI::MP::SendChatMessage(int ID, const std::string& Message, const bool& LogChat) { @@ -189,21 +192,26 @@ std::pair LuaAPI::MP::SendChatMessage(int ID, const std::stri Result.first = true; } else { auto MaybeClient = GetClient(Engine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - auto c = MaybeClient.value().lock(); - if (!c->IsSynced()) { + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + if (!c->IsSynced()) { + Result.first = false; + Result.second = "Player still syncing data"; + return Result; + } + if (LogChat) { + LogChatMessage(" (to \"" + c->GetName() + "\")", -1, Message); + } + if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { + beammp_errorf("Failed to send chat message back to sender (id {}) - did the sender disconnect?", ID); + // TODO: should we return an error here? + } + Result.first = true; + } else { + beammp_lua_error("SendChatMessage invalid argument [1] invalid ID"); Result.first = false; - Result.second = "Player still syncing data"; - return Result; + Result.second = "Invalid Player ID"; } - if (LogChat) { - LogChatMessage(" (to \"" + c->GetName() + "\")", -1, Message); - } - if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { - beammp_errorf("Failed to send chat message back to sender (id {}) - did the sender disconnect?", ID); - // TODO: should we return an error here? - } - Result.first = true; } else { beammp_lua_error("SendChatMessage invalid argument [1] invalid ID"); Result.first = false; @@ -223,18 +231,23 @@ std::pair LuaAPI::MP::SendNotification(int ID, const std::str } else { auto MaybeClient = GetClient(Engine->Server(), ID); if (MaybeClient) { - auto c = MaybeClient.value().lock(); - if (!c->IsSynced()) { + if (auto c = MaybeClient.value().lock()) { + if (!c->IsSynced()) { + Result.first = false; + Result.second = "Player is not synced yet"; + return Result; + } + if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { + beammp_errorf("Failed to send notification to player (id {}) - did the player disconnect?", ID); + Result.first = false; + Result.second = "Failed to send packet"; + } + Result.first = true; + } else { + beammp_lua_error("SendNotification invalid argument [1] invalid ID"); Result.first = false; - Result.second = "Player is not synced yet"; - return Result; + Result.second = "Invalid Player ID"; } - if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { - beammp_errorf("Failed to send notification to player (id {}) - did the player disconnect?", ID); - Result.first = false; - Result.second = "Failed to send packet"; - } - Result.first = true; } else { beammp_lua_error("SendNotification invalid argument [1] invalid ID"); Result.first = false; @@ -265,18 +278,23 @@ std::pair LuaAPI::MP::ConfirmationDialog(int ID, const std::s } else { auto MaybeClient = GetClient(Engine->Server(), ID); if (MaybeClient) { - auto c = MaybeClient.value().lock(); - if (!c->IsSynced()) { + if (auto c = MaybeClient.value().lock()) { + if (!c->IsSynced()) { + Result.first = false; + Result.second = "Player is not synced yet"; + return Result; + } + if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { + beammp_errorf("Failed to send confirmation dialog to player (id {}) - did the player disconnect?", ID); + Result.first = false; + Result.second = "Failed to send packet"; + } + Result.first = true; + } else { + beammp_lua_error("ConfirmationDialog invalid argument [1] invalid ID"); Result.first = false; - Result.second = "Player is not synced yet"; - return Result; + Result.second = "Invalid Player ID"; } - if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) { - beammp_errorf("Failed to send confirmation dialog to player (id {}) - did the player disconnect?", ID); - Result.first = false; - Result.second = "Failed to send packet"; - } - Result.first = true; } else { beammp_lua_error("ConfirmationDialog invalid argument [1] invalid ID"); Result.first = false; @@ -290,22 +308,27 @@ std::pair LuaAPI::MP::ConfirmationDialog(int ID, const std::s std::pair LuaAPI::MP::RemoveVehicle(int PID, int VID) { std::pair Result; auto MaybeClient = GetClient(Engine->Server(), PID); - if (!MaybeClient || MaybeClient.value().expired()) { + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + if (c->GetCarData(VID) != nlohmann::detail::value_t::null) { + std::string Destroy = "Od:" + std::to_string(PID) + "-" + std::to_string(VID); + LuaAPI::MP::Engine->ReportErrors(LuaAPI::MP::Engine->TriggerEvent("onVehicleDeleted", "", PID, VID)); + Engine->Network().SendToAll(nullptr, StringToVector(Destroy), true, true); + c->DeleteCar(VID); + Result.first = true; + } else { + Result.first = false; + Result.second = "Vehicle does not exist"; + } + } else { + beammp_lua_error("RemoveVehicle invalid Player ID"); + Result.first = false; + Result.second = "Invalid Player ID"; + } + } else { beammp_lua_error("RemoveVehicle invalid Player ID"); Result.first = false; Result.second = "Invalid Player ID"; - return Result; - } - auto c = MaybeClient.value().lock(); - if (c->GetCarData(VID) != nlohmann::detail::value_t::null) { - std::string Destroy = "Od:" + std::to_string(PID) + "-" + std::to_string(VID); - LuaAPI::MP::Engine->ReportErrors(LuaAPI::MP::Engine->TriggerEvent("onVehicleDeleted", "", PID, VID)); - Engine->Network().SendToAll(nullptr, StringToVector(Destroy), true, true); - c->DeleteCar(VID); - Result.first = true; - } else { - Result.first = false; - Result.second = "Vehicle does not exist"; } return Result; } @@ -412,20 +435,22 @@ void LuaAPI::MP::Sleep(size_t Ms) { bool LuaAPI::MP::IsPlayerConnected(int ID) { auto MaybeClient = GetClient(Engine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - return MaybeClient.value().lock()->IsUDPConnected(); - } else { - return false; + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + return c->IsUDPConnected(); + } } + return false; } bool LuaAPI::MP::IsPlayerGuest(int ID) { auto MaybeClient = GetClient(Engine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - return MaybeClient.value().lock()->IsGuest(); - } else { - return false; + if (MaybeClient) { + if (auto c = MaybeClient.value().lock()) { + return c->IsGuest(); + } } + return false; } void LuaAPI::MP::PrintRaw(sol::variadic_args Args) { diff --git a/src/TConsole.cpp b/src/TConsole.cpp index 5e41f82..2aca3a0 100644 --- a/src/TConsole.cpp +++ b/src/TConsole.cpp @@ -296,8 +296,7 @@ void TConsole::Command_NetTest(const std::string& cmd, const std::vector& return StringStartsWith(Name1, Name2) || StringStartsWith(Name2, Name1); }; mLuaEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { - if (!Client.expired()) { - auto locked = Client.lock(); - if (NameCompare(locked->GetName(), Name)) { - mLuaEngine->Network().ClientKick(*locked, Reason); + if (auto Locked = Client.lock()) { + if (NameCompare(Locked->GetName(), Name)) { + mLuaEngine->Network().ClientKick(*Locked, Reason); Kicked = true; return false; } @@ -356,7 +354,7 @@ std::tuple> TConsole::ParseCommand(const s // It correctly splits arguments, including respecting single and double quotes, as well as backticks auto End_i = CommandWithArgs.find_first_of(' '); std::string Command = CommandWithArgs.substr(0, End_i); - std::string ArgsStr {}; + std::string ArgsStr { }; if (End_i != std::string::npos) { ArgsStr = CommandWithArgs.substr(End_i); } @@ -566,11 +564,10 @@ void TConsole::Command_List(const std::string&, const std::vector& std::stringstream ss; ss << std::left << std::setw(25) << "Name" << std::setw(6) << "ID" << std::setw(6) << "Cars" << std::endl; mLuaEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { - if (!Client.expired()) { - auto locked = Client.lock(); - ss << std::left << std::setw(25) << locked->GetName() - << std::setw(6) << locked->GetID() - << std::setw(6) << locked->GetCarCount() << "\n"; + if (auto Locked = Client.lock()) { + ss << std::left << std::setw(25) << Locked->GetName() + << std::setw(6) << Locked->GetID() + << std::setw(6) << Locked->GetCarCount() << "\n"; } return true; }); @@ -593,8 +590,7 @@ void TConsole::Command_Status(const std::string&, const std::vector size_t MissedPacketQueueSum = 0; int LargestSecondsSinceLastPing = 0; mLuaEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { - if (!Client.expired()) { - auto Locked = Client.lock(); + if (auto Locked = Client.lock()) { CarCount += Locked->GetCarCount(); ConnectedCount += Locked->IsUDPConnected() ? 1 : 0; GuestCount += Locked->IsGuest() ? 1 : 0; @@ -613,11 +609,11 @@ void TConsole::Command_Status(const std::string&, const std::vector size_t SystemsBad = 0; size_t SystemsShuttingDown = 0; size_t SystemsShutdown = 0; - std::string SystemsBadList {}; - std::string SystemsGoodList {}; - std::string SystemsStartingList {}; - std::string SystemsShuttingDownList {}; - std::string SystemsShutdownList {}; + std::string SystemsBadList { }; + std::string SystemsGoodList { }; + std::string SystemsStartingList { }; + std::string SystemsShuttingDownList { }; + std::string SystemsShutdownList { }; auto Statuses = Application::GetSubsystemStatuses(); for (const auto& NameStatusPair : Statuses) { switch (NameStatusPair.second) { @@ -847,7 +843,7 @@ void TConsole::InitializeCommandline() { if (!mLuaEngine) { beammp_info("Lua not started yet, please try again in a second"); } else { - std::string prefix {}; // stores non-table part of input + std::string prefix { }; // stores non-table part of input for (size_t i = stub.length(); i > 0; i--) { // separate table from input if (!std::isalnum(stub[i - 1]) && stub[i - 1] != '_' && stub[i - 1] != '.') { prefix = stub.substr(0, i); diff --git a/src/THeartbeatThread.cpp b/src/THeartbeatThread.cpp index 36cdde1..347f59a 100644 --- a/src/THeartbeatThread.cpp +++ b/src/THeartbeatThread.cpp @@ -181,8 +181,8 @@ 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() + ";"; + if (auto Client = ClientPtr.lock()) { + Return += Client->GetName() + ";"; } return true; }); diff --git a/src/TLuaEngine.cpp b/src/TLuaEngine.cpp index d1d8667..7fdd91a 100644 --- a/src/TLuaEngine.cpp +++ b/src/TLuaEngine.cpp @@ -120,7 +120,7 @@ void TLuaEngine::operator()() { std::unique_lock StateLock(mLuaStatesMutex); std::unique_lock Lock2(mResultsToCheckMutex); for (auto& Handler : Handlers) { - auto Res = mLuaStates[Timer.StateId]->EnqueueFunctionCallFromCustomEvent(Handler, {}, Timer.EventName, Timer.Strategy); + auto Res = mLuaStates[Timer.StateId]->EnqueueFunctionCallFromCustomEvent(Handler, { }, Timer.EventName, Timer.Strategy); if (Res) { mResultsToCheck.push_back(Res); mResultsToCheckCond.notify_one(); @@ -268,7 +268,7 @@ std::vector TLuaEngine::StateThreadData::GetStateTableKeys(const st auto globals = mStateView.globals(); sol::table current = globals; - std::vector Result {}; + std::vector Result { }; for (const auto& [key, value] : current) { std::string s = key.as(); @@ -471,7 +471,7 @@ std::vector TLuaEngine::StateThreadData::JsonStringToArray(JsonStri auto LocalTable = Lua_JsonDecode(Str.value).as>(); for (auto& value : LocalTable) { if (value.is() && value.as() == BEAMMP_INTERNAL_NIL) { - value = sol::object {}; + value = sol::object { }; } } return LocalTable; @@ -613,36 +613,37 @@ sol::table TLuaEngine::StateThreadData::Lua_TriggerLocalEvent(const std::string& sol::table TLuaEngine::StateThreadData::Lua_GetPlayerIdentifiers(int ID) { auto MaybeClient = GetClient(mEngine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - auto IDs = MaybeClient.value().lock()->GetIdentifiers(); - if (IDs.empty()) { - return sol::lua_nil; + if (MaybeClient) { + if (std::shared_ptr Locked = MaybeClient.value().lock()) { + auto IDs = Locked->GetIdentifiers(); + if (IDs.empty()) { + return sol::lua_nil; + } + sol::table Result = mStateView.create_table(); + for (const auto& Pair : IDs) { + Result.set(Pair.first, Pair.second); + } + return Result; } - sol::table Result = mStateView.create_table(); - for (const auto& Pair : IDs) { - Result.set(Pair.first, Pair.second); - } - return Result; - } else { - return sol::lua_nil; } + return sol::lua_nil; } std::variant TLuaEngine::StateThreadData::Lua_GetPlayerRole(int ID) { auto MaybeClient = GetClient(mEngine->Server(), ID); if (MaybeClient) { - return MaybeClient.value().lock()->GetRoles(); - } else { - return sol::nil; + if (auto Locked = MaybeClient.value().lock()) { + return Locked->GetRoles(); + } } + return sol::nil; } sol::table TLuaEngine::StateThreadData::Lua_GetPlayers() { sol::table Result = mStateView.create_table(); mEngine->Server().ForEachClient([&](std::weak_ptr Client) -> bool { - if (!Client.expired()) { - auto locked = Client.lock(); - Result[locked->GetID()] = locked->GetName(); + if (auto Locked = Client.lock()) { + Result[Locked->GetID()] = Locked->GetName(); } return true; }); @@ -652,10 +653,9 @@ sol::table TLuaEngine::StateThreadData::Lua_GetPlayers() { int TLuaEngine::StateThreadData::Lua_GetPlayerIDByName(const std::string& Name) { int Id = -1; mEngine->mServer->ForEachClient([&Id, &Name](std::weak_ptr Client) -> bool { - if (!Client.expired()) { - auto locked = Client.lock(); - if (locked->GetName() == Name) { - Id = locked->GetID(); + if (auto Locked = Client.lock()) { + if (Locked->GetName() == Name) { + Id = Locked->GetID(); return false; } } @@ -692,60 +692,59 @@ sol::table TLuaEngine::StateThreadData::Lua_FS_ListDirectories(const std::string std::string TLuaEngine::StateThreadData::Lua_GetPlayerName(int ID) { auto MaybeClient = GetClient(mEngine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - return MaybeClient.value().lock()->GetName(); - } else { - return ""; + if (MaybeClient) { + if (auto Locked = MaybeClient.value().lock()) { + return Locked->GetName(); + } } + return ""; } sol::table TLuaEngine::StateThreadData::Lua_GetPlayerVehicles(int ID) { auto MaybeClient = GetClient(mEngine->Server(), ID); - if (MaybeClient && !MaybeClient.value().expired()) { - auto Client = MaybeClient.value().lock(); - TClient::TSetOfVehicleData VehicleData; - { // Vehicle Data Lock Scope - auto LockedData = Client->GetAllCars(); - VehicleData = *LockedData.VehicleData; - } // End Vehicle Data Lock Scope - if (VehicleData.empty()) { - return sol::lua_nil; + if (MaybeClient) { + if (auto Client = MaybeClient.value().lock()) { + TClient::TSetOfVehicleData VehicleData; + { // Vehicle Data Lock Scope + auto LockedData = Client->GetAllCars(); + VehicleData = *LockedData.VehicleData; + } // End Vehicle Data Lock Scope + if (VehicleData.empty()) { + return sol::lua_nil; + } + sol::state_view StateView(mState); + sol::table Result = StateView.create_table(); + for (const auto& v : VehicleData) { + Result[v.ID()] = v.DataAsPacket(Client->GetRoles(), Client->GetName(), Client->GetID()).substr(3); + } + return Result; } - sol::state_view StateView(mState); - sol::table Result = StateView.create_table(); - for (const auto& v : VehicleData) { - Result[v.ID()] = v.DataAsPacket(Client->GetRoles(), Client->GetName(), Client->GetID()).substr(3); - } - return Result; - } else - return sol::lua_nil; + } + return sol::lua_nil; } std::pair TLuaEngine::StateThreadData::Lua_GetPositionRaw(int PID, int VID) { std::pair Result; auto MaybeClient = GetClient(mEngine->Server(), PID); - if (MaybeClient && !MaybeClient.value().expired()) { - auto Client = MaybeClient.value().lock(); - std::string VehiclePos = Client->GetCarPositionRaw(VID); + if (MaybeClient) { + if (auto Client = MaybeClient.value().lock()) { + std::string VehiclePos = Client->GetCarPositionRaw(VID); - if (VehiclePos.empty()) { - // return std::make_tuple(sol::lua_nil, sol::make_object(StateView, "Vehicle not found")); - Result.second = "Vehicle not found"; + if (VehiclePos.empty()) { + Result.second = "Vehicle not found"; + return Result; + } + + sol::table t = Lua_JsonDecode(VehiclePos); + if (t == sol::lua_nil) { + Result.second = "Packet decode failed"; + } + Result.first = t; return Result; } - - sol::table t = Lua_JsonDecode(VehiclePos); - if (t == sol::lua_nil) { - Result.second = "Packet decode failed"; - } - // return std::make_tuple(Result, sol::make_object(StateView, sol::lua_nil)); - Result.first = t; - return Result; - } else { - // return std::make_tuple(sol::lua_nil, sol::make_object(StateView, "Client expired")); - Result.second = "No such player"; - return Result; } + Result.second = "No such player"; + return Result; } sol::table TLuaEngine::StateThreadData::Lua_HttpCreateConnection(const std::string& host, uint16_t port) { @@ -786,22 +785,31 @@ static bool mDisableMPSet = [] { static auto GetSettingName = [](int id) -> const char* { switch (id) { - case 0: return "Debug"; - case 1: return "Private"; - case 2: return "MaxCars"; - case 3: return "MaxPlayers"; - case 4: return "Map"; - case 5: return "Name"; - case 6: return "Description"; - case 7: return "InformationPacket"; - default: return "Unknown"; + case 0: + return "Debug"; + case 1: + return "Private"; + case 2: + return "MaxCars"; + case 3: + return "MaxPlayers"; + case 4: + return "Map"; + case 5: + return "Name"; + case 6: + return "Description"; + case 7: + return "InformationPacket"; + default: + return "Unknown"; } }; static void JsonDecodeRecursive(sol::state_view& StateView, sol::table& table, const std::string& left, const nlohmann::json& right) { switch (right.type()) { case nlohmann::detail::value_t::null: - AddToTable(table, left, sol::lua_nil_t {}); + AddToTable(table, left, sol::lua_nil_t { }); return; case nlohmann::detail::value_t::object: { auto value = table.create(); @@ -949,12 +957,9 @@ TLuaEngine::StateThreadData::StateThreadData(const std::string& Name, TLuaStateI beammp_lua_error("SendNotification expects 2, 3 or 4 arguments."); } }); - MPTable.set_function("ConfirmationDialog", sol::overload( - &LuaAPI::MP::ConfirmationDialog, - [&](const int& ID, const std::string& Title, const std::string& Body, const sol::table& Buttons, const std::string& InteractionID) { - LuaAPI::MP::ConfirmationDialog(ID, Title, Body, Buttons, InteractionID); - } - )); + MPTable.set_function("ConfirmationDialog", sol::overload(&LuaAPI::MP::ConfirmationDialog, [&](const int& ID, const std::string& Title, const std::string& Body, const sol::table& Buttons, const std::string& InteractionID) { + LuaAPI::MP::ConfirmationDialog(ID, Title, Body, Buttons, InteractionID); + })); MPTable.set_function("GetPlayers", [&]() -> sol::table { return Lua_GetPlayers(); }); diff --git a/src/TNetwork.cpp b/src/TNetwork.cpp index 764584c..c7aff28 100644 --- a/src/TNetwork.cpp +++ b/src/TNetwork.cpp @@ -101,8 +101,8 @@ TNetwork::TNetwork(TServer& Server, TPPSMonitor& PPSMonitor, TResourceManager& R Application::RegisterShutdownHandler([&] { beammp_debug("Kicking all players due to shutdown"); Server.ForEachClient([&](std::weak_ptr client) -> bool { - if (!client.expired()) { - ClientKick(*client.lock(), "Server shutdown"); + if (auto Locked = client.lock()) { + ClientKick(*Locked, "Server shutdown"); } return true; }); @@ -181,8 +181,8 @@ void TNetwork::UDPServerMain() { std::shared_ptr Client; { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - Client = ClientPtr.lock(); + if (auto Locked = ClientPtr.lock()) { + Client = std::move(Locked); } else return true; } @@ -489,8 +489,8 @@ std::shared_ptr TNetwork::Authentication(TConnection&& RawConnection) { std::shared_ptr Cl; { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - Cl = ClientPtr.lock(); + if (auto Locked = ClientPtr.lock()) { + Cl = std::move(Locked); } else return true; } @@ -712,8 +712,11 @@ void TNetwork::DisconnectClient(TClient& c, const std::string& R) { void TNetwork::Looper(const std::weak_ptr& c) { RegisterThreadAuto(); - while (!c.expired()) { + while (true) { auto Client = c.lock(); + if (!Client) { + break; + } if (Client->IsDisconnected()) { beammp_debug("client is disconnected, breaking client loop"); break; @@ -747,20 +750,29 @@ void TNetwork::Looper(const std::weak_ptr& c) { } void TNetwork::TCPClient(const std::weak_ptr& c) { - // TODO: the c.expired() might cause issues here, remove if you end up here with your debugger - if (c.expired() || c.lock()->IsDisconnected()) { + if (auto Client = c.lock()) { + if (Client->IsDisconnected()) { + mServer.RemoveClient(c); + return; + } + } else { mServer.RemoveClient(c); return; } OnConnect(c); - RegisterThread("(" + std::to_string(c.lock()->GetID()) + ") \"" + c.lock()->GetName() + "\""); + if (auto Client = c.lock()) { + RegisterThread("(" + std::to_string(Client->GetID()) + ") \"" + Client->GetName() + "\""); + } else { + return; + } std::jthread QueueSync(&TNetwork::Looper, this, c); while (true) { - if (c.expired()) - break; auto Client = c.lock(); + if (!Client) { + break; + } if (Client->IsDisconnected()) { beammp_debug("client status < 0, breaking client loop"); break; @@ -781,8 +793,7 @@ void TNetwork::TCPClient(const std::weak_ptr& c) { } } - if (!c.expired()) { - auto Client = c.lock(); + if (auto Client = c.lock()) { OnDisconnect(c); } else { beammp_warn("client expired in TCPClient, should never happen"); @@ -793,8 +804,7 @@ void TNetwork::UpdatePlayer(TClient& Client) { std::string Packet = ("Ss") + std::to_string(mServer.ClientCount()) + "/" + std::to_string(Application::Settings.getAsInt(Settings::Key::General_MaxPlayers)) + ":"; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - auto c = ClientPtr.lock(); + if (auto c = ClientPtr.lock()) { Packet += c->GetName() + ","; } return true; @@ -937,14 +947,11 @@ TEST_CASE("ReadSocketWithTimeout can timeout then retry successfully") { } void TNetwork::OnDisconnect(const std::weak_ptr& ClientPtr) { - std::shared_ptr LockedClientPtr { nullptr }; - try { - LockedClientPtr = ClientPtr.lock(); - } catch (const std::exception&) { + auto LockedClientPtr = ClientPtr.lock(); + if (!LockedClientPtr) { beammp_warn("Client expired in OnDisconnect, this is unexpected"); return; } - beammp_assert(LockedClientPtr != nullptr); TClient& c = *LockedClientPtr; beammp_info(c.GetName() + (" Connection Terminated")); std::string Packet; @@ -975,8 +982,7 @@ int TNetwork::OpenID() { found = true; mServer.ForEachClient([&](const std::weak_ptr& ClientPtr) -> bool { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - auto c = ClientPtr.lock(); + if (auto c = ClientPtr.lock()) { if (c->GetID() == ID) { found = false; ID++; @@ -989,9 +995,11 @@ int TNetwork::OpenID() { } void TNetwork::OnConnect(const std::weak_ptr& c) { - beammp_assert(!c.expired()); - beammp_info("Client connected"); auto LockedClient = c.lock(); + if (!LockedClient) { + return; + } + beammp_info("Client connected"); LockedClient->SetID(OpenID()); beammp_info("Assigned ID " + std::to_string(LockedClient->GetID()) + " to " + LockedClient->GetName()); LuaAPI::MP::Engine->ReportErrors(LuaAPI::MP::Engine->TriggerEvent("onPlayerConnecting", "", LockedClient->GetID())); @@ -1218,10 +1226,10 @@ bool TNetwork::Respond(TClient& c, const std::vector& MSG, bool Rel, bo } bool TNetwork::SyncClient(const std::weak_ptr& c) { - if (c.expired()) { + auto LockedClient = c.lock(); + if (!LockedClient) { return false; } - auto LockedClient = c.lock(); if (LockedClient->IsSynced()) return true; // Syncing, later set isSynced @@ -1240,8 +1248,8 @@ bool TNetwork::SyncClient(const std::weak_ptr& c) { std::shared_ptr client; { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - client = ClientPtr.lock(); + if (auto Locked = ClientPtr.lock()) { + client = std::move(Locked); } else return true; } @@ -1278,14 +1286,14 @@ void TNetwork::SendToAll(TClient* c, const std::vector& Data, bool Self char C = Data.at(0); bool ret = true; mServer.ForEachClient([&](std::weak_ptr ClientPtr) -> bool { - std::shared_ptr Client; - try { + std::shared_ptr Client { nullptr }; + { ReadLock Lock(mServer.GetClientMutex()); - Client = ClientPtr.lock(); - } catch (const std::exception&) { - // continue - beammp_warn("Client expired, shouldn't happen - if a client disconnected recently, you can ignore this"); - return true; + if (auto Locked = ClientPtr.lock()) { + Client = std::move(Locked); + } else { + return true; + } } if (Self || Client.get() != c) { if (Client->IsSynced() || Client->IsSyncing()) { diff --git a/src/TPPSMonitor.cpp b/src/TPPSMonitor.cpp index 7fe5c6f..859871e 100644 --- a/src/TPPSMonitor.cpp +++ b/src/TPPSMonitor.cpp @@ -55,8 +55,8 @@ void TPPSMonitor::operator()() { std::shared_ptr c; { ReadLock Lock(mServer.GetClientMutex()); - if (!ClientPtr.expired()) { - c = ClientPtr.lock(); + if (auto Locked = ClientPtr.lock()) { + c = std::move(Locked); } else return true; } diff --git a/src/TServer.cpp b/src/TServer.cpp index 2c47542..f4b0a45 100644 --- a/src/TServer.cpp +++ b/src/TServer.cpp @@ -127,19 +127,15 @@ TServer::TServer(const std::vector& Arguments) { } void TServer::RemoveClient(const std::weak_ptr& WeakClientPtr) { - std::shared_ptr LockedClientPtr { nullptr }; - try { - LockedClientPtr = WeakClientPtr.lock(); - } catch (const std::exception&) { - // silently fail, as there's nothing to do + auto LockedClientPtr = WeakClientPtr.lock(); + if (!LockedClientPtr) { return; } - beammp_assert(LockedClientPtr != nullptr); TClient& Client = *LockedClientPtr; beammp_debug("removing client " + Client.GetName() + " (" + std::to_string(ClientCount()) + ")"); Client.ClearCars(); WriteLock Lock(mClientsMutex); - mClients.erase(WeakClientPtr.lock()); + mClients.erase(LockedClientPtr); } void TServer::ForEachClient(const std::function)>& Fn) { @@ -167,14 +163,16 @@ void TServer::GlobalParser(const std::weak_ptr& Client, std::vectorGetID()); - Network.ClientKick(*LockedClient, "Sent invalid compressed packet (this is likely a bug on your end)"); + if (auto LockedClient = Client.lock()) { + beammp_errorf("Failed to decompress packet from client {}. The client sent invalid data and will now be disconnected.", LockedClient->GetID()); + Network.ClientKick(*LockedClient, "Sent invalid compressed packet (this is likely a bug on your end)"); + } return; } catch (const std::runtime_error& e) { - auto LockedClient = Client.lock(); - beammp_errorf("Failed to decompress packet from client {}: {}. The server might be out of RAM! The client will now be disconnected.", LockedClient->GetID(), e.what()); - Network.ClientKick(*LockedClient, "Decompression failed (likely a server-side problem)"); + if (auto LockedClient = Client.lock()) { + beammp_errorf("Failed to decompress packet from client {}: {}. The server might be out of RAM! The client will now be disconnected.", LockedClient->GetID(), e.what()); + Network.ClientKick(*LockedClient, "Decompression failed (likely a server-side problem)"); + } return; } } @@ -182,10 +180,10 @@ void TServer::GlobalParser(const std::weak_ptr& Client, std::vector