From 532b4125d56fa563c80069833bc770189a21202f Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Sat, 27 Aug 2022 23:45:58 +0200 Subject: [PATCH] More thread safety improvements * Run 'CHostState::Think()' in the main thread. * Construct 'NetGameServer_t' objects in main thread before dispatching (TODO: browser). * Dispatch the call to 'CBanSystem::AddConnectionRefuse' from 'SV_IsClientBanned' to the main thread if we aren't main. * Return bool for CBanSystem::AddEntry and CBanSystem::DeleteEntry for future optimizations (next commit). --- r5dev/engine/host_state.cpp | 94 +++++++++++++++++------------ r5dev/engine/server/server.cpp | 3 +- r5dev/engine/server/server.h | 2 +- r5dev/engine/server/sv_main.cpp | 15 ++++- r5dev/engine/server/sv_main.h | 2 +- r5dev/gameui/IBrowser.cpp | 4 +- r5dev/networksystem/bansystem.cpp | 25 +++++--- r5dev/networksystem/bansystem.h | 4 +- r5dev/networksystem/listmanager.cpp | 18 ++++++ r5dev/networksystem/pylon.cpp | 37 ++---------- r5dev/networksystem/pylon.h | 2 +- r5dev/vstdlib/callback.cpp | 12 ++-- 12 files changed, 125 insertions(+), 93 deletions(-) diff --git a/r5dev/engine/host_state.cpp b/r5dev/engine/host_state.cpp index a64e3ed1..ca3df20d 100644 --- a/r5dev/engine/host_state.cpp +++ b/r5dev/engine/host_state.cpp @@ -42,10 +42,12 @@ #endif // DEDICATED #include "networksystem/pylon.h" #include "networksystem/bansystem.h" +#include "networksystem/listmanager.h" #include "public/edict.h" #ifndef CLIENT_DLL #include "game/server/gameinterface.h" #endif // !CLIENT_DLL +#include "squirrel/sqinit.h" //----------------------------------------------------------------------------- // Purpose: state machine's main processing loop @@ -58,6 +60,8 @@ FORCEINLINE void CHostState::FrameUpdate(CHostState* pHostState, double flCurren g_pHostState->Setup(); bInitialized = true; } + + g_pHostState->Think(); #ifdef DEDICATED RCONServer()->RunFrame(); #else // @@ -162,11 +166,11 @@ FORCEINLINE void CHostState::Init(void) m_iNextState = HostStates_t::HS_RUN; } } - m_flShortFrameTime = 1.0; + m_flShortFrameTime = 1.0f; m_levelName[0] = 0; m_landMarkName[0] = 0; m_mapGroupName[0] = 0; - m_bSplitScreenConnect = 256; + m_bSplitScreenConnect = 256; // Is this actually 'm_bSplitScreenConnect'? (assembly sets this value, other 3 bytes are padded which makes this operation valid still). m_vecLocation.Init(); m_angLocation.Init(); m_iCurrentState = HostStates_t::HS_NEW_GAME; @@ -180,9 +184,6 @@ FORCEINLINE void CHostState::Setup(void) g_pHostState->LoadConfig(); g_pConVar->PurgeHostNames(); - std::thread think(&CHostState::Think, this); - think.detach(); - net_usesocketsforloopback->SetValue(1); if (net_useRandomKey->GetBool()) { @@ -201,46 +202,61 @@ FORCEINLINE void CHostState::Think(void) const static CFastTimer pylonTimer; static CFastTimer statsTimer; - for (;;) // Loop running at 20-tps. + if (!bInitialized) // Initialize clocks. { - if (!bInitialized) // Initialize clocks. - { #ifndef CLIENT_DLL - banListTimer.Start(); + banListTimer.Start(); #ifdef DEDICATED - pylonTimer.Start(); + pylonTimer.Start(); #endif // DEDICATED - statsTimer.Start(); -#endif // !CLIENT_DLL - bInitialized = true; - } -#ifndef CLIENT_DLL - if (banListTimer.GetDurationInProgress().GetSeconds() > sv_banlistRefreshInterval->GetDouble()) - { - g_pBanSystem->BanListCheck(); - banListTimer.Start(); - } -#endif // !CLIENT_DLL -#ifdef DEDICATED - if (pylonTimer.GetDurationInProgress().GetSeconds() > sv_pylonRefreshInterval->GetDouble()) - { - KeepAliveToPylon(); - pylonTimer.Start(); - } -#endif // DEDICATED -#ifndef CLIENT_DLL - if (statsTimer.GetDurationInProgress().GetSeconds() > sv_statusRefreshInterval->GetDouble()) - { - string svCurrentPlaylist = KeyValues_GetCurrentPlaylist(); - int32_t nPlayerCount = g_pServer->GetNumHumanPlayers(); - - SetConsoleTitleA(fmt::format("{:s} - {:d}/{:d} Players ({:s} on {:s})", - hostname->GetString(), nPlayerCount, g_ServerGlobalVariables->m_nMaxClients, svCurrentPlaylist, m_levelName).c_str()); - statsTimer.Start(); - } - std::this_thread::sleep_for(std::chrono::milliseconds(50)); + statsTimer.Start(); #endif // !CLIENT_DLL + bInitialized = true; } +#ifndef CLIENT_DLL + if (banListTimer.GetDurationInProgress().GetSeconds() > sv_banlistRefreshInterval->GetDouble()) + { + g_pBanSystem->BanListCheck(); + banListTimer.Start(); + } +#endif // !CLIENT_DLL +#ifdef DEDICATED + if (pylonTimer.GetDurationInProgress().GetSeconds() > sv_pylonRefreshInterval->GetDouble()) + { + const NetGameServer_t netGameServer + { + hostname->GetString(), + hostdesc->GetString(), + sv_pylonVisibility->GetInt() == EServerVisibility_t::HIDDEN, + g_pHostState->m_levelName, + mp_gamemode->GetString(), + hostip->GetString(), + hostport->GetString(), + g_svNetKey, + std::to_string(*g_nServerRemoteChecksum), + SDK_VERSION, + std::to_string(g_pServer->GetNumHumanPlayers() + g_pServer->GetNumFakeClients()), + std::to_string(g_ServerGlobalVariables->m_nMaxClients), + std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() + ).count() + }; + + std::thread(KeepAliveToPylon, netGameServer).detach(); + pylonTimer.Start(); + } +#endif // DEDICATED +#ifndef CLIENT_DLL + if (statsTimer.GetDurationInProgress().GetSeconds() > sv_statusRefreshInterval->GetDouble()) + { + string svCurrentPlaylist = KeyValues_GetCurrentPlaylist(); + int32_t nPlayerCount = g_pServer->GetNumHumanPlayers(); + + SetConsoleTitleA(fmt::format("{:s} - {:d}/{:d} Players ({:s} on {:s})", + hostname->GetString(), nPlayerCount, g_ServerGlobalVariables->m_nMaxClients, svCurrentPlaylist, m_levelName).c_str()); + statsTimer.Start(); + } +#endif // !CLIENT_DLL } //----------------------------------------------------------------------------- diff --git a/r5dev/engine/server/server.cpp b/r5dev/engine/server/server.cpp index 0f34a423..45c78a30 100644 --- a/r5dev/engine/server/server.cpp +++ b/r5dev/engine/server/server.cpp @@ -71,7 +71,7 @@ CClient* CServer::Authenticate(CServer* pServer, user_creds_s* pInpacket) DevMsg(eDLL_T::SERVER, "\n"); DevMsg(eDLL_T::SERVER, "______________________________________________________________\n"); DevMsg(eDLL_T::SERVER, "] AUTHENTICATION ---------------------------------------------\n"); - DevMsg(eDLL_T::SERVER, "] UID : | '%s'\n", pInpacket->m_nUserID); + DevMsg(eDLL_T::SERVER, "] UID : | '%s'\n", pInpacket->m_pUserID); DevMsg(eDLL_T::SERVER, "] OID : | '%llu'\n", pInpacket->m_nNucleusID); DevMsg(eDLL_T::SERVER, "] ADR : | '%s'\n", svIpAddress.c_str()); DevMsg(eDLL_T::SERVER, "--------------------------------------------------------------\n"); @@ -82,7 +82,6 @@ CClient* CServer::Authenticate(CServer* pServer, user_creds_s* pInpacket) if (g_pBanSystem->IsBanned(svIpAddress, pInpacket->m_nNucleusID)) // Is the client trying to connect banned? { v_CServer_RejectConnection(pServer, pServer->m_Socket, pInpacket, "You have been banned from this server."); // RejectConnection for the client. - if (sv_showconnecting->GetBool()) { Warning(eDLL_T::SERVER, "Connection rejected for '%s' ('%llu' is banned from this server!)\n", svIpAddress.c_str(), pInpacket->m_nNucleusID); diff --git a/r5dev/engine/server/server.h b/r5dev/engine/server/server.h index fa86438d..ce5622fa 100644 --- a/r5dev/engine/server/server.h +++ b/r5dev/engine/server/server.h @@ -23,7 +23,7 @@ struct user_creds_s int32_t m_nchallenge; uint8_t gap2[8]; uint64_t m_nNucleusID; - uint8_t* m_nUserID; + uint8_t* m_pUserID; }; class CServer : public IServer diff --git a/r5dev/engine/server/sv_main.cpp b/r5dev/engine/server/sv_main.cpp index b91033be..d241535d 100644 --- a/r5dev/engine/server/sv_main.cpp +++ b/r5dev/engine/server/sv_main.cpp @@ -1,4 +1,6 @@ #include "core/stdafx.h" +#include "tier0/threadtools.h" +#include "tier0/frametask.h" #include "engine/server/sv_main.h" #include "networksystem/pylon.h" #include "networksystem/bansystem.h" @@ -6,15 +8,22 @@ //----------------------------------------------------------------------------- // Purpose: checks if particular client is banned on the comp server //----------------------------------------------------------------------------- -void SV_IsClientBanned(CPylon* pPylon, const string svIPAddr, uint64_t nNucleusID) +void SV_IsClientBanned(CPylon* pPylon, const string& svIPAddr, const uint64_t nNucleusID) { - string svError = string(); + string svError; bool bCompBanned = pPylon->GetClientIsBanned(svIPAddr, nNucleusID, svError); if (bCompBanned) { DevMsg(eDLL_T::SERVER, "Connection rejected for '%s' ('%llu' is banned from the master server!)\n", svIPAddr.c_str(), nNucleusID); - g_pBanSystem->AddConnectionRefuse(svError, nNucleusID); // Add to the vector. + + if (!ThreadInMainThread()) + { + g_TaskScheduler->Dispatch([svError, nNucleusID] + { + g_pBanSystem->AddConnectionRefuse(svError, nNucleusID); // Add to the vector. + }, 0); + } } } diff --git a/r5dev/engine/server/sv_main.h b/r5dev/engine/server/sv_main.h index 8dc7e369..9df3c5ba 100644 --- a/r5dev/engine/server/sv_main.h +++ b/r5dev/engine/server/sv_main.h @@ -21,7 +21,7 @@ inline bool* s_bDedicated = nullptr; /////////////////////////////////////////////////////////////////////////////// -void SV_IsClientBanned(CPylon* pPylon, const string svIPAddr, uint64_t nNucleusID); +void SV_IsClientBanned(CPylon* pPylon, const string& svIPAddr, const uint64_t nNucleusID); /////////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////// diff --git a/r5dev/gameui/IBrowser.cpp b/r5dev/gameui/IBrowser.cpp index 49796df2..0f2b7f4c 100644 --- a/r5dev/gameui/IBrowser.cpp +++ b/r5dev/gameui/IBrowser.cpp @@ -592,7 +592,7 @@ void CBrowser::UpdateHostingStatus(void) break; } - NetGameServer_t gameServer // !FIXME: create from main thread. + NetGameServer_t netGameServer // !FIXME: create from main thread. { g_pServerListManager->m_Server.m_svHostName, g_pServerListManager->m_Server.m_svDescription, @@ -611,7 +611,7 @@ void CBrowser::UpdateHostingStatus(void) ).count() }; - std::thread post(&CBrowser::SendHostingPostRequest, this, gameServer); + std::thread post(&CBrowser::SendHostingPostRequest, this, netGameServer); post.detach(); break; diff --git a/r5dev/networksystem/bansystem.cpp b/r5dev/networksystem/bansystem.cpp index edb88b74..48167b1d 100644 --- a/r5dev/networksystem/bansystem.cpp +++ b/r5dev/networksystem/bansystem.cpp @@ -32,7 +32,7 @@ void CBanSystem::operator[](std::pair pair) //----------------------------------------------------------------------------- void CBanSystem::Load(void) { - fs::path path = std::filesystem::current_path() /= "platform\\banlist.json"; + fs::path path = std::filesystem::current_path() /= "platform\\banlist.json"; // !TODO: Use FS "PLATFORM" nlohmann::json jsIn; ifstream banFile(path, std::ios::in); @@ -82,7 +82,7 @@ void CBanSystem::Save(void) const jsOut[std::to_string(i)]["originID"] = m_vBanList[i].second; } - fs::path path = std::filesystem::current_path() /= "platform\\banlist.json"; + fs::path path = std::filesystem::current_path() /= "platform\\banlist.json"; // !TODO: Use FS "PLATFORM". ofstream outFile(path, std::ios::out | std::ios::trunc); // Write config file.. outFile << jsOut.dump(4); @@ -93,7 +93,7 @@ void CBanSystem::Save(void) const // Input : &svIpAddress - // nOriginID - //----------------------------------------------------------------------------- -void CBanSystem::AddEntry(const string& svIpAddress, const uint64_t nOriginID) +bool CBanSystem::AddEntry(const string& svIpAddress, const uint64_t nOriginID) { if (!svIpAddress.empty()) { @@ -101,8 +101,10 @@ void CBanSystem::AddEntry(const string& svIpAddress, const uint64_t nOriginID) if (it == m_vBanList.end()) { m_vBanList.push_back(std::make_pair(svIpAddress, nOriginID)); + return true; } } + return false; } //----------------------------------------------------------------------------- @@ -110,15 +112,19 @@ void CBanSystem::AddEntry(const string& svIpAddress, const uint64_t nOriginID) // Input : &svIpAddress - // nOriginID - //----------------------------------------------------------------------------- -void CBanSystem::DeleteEntry(const string& svIpAddress, const uint64_t nOriginID) +bool CBanSystem::DeleteEntry(const string& svIpAddress, const uint64_t nOriginID) { + bool result = false; for (size_t i = 0; i < m_vBanList.size(); i++) { if (svIpAddress.compare(m_vBanList[i].first) == NULL || nOriginID == m_vBanList[i].second) { m_vBanList.erase(m_vBanList.begin() + i); + result = true; } } + + return result; } //----------------------------------------------------------------------------- @@ -166,11 +172,12 @@ void CBanSystem::BanListCheck(void) { if (IsRefuseListValid()) { + bool bSave = false; for (size_t i = 0; i < m_vRefuseList.size(); i++) { for (int c = 0; c < MAX_PLAYERS; c++) // Loop through all possible client instances. { - CClient* pClient = g_pClient->GetClient(i); + CClient* pClient = g_pClient->GetClient(c); if (!pClient) continue; @@ -184,11 +191,15 @@ void CBanSystem::BanListCheck(void) string svIpAddress = pNetChan->GetAddress(); Warning(eDLL_T::SERVER, "Connection rejected for '%s' ('%llu' is banned from this server!)\n", svIpAddress.c_str(), pClient->GetOriginID()); - AddEntry(svIpAddress, pClient->GetOriginID()); - Save(); // Save banlist to file. NET_DisconnectClient(pClient, c, m_vRefuseList[i].first.c_str(), 0, true); + + if (AddEntry(svIpAddress, pClient->GetOriginID() && !bSave)) + bSave = true; } } + + if (bSave) + Save(); // Save banlist to file. } } diff --git a/r5dev/networksystem/bansystem.h b/r5dev/networksystem/bansystem.h index 8f981775..0e31c324 100644 --- a/r5dev/networksystem/bansystem.h +++ b/r5dev/networksystem/bansystem.h @@ -9,8 +9,8 @@ public: void Load(void); void Save(void) const; - void AddEntry(const string& svIpAddress, const uint64_t nOriginID); - void DeleteEntry(const string& svIpAddress, const uint64_t nOriginID); + bool AddEntry(const string& svIpAddress, const uint64_t nOriginID); + bool DeleteEntry(const string& svIpAddress, const uint64_t nOriginID); void AddConnectionRefuse(const string& svError, const uint64_t nOriginID); void DeleteConnectionRefuse(const uint64_t nOriginID); diff --git a/r5dev/networksystem/listmanager.cpp b/r5dev/networksystem/listmanager.cpp index bf9db4aa..b7c7c6e6 100644 --- a/r5dev/networksystem/listmanager.cpp +++ b/r5dev/networksystem/listmanager.cpp @@ -96,6 +96,15 @@ void CServerListManager::LaunchServer(void) const //----------------------------------------------------------------------------- void CServerListManager::ConnectToServer(const string& svIp, const string& svPort, const string& svNetKey) const { + if (!ThreadInMainThread()) + { + g_TaskScheduler->Dispatch([this, svIp, svPort, svNetKey]() + { + this->ConnectToServer(svIp, svPort, svNetKey); + }, 0); + return; + } + if (!svNetKey.empty()) { NET_SetKey(svNetKey); @@ -110,6 +119,15 @@ void CServerListManager::ConnectToServer(const string& svIp, const string& svPor //----------------------------------------------------------------------------- void CServerListManager::ConnectToServer(const string& svServer, const string& svNetKey) const { + if (!ThreadInMainThread()) + { + g_TaskScheduler->Dispatch([this, svServer, svNetKey]() + { + this->ConnectToServer(svServer, svNetKey); + }, 0); + return; + } + if (!svNetKey.empty()) { NET_SetKey(svNetKey); diff --git a/r5dev/networksystem/pylon.cpp b/r5dev/networksystem/pylon.cpp index 08b47f97..c3d73e06 100644 --- a/r5dev/networksystem/pylon.cpp +++ b/r5dev/networksystem/pylon.cpp @@ -7,50 +7,25 @@ #include #include -#include #include -#include -#ifndef CLIENT_DLL -#include -#endif -#include #include -#include -#include //----------------------------------------------------------------------------- // Purpose: Send keep alive request to Pylon Master Server. // NOTE: When Pylon update reaches indev remove this and implement properly. //----------------------------------------------------------------------------- -void KeepAliveToPylon() +bool KeepAliveToPylon(const NetGameServer_t& netGameServer) { #ifndef CLIENT_DLL if (g_pHostState->m_bActiveGame && sv_pylonVisibility->GetBool()) // Check for active game. { - std::string m_szHostToken = std::string(); - std::string m_szHostRequestMessage = std::string(); + string m_szHostToken; + string m_szHostRequestMessage; - bool result = g_pMasterServer->PostServerHost(m_szHostRequestMessage, m_szHostToken, - NetGameServer_t - { - hostname->GetString(), - hostdesc->GetString(), - sv_pylonVisibility->GetInt() == EServerVisibility_t::HIDDEN, - g_pHostState->m_levelName, - mp_gamemode->GetString(), - hostip->GetString(), - hostport->GetString(), - g_svNetKey, - std::to_string(*g_nServerRemoteChecksum), - SDK_VERSION, - std::to_string(g_pServer->GetNumHumanPlayers() + g_pServer->GetNumFakeClients()), - std::to_string(g_ServerGlobalVariables->m_nMaxClients), - std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch() - ).count() - } - ); + bool result = g_pMasterServer->PostServerHost(m_szHostRequestMessage, m_szHostToken, netGameServer); + return result; } + return false; #endif // !CLIENT_DLL } diff --git a/r5dev/networksystem/pylon.h b/r5dev/networksystem/pylon.h index c5b0b81d..36e28d5d 100644 --- a/r5dev/networksystem/pylon.h +++ b/r5dev/networksystem/pylon.h @@ -1,7 +1,7 @@ #pragma once #include "serverlisting.h" -void KeepAliveToPylon(); +bool KeepAliveToPylon(const NetGameServer_t& netGameServer); class CPylon { diff --git a/r5dev/vstdlib/callback.cpp b/r5dev/vstdlib/callback.cpp index 458d014f..27a804ec 100644 --- a/r5dev/vstdlib/callback.cpp +++ b/r5dev/vstdlib/callback.cpp @@ -307,13 +307,17 @@ void Host_Unban_f(const CCommand& args) { if (args.HasOnlyDigits(1)) // Check if we have an ip address or origin ID. { - g_pBanSystem->DeleteEntry("noIP", std::stoll(args.Arg(1))); // Delete ban entry. - g_pBanSystem->Save(); // Save modified vector to file. + if (g_pBanSystem->DeleteEntry("noIP", std::stoll(args.Arg(1)))) // Delete ban entry. + { + g_pBanSystem->Save(); // Save modified vector to file. + } } else { - g_pBanSystem->DeleteEntry(args.Arg(1), 0); // Delete ban entry. - g_pBanSystem->Save(); // Save modified vector to file. + if (g_pBanSystem->DeleteEntry(args.Arg(1), 0)) // Delete ban entry. + { + g_pBanSystem->Save(); // Save modified vector to file. + } } } catch (std::exception& e)