From 8571f0c9f4381c24f01c6210794d8366d32ed04b Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Tue, 27 Dec 2016 00:00:02 -0600 Subject: [PATCH 01/13] remove my currently unused code for blacklisting people as iptables seems to suffice --- .gitignore | 1 + .../3rd/library/udplibrary/UdpLibrary.cpp | 30 ------------------- .../3rd/library/udplibrary/UdpLibrary.hpp | 21 ------------- 3 files changed, 1 insertion(+), 51 deletions(-) diff --git a/.gitignore b/.gitignore index b112824a..6bf76060 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +cmake-build* *.geany PVS* *.cfg diff --git a/external/3rd/library/udplibrary/UdpLibrary.cpp b/external/3rd/library/udplibrary/UdpLibrary.cpp index f8e7c870..8d41eb99 100755 --- a/external/3rd/library/udplibrary/UdpLibrary.cpp +++ b/external/3rd/library/udplibrary/UdpLibrary.cpp @@ -157,13 +157,6 @@ char *UdpIpAddress::GetAddress(char *buffer) const return(buffer); } -char *UdpIpAddress::GetV4Address() const -{ - struct sockaddr_in addr_serverUDP; - addr_serverUDP.sin_addr.s_addr = mIp; - return inet_ntoa(addr_serverUDP.sin_addr); -} - ///////////////////////////////////////////////////////////////////////////////////////////////////// // UdpManager::Params initializations constructor (ie. default values) ///////////////////////////////////////////////////////////////////////////////////////////////////// @@ -1082,7 +1075,6 @@ void UdpManager::ProcessRawPacket(const PacketHistoryEntry *e) } } - // got a packet from somebody and we don't know who they are and the packet we got was not a connection request // just in case they are a previous client who thinks they are still connected, we will send them an internal // packet telling them that we don't know who they are @@ -1110,28 +1102,6 @@ void UdpManager::ProcessRawPacket(const PacketHistoryEntry *e) con->Release(); } -bool UdpManager::isBlacklisted(unsigned int clientAddr) -{ - return false; -} - -void UdpManager::disconnectByIp(unsigned int clientAddr) -{ - while (mConnectionList != nullptr) - { - if (mConnectionList->mIp.GetAddress() == clientAddr) - { - mConnectionList->SetSilentDisconnect(true); - mConnectionList->InternalDisconnect(0, UdpConnection::cDisconnectReasonDosAttack); - - } - } -} - -void UdpManager::addStrike(UdpIpAddress clientIp, int type) -{ -} - UdpConnection *UdpManager::AddressGetConnection(UdpIpAddress ip, int port) const { UdpConnection *found = static_cast(mAddressHashTable->FindFirst(AddressHashValue(ip, port))); diff --git a/external/3rd/library/udplibrary/UdpLibrary.hpp b/external/3rd/library/udplibrary/UdpLibrary.hpp index 52922dab..3117e386 100644 --- a/external/3rd/library/udplibrary/UdpLibrary.hpp +++ b/external/3rd/library/udplibrary/UdpLibrary.hpp @@ -2,7 +2,6 @@ #define UDPLIBRARY_HPP #include -#include #include "UdpHandler.hpp" #include "priority.hpp" @@ -101,7 +100,6 @@ class UdpIpAddress public: UdpIpAddress(unsigned int ip = 0); unsigned int GetAddress() const { return(mIp); } - char *GetV4Address() const; char *GetAddress(char *buffer) const; bool operator==(const UdpIpAddress& e) const { return(mIp == e.mIp); } protected: @@ -926,9 +924,6 @@ class UdpManager // to explicitly call this function. LogicalPacket *CreatePacket(const void *data, int dataLen, const void *data2 = nullptr, int dataLen2 = 0); - // is the given unsigned int expressed ip blacklisted? - bool isBlacklisted(unsigned int); - protected: friend class PooledLogicalPacket; void PoolReturn(PooledLogicalPacket *packet); // so pooled packets can add themselves back to the pool @@ -1045,22 +1040,6 @@ class UdpManager // typically it is recommended that all UdpConnection objects be destroyed before destroying this manager object int mRefCount; - - // number of strikes - static const int strikeOut = 3; - - // actual count of connections for a given ip - std::unordered_map mIpConnectionCount; - - - // count of strikes against a given ip - 3 successive DoS attempts and they are banned til next restart at best - std::unordered_map blacklist; - - // does what it says - void disconnectByIp (unsigned int); - - // add a strike - void addStrike(UdpIpAddress clientIp, int type); }; //////////////////////////////////////////////////////////////////////////////////////////////////////////// From 9cf6378076b9a129debe7da5972c565496f14b7d Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Tue, 27 Dec 2016 09:27:38 +0000 Subject: [PATCH 02/13] well, that would do it --- external/3rd/library/udplibrary/UdpLibrary.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/external/3rd/library/udplibrary/UdpLibrary.cpp b/external/3rd/library/udplibrary/UdpLibrary.cpp index 8d41eb99..4105355f 100755 --- a/external/3rd/library/udplibrary/UdpLibrary.cpp +++ b/external/3rd/library/udplibrary/UdpLibrary.cpp @@ -402,8 +402,6 @@ UdpManager::~UdpManager() TerminateOperatingSystem(); delete mAddressHashTable; - mIpConnectionCount.clear(); - blacklist.clear(); delete mConnectCodeHashTable; delete mPriorityQueue; @@ -563,14 +561,6 @@ void UdpManager::RemoveConnection(UdpConnection *con) mAddressHashTable->Remove(con, AddressHashValue(con->mIp, con->mPort)); unsigned int addy = con->mIp.GetAddress(); - if (mIpConnectionCount[addy] > 1) - { - mIpConnectionCount[addy]--; - } - else - { - mIpConnectionCount.erase(addy); - } mConnectCodeHashTable->Remove(con, con->mConnectCode); } @@ -587,7 +577,6 @@ void UdpManager::AddConnection(UdpConnection *con) mConnectionListCount++; mAddressHashTable->Insert(con, AddressHashValue(con->mIp, con->mPort)); - mIpConnectionCount[con->mIp.GetAddress()]++; mConnectCodeHashTable->Insert(con, con->mConnectCode); } From 058bcd9b3ded502a9f9514f125d561d1f4a697f2 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Tue, 27 Dec 2016 19:59:17 +0000 Subject: [PATCH 03/13] check for gm in ConsoleCommandParserScript --- .../src/shared/console/ConsoleCommandParserScript.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/engine/server/library/serverGame/src/shared/console/ConsoleCommandParserScript.cpp b/engine/server/library/serverGame/src/shared/console/ConsoleCommandParserScript.cpp index 99f1f236..d661a639 100755 --- a/engine/server/library/serverGame/src/shared/console/ConsoleCommandParserScript.cpp +++ b/engine/server/library/serverGame/src/shared/console/ConsoleCommandParserScript.cpp @@ -51,6 +51,17 @@ CommandParser ("script", 0, "...", "Script related commands.", 0) bool ConsoleCommandParserScript::performParsing (const NetworkId & userId, const StringVector_t & argv, const String_t & originalCommand, String_t & result, const CommandParser * node) { + CreatureObject * const playerObject = dynamic_cast(ServerWorld::findObjectByNetworkId(userId)); + if (!playerObject) + { + WARNING_STRICT_FATAL(true, ("Console command executed on invalid player object %s", userId.getValueString().c_str())); + return false; + } + + if (!playerObject->getClient()->isGod()) { + return false; // <3 you seefo + } + NOT_NULL (node); UNREF(originalCommand); From 9a187477f604ac84f68f0c6911fdec56c1623cfd Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Wed, 28 Dec 2016 19:01:59 +0000 Subject: [PATCH 04/13] clean up the login server output --- .../src/shared/ClientConnection.cpp | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/engine/server/application/LoginServer/src/shared/ClientConnection.cpp b/engine/server/application/LoginServer/src/shared/ClientConnection.cpp index e48e6463..54707546 100755 --- a/engine/server/application/LoginServer/src/shared/ClientConnection.cpp +++ b/engine/server/application/LoginServer/src/shared/ClientConnection.cpp @@ -169,8 +169,8 @@ void ClientConnection::validateClient(const std::string &id, const std::string & bool authOK = false; StationId suid = atoi(id.c_str()); static const std::string authURL(ConfigLoginServer::getExternalAuthUrl()); - std::string uname; + std::string uname; std::string parentAccount; std::vector childAccounts; @@ -194,7 +194,7 @@ void ClientConnection::validateClient(const std::string &id, const std::string & parentAccount = api.getString("mainAccount"); childAccounts = api.getStringVector("subAccounts"); } else { - std::string msg = api.getString("message"); + std::string msg(api.getString("message")); if (msg.empty()) { msg = "Invalid username or password."; } @@ -214,30 +214,44 @@ void ClientConnection::validateClient(const std::string &id, const std::string & if (authOK) { if (suid == 0) { - if (uname.length() > MAX_ACCOUNT_NAME_LENGTH) { - uname.resize(MAX_ACCOUNT_NAME_LENGTH); - } + uname.resize(MAX_ACCOUNT_NAME_LENGTH); - std::hash h; - suid = h(uname.c_str()); + std::hash hasher; + suid = hasher(uname); } - std::hash h; - StationId parent = h(parentAccount); + REPORT_LOG(true, ("Client connected. Username: %s (%lu) \n", uname.c_str(), suid)); - REPORT_LOG(true, - ("Client connected. Station Id: %llu, Username: %s, Parent %s\n", suid, uname.c_str(), parentAccount.c_str())); + StationId parent = -1; + + if (!parentAccount.empty()) { + parentAccount.resize(MAX_ACCOUNT_NAME_LENGTH); + + std::hash hasher; + parent = hasher(parentAccount); + + if (parentAccount != uname) { + REPORT_LOG(true, ("\t%s's parent is %s (%lu) \n", uname.c_str(), parentAccount.c_str(), parent)); + } + } else { + parentAccount = "(Empty Parent!) "+uname; + } for (auto i : childAccounts) { - if (i.length() > MAX_ACCOUNT_NAME_LENGTH) { - i.resize(MAX_ACCOUNT_NAME_LENGTH); - } + std::string child(i); - StationId childID = h(i); - REPORT_LOG(true, ("\tA child account for %s is %s (%llu)\n", parentAccount.c_str(), i.c_str(), childID)); + if (!child.empty()) { + std::hash hasher; + child.resize(MAX_ACCOUNT_NAME_LENGTH); - // insert all related accounts, if not already there, into the db - DatabaseConnection::getInstance().upsertAccountRelationship(parent, childID); + StationId childID = hasher(child); + REPORT_LOG(true, ("\tchild of %s (%lu) is %s (%lu) \n", parentAccount.c_str(), parent, child.c_str(), childID)); + + // insert all related accounts, if not already there, into the db + DatabaseConnection::getInstance().upsertAccountRelationship(parent, childID); + } else { + WARNING(true, ("Login API returned empty child account(s).")); + } } LOG("LoginClientConnection", From 2f85f221baed79a7e13b87b0a2bf6bbd506d4506 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Wed, 28 Dec 2016 21:56:24 +0000 Subject: [PATCH 05/13] this works even though it doesn't look right --- .../src/shared/ClientConnection.cpp | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/engine/server/application/LoginServer/src/shared/ClientConnection.cpp b/engine/server/application/LoginServer/src/shared/ClientConnection.cpp index 54707546..18c3ebb2 100755 --- a/engine/server/application/LoginServer/src/shared/ClientConnection.cpp +++ b/engine/server/application/LoginServer/src/shared/ClientConnection.cpp @@ -214,10 +214,11 @@ void ClientConnection::validateClient(const std::string &id, const std::string & if (authOK) { if (suid == 0) { - uname.resize(MAX_ACCOUNT_NAME_LENGTH); + if (uname.length() > MAX_ACCOUNT_NAME_LENGTH) + uname.resize(MAX_ACCOUNT_NAME_LENGTH); std::hash hasher; - suid = hasher(uname); + suid = hasher(uname.c_str()); } REPORT_LOG(true, ("Client connected. Username: %s (%lu) \n", uname.c_str(), suid)); @@ -225,10 +226,11 @@ void ClientConnection::validateClient(const std::string &id, const std::string & StationId parent = -1; if (!parentAccount.empty()) { - parentAccount.resize(MAX_ACCOUNT_NAME_LENGTH); + if (parentAccount.length() > MAX_ACCOUNT_NAME_LENGTH) + parentAccount.resize(MAX_ACCOUNT_NAME_LENGTH); std::hash hasher; - parent = hasher(parentAccount); + parent = hasher(parentAccount.c_str()); if (parentAccount != uname) { REPORT_LOG(true, ("\t%s's parent is %s (%lu) \n", uname.c_str(), parentAccount.c_str(), parent)); @@ -241,11 +243,13 @@ void ClientConnection::validateClient(const std::string &id, const std::string & std::string child(i); if (!child.empty()) { - std::hash hasher; - child.resize(MAX_ACCOUNT_NAME_LENGTH); + if (child.length() > MAX_ACCOUNT_NAME_LENGTH) + child.resize(MAX_ACCOUNT_NAME_LENGTH); - StationId childID = hasher(child); - REPORT_LOG(true, ("\tchild of %s (%lu) is %s (%lu) \n", parentAccount.c_str(), parent, child.c_str(), childID)); + std::hash hasher; + StationId childID = hasher(child.c_str()); + + REPORT_LOG(true, ("\tchild of %s (%lu) is %s (%lu) \n", parentAccount.c_str(), parent, child.c_str(), childID)); // insert all related accounts, if not already there, into the db DatabaseConnection::getInstance().upsertAccountRelationship(parent, childID); From b8280bad646d9a426ea1b6db38716ecc1ce0caa0 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Wed, 28 Dec 2016 22:37:02 +0000 Subject: [PATCH 06/13] Revert "It's 2016 and C++11 supports these natively" This reverts commit 3c0115921ee2448d0a6e952eb43860d971bfb7a7. --- .../sharedFoundationTypes/src/linux/FoundationTypesLinux.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/shared/library/sharedFoundationTypes/src/linux/FoundationTypesLinux.h b/engine/shared/library/sharedFoundationTypes/src/linux/FoundationTypesLinux.h index bd4bdc9d..386c4ffb 100755 --- a/engine/shared/library/sharedFoundationTypes/src/linux/FoundationTypesLinux.h +++ b/engine/shared/library/sharedFoundationTypes/src/linux/FoundationTypesLinux.h @@ -10,7 +10,6 @@ #define PLATFORM_LINUX #include -#include // ====================================================================== // basic types that we assume to be around @@ -21,8 +20,8 @@ typedef unsigned long uint32; typedef signed char int8; typedef signed short int16; typedef signed long int32; -typedef int64_t int64; -typedef uint64_t uint64; +typedef signed long long int int64; +typedef unsigned long long int uint64; typedef float real; typedef FILE* FILE_HANDLE; From af37f5b847b15d22b81c43425293b1d242c83ee7 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 04:28:37 +0000 Subject: [PATCH 07/13] get better warning/error output on character creation failures --- .../src/shared/CreateCharacterCustomPersistStep.cpp | 8 ++++++-- .../library/serverDatabase/src/shared/Persister.cpp | 7 ++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/CreateCharacterCustomPersistStep.cpp b/engine/server/library/serverDatabase/src/shared/CreateCharacterCustomPersistStep.cpp index fb6072b9..0ff64fc7 100755 --- a/engine/server/library/serverDatabase/src/shared/CreateCharacterCustomPersistStep.cpp +++ b/engine/server/library/serverDatabase/src/shared/CreateCharacterCustomPersistStep.cpp @@ -46,10 +46,14 @@ bool CreateCharacterCustomPersistStep::beforePersist(DB::Session *session) bool CreateCharacterCustomPersistStep::afterPersist(DB::Session *session) { -// std::string characterName = Unicode::wideToNarrow(DatabaseProcess::getInstance().getNameByStationId(m_stationId)); DBQuery::AddCharacter qry(m_stationId,m_characterObject, m_characterName, m_normalizedName); - if (! (session->exec(&qry))) + + if (! (session->exec(&qry))) { + std::string characterName(Unicode::wideToNarrow(m_characterName)); + WARNING(true, ("CreateCharacterCustomPersistStep: Failed saving character and character object for %s", characterName.c_str())); return false; + } + qry.done(); return true; } diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index fa57c58b..11fcd82c 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -993,9 +993,10 @@ void Persister::addCharacter(uint32 stationId, const NetworkId &characterObject, m_pendingCharacters[characterObject]=temp; //TODO: remove this hack: match up create and end messages because we can't count on having all the data at a frame bounday - auto i=m_newCharacterLock.find(creationGameServer); - UNREF(i); - DEBUG_FATAL(i!=m_newCharacterLock.end(),("Programmer bug: got an addCharacter from server %i before we received EndBaselines from the previous addCharacter. Indicates we're getting network messages out of order.\n",creationGameServer)); + if (m_newCharacterLock.find(creationGameServer) != m_newCharacterLock.end()) { + WARNING(true,("Programmer bug: got an addCharacter from server %i before we received EndBaselines from the previous addCharacter. Indicates we're getting network messages out of order.\n",creationGameServer)); + } + m_newCharacterLock.insert(creationGameServer); } From ef1b4d90d690f6df1ad71105ba242fecd29c1ff8 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 00:15:28 -0600 Subject: [PATCH 08/13] try a mutex to see if it keeps us from deleting our playerobjects --- .../serverDatabase/src/shared/Persister.cpp | 42 ++++++++++++++++++- .../serverDatabase/src/shared/Persister.h | 4 ++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index 11fcd82c..a28fb55e 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -5,8 +5,6 @@ // // ====================================================================== -#include - #include "serverDatabase/FirstServerDatabase.h" #include "serverDatabase/Persister.h" @@ -216,6 +214,8 @@ void Persister::update(real updateTime) void Persister::onFrameBarrierReached() { + pad.lock(); + if (m_newCharacterTaskQueue->getNumPendingTasks() == 0) { ServerSnapshotMap delayedSaves; @@ -281,6 +281,8 @@ void Persister::onFrameBarrierReached() taskQueue->report(); } } + + pad.unlock(); } // ---------------------------------------------------------------------- @@ -305,6 +307,8 @@ void Persister::startSave(void) m_newObjectCount=0; + pad.lock(); + // delete any characters for this save cycle if (m_charactersToDeleteNextSaveCycle && m_charactersToDeleteThisSaveCycle) { @@ -375,6 +379,8 @@ void Persister::startSave(void) if (ConfigServerDatabase::getReportSaveTimes()) m_saveStartTime = Clock::timeMs(); + + pad.unlock(); } // ---------------------------------------------------------------------- @@ -386,16 +392,24 @@ void Persister::startSave(void) */ Snapshot * Persister::getSnapshotForObject(const NetworkId &networkId, uint32 serverId) { + pad.lock(); + auto i = m_objectSnapshotMap.find(networkId); if (i!=m_objectSnapshotMap.end()) { NOT_NULL(i->second); + + pad.unlock(); + return i->second; } else { Snapshot *snap = getSnapshotForServer(serverId); m_objectSnapshotMap[networkId]=snap; + + pad.unlock(); + return snap; } } @@ -412,6 +426,8 @@ bool Persister::hasDataForObject(const NetworkId &objectId) const Snapshot *Persister::getSnapshotForServer(uint32 serverId) { + pad.lock(); + if (serverId==0) { if (!m_arbitraryGameDataSnapshot) @@ -419,6 +435,9 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) m_arbitraryGameDataSnapshot = makeSnapshot(DB::ModeQuery::mode_UPDATE); m_currentSnapshots[0] = m_arbitraryGameDataSnapshot; } + + pad.unlock(); + return m_arbitraryGameDataSnapshot; } else @@ -433,6 +452,8 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) if (!m_arbitraryGameDataSnapshot) { m_arbitraryGameDataSnapshot = snap; } + + pad.unlock(); return snap; @@ -440,6 +461,9 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) else { NOT_NULL (j->second); + + pad.unlock(); + return j->second; } } @@ -514,9 +538,12 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa { UNREF(serverId); + pad.lock(); + if (m_objectSnapshotMap.find(objectId)!=m_objectSnapshotMap.end()) { DEBUG_WARNING(true,("Database received multiple new object messages for object %s",objectId.getValueString().c_str())); + pad.unlock(); return; } @@ -571,6 +598,8 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa NOT_NULL(snap); snap->newObject(objectId, templateId, typeId); m_objectSnapshotMap[objectId]=snap; + + pad.unlock(); } // ---------------------------------------------------------------------- @@ -583,6 +612,9 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) { + + pad.lock(); + //TODO: This is a hack until we remove frame boundaries and have "end frame" messages from the game server. Apparently the game // server can split baselines across frame boundaries, so we can't assume we have all the data for a character when we hit a // frame bounday. @@ -592,6 +624,8 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) m_pendingCharacters.erase(chardata); m_newCharacterLock.erase(serverId); } + + pad.unlock(); } // ---------------------------------------------------------------------- @@ -602,6 +636,8 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) void Persister::saveCompleted(Snapshot *completedSnapshot) { + pad.lock(); + auto i=std::remove(m_savingSnapshots.begin(),m_savingSnapshots.end(),completedSnapshot); if (i!=m_savingSnapshots.end()) { @@ -657,6 +693,8 @@ void Persister::saveCompleted(Snapshot *completedSnapshot) DEBUG_REPORT_LOG(ConfigServerDatabase::getReportSaveTimes(),("New character save completed\n")); } + + pad.unlock(); } diff --git a/engine/server/library/serverDatabase/src/shared/Persister.h b/engine/server/library/serverDatabase/src/shared/Persister.h index 31b09608..6f6ba2d6 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.h +++ b/engine/server/library/serverDatabase/src/shared/Persister.h @@ -16,6 +16,8 @@ #include #include //TODO: remove when we clean up newCharacterLock hack +#include + #include "Unicode.h" #include "serverNetworkMessages/MessageToPayload.h" #include "sharedDatabaseInterface/DbModeQuery.h" @@ -97,6 +99,8 @@ class Persister : public MessageDispatch::Receiver DB::TaskQueue *m_newCharacterTaskQueue; private: + std::mutex pad; + struct PendingCharacter { uint32 stationId; From 318eb0353e97aa15a9d9a00bee2dff5faaceee09 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 07:16:48 +0000 Subject: [PATCH 09/13] here's hoping these are enough --- .../serverDatabase/src/shared/Persister.cpp | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index a28fb55e..e9d62df0 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -214,8 +214,6 @@ void Persister::update(real updateTime) void Persister::onFrameBarrierReached() { - pad.lock(); - if (m_newCharacterTaskQueue->getNumPendingTasks() == 0) { ServerSnapshotMap delayedSaves; @@ -281,8 +279,6 @@ void Persister::onFrameBarrierReached() taskQueue->report(); } } - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -392,24 +388,16 @@ void Persister::startSave(void) */ Snapshot * Persister::getSnapshotForObject(const NetworkId &networkId, uint32 serverId) { - pad.lock(); - auto i = m_objectSnapshotMap.find(networkId); if (i!=m_objectSnapshotMap.end()) { NOT_NULL(i->second); - - pad.unlock(); - return i->second; } else { Snapshot *snap = getSnapshotForServer(serverId); m_objectSnapshotMap[networkId]=snap; - - pad.unlock(); - return snap; } } @@ -426,8 +414,6 @@ bool Persister::hasDataForObject(const NetworkId &objectId) const Snapshot *Persister::getSnapshotForServer(uint32 serverId) { - pad.lock(); - if (serverId==0) { if (!m_arbitraryGameDataSnapshot) @@ -435,9 +421,6 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) m_arbitraryGameDataSnapshot = makeSnapshot(DB::ModeQuery::mode_UPDATE); m_currentSnapshots[0] = m_arbitraryGameDataSnapshot; } - - pad.unlock(); - return m_arbitraryGameDataSnapshot; } else @@ -452,18 +435,12 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) if (!m_arbitraryGameDataSnapshot) { m_arbitraryGameDataSnapshot = snap; } - - pad.unlock(); - return snap; } else { NOT_NULL (j->second); - - pad.unlock(); - return j->second; } } From dd9d5ae6bd23535ee0a57e7a0dfb4adf2af1952f Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 07:16:48 +0000 Subject: [PATCH 10/13] more tracking of creation --- .../serverDatabase/src/shared/Persister.cpp | 23 ------------------- .../serverGame/src/shared/core/GameServer.cpp | 6 +++++ 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index a28fb55e..e9d62df0 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -214,8 +214,6 @@ void Persister::update(real updateTime) void Persister::onFrameBarrierReached() { - pad.lock(); - if (m_newCharacterTaskQueue->getNumPendingTasks() == 0) { ServerSnapshotMap delayedSaves; @@ -281,8 +279,6 @@ void Persister::onFrameBarrierReached() taskQueue->report(); } } - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -392,24 +388,16 @@ void Persister::startSave(void) */ Snapshot * Persister::getSnapshotForObject(const NetworkId &networkId, uint32 serverId) { - pad.lock(); - auto i = m_objectSnapshotMap.find(networkId); if (i!=m_objectSnapshotMap.end()) { NOT_NULL(i->second); - - pad.unlock(); - return i->second; } else { Snapshot *snap = getSnapshotForServer(serverId); m_objectSnapshotMap[networkId]=snap; - - pad.unlock(); - return snap; } } @@ -426,8 +414,6 @@ bool Persister::hasDataForObject(const NetworkId &objectId) const Snapshot *Persister::getSnapshotForServer(uint32 serverId) { - pad.lock(); - if (serverId==0) { if (!m_arbitraryGameDataSnapshot) @@ -435,9 +421,6 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) m_arbitraryGameDataSnapshot = makeSnapshot(DB::ModeQuery::mode_UPDATE); m_currentSnapshots[0] = m_arbitraryGameDataSnapshot; } - - pad.unlock(); - return m_arbitraryGameDataSnapshot; } else @@ -452,18 +435,12 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) if (!m_arbitraryGameDataSnapshot) { m_arbitraryGameDataSnapshot = snap; } - - pad.unlock(); - return snap; } else { NOT_NULL (j->second); - - pad.unlock(); - return j->second; } } diff --git a/engine/server/library/serverGame/src/shared/core/GameServer.cpp b/engine/server/library/serverGame/src/shared/core/GameServer.cpp index e8f25f32..e760ae70 100755 --- a/engine/server/library/serverGame/src/shared/core/GameServer.cpp +++ b/engine/server/library/serverGame/src/shared/core/GameServer.cpp @@ -4769,10 +4769,14 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse // ---------------------------------------------------------------------- // Set up the PlayerObject + WARNING(true, ("Setting up the PlayerObject for %s", vrn.getName().c_str())); + ServerObject *playerServerObject = ServerWorld::createNewObject(ConfigServerGame::getPlayerObjectTemplate(), *newCharacterObject, false); PlayerObject *play = dynamic_cast(playerServerObject); if (play) { + WARNING(true, ("Player object is good...")); + play->setStationId(createMessage->getStationId()); play->setBornDate(); play->setSkillTemplate(createMessage->getSkillTemplate(), true); @@ -4807,6 +4811,8 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse WARNING_STRICT_FATAL(true,("Unable to create PlayerObject for new character %s.\n",newCharacterObject->getNetworkId().getValueString().c_str())); } + + WARNING(true, ("Setting scene and sending to DB for persist.")); newCharacterObject->setSceneIdOnThisAndContents(createMessage->getPlanetName()); // ---------------------------------------------------------------------- From 9e8beefada276da0cf69d1856e78664d2d277dd3 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 08:45:40 +0000 Subject: [PATCH 11/13] general cleanup; remove those mutexes and debug output as they didn't get us anywhere --- .../serverDatabase/src/shared/Persister.cpp | 92 +++++++------------ .../serverDatabase/src/shared/Persister.h | 4 - .../serverGame/src/shared/core/GameServer.cpp | 7 -- 3 files changed, 35 insertions(+), 68 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index e9d62df0..c6f12b2e 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -303,8 +303,6 @@ void Persister::startSave(void) m_newObjectCount=0; - pad.lock(); - // delete any characters for this save cycle if (m_charactersToDeleteNextSaveCycle && m_charactersToDeleteThisSaveCycle) { @@ -375,8 +373,6 @@ void Persister::startSave(void) if (ConfigServerDatabase::getReportSaveTimes()) m_saveStartTime = Clock::timeMs(); - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -515,12 +511,9 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa { UNREF(serverId); - pad.lock(); - if (m_objectSnapshotMap.find(objectId)!=m_objectSnapshotMap.end()) { DEBUG_WARNING(true,("Database received multiple new object messages for object %s",objectId.getValueString().c_str())); - pad.unlock(); return; } @@ -575,8 +568,6 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa NOT_NULL(snap); snap->newObject(objectId, templateId, typeId); m_objectSnapshotMap[objectId]=snap; - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -589,9 +580,6 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) { - - pad.lock(); - //TODO: This is a hack until we remove frame boundaries and have "end frame" messages from the game server. Apparently the game // server can split baselines across frame boundaries, so we can't assume we have all the data for a character when we hit a // frame bounday. @@ -601,8 +589,6 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) m_pendingCharacters.erase(chardata); m_newCharacterLock.erase(serverId); } - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -613,65 +599,57 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) void Persister::saveCompleted(Snapshot *completedSnapshot) { - pad.lock(); - - auto i=std::remove(m_savingSnapshots.begin(),m_savingSnapshots.end(),completedSnapshot); - if (i!=m_savingSnapshots.end()) - { - m_savingSnapshots.erase(i, m_savingSnapshots.end()); - - if (completedSnapshot != nullptr) { - delete completedSnapshot; - completedSnapshot = nullptr; + bool found = false; + for (auto i = m_savingSnapshots.begin(); i != m_savingSnapshots.end();) { + if (*i == completedSnapshot) { + i = m_savingSnapshots.erase(i); + found = true; + } else { + ++i; } + } - if (m_savingSnapshots.empty() && ConfigServerDatabase::getReportSaveTimes()) - { + if (m_savingSnapshots.empty()) + { + if (found && ConfigServerDatabase::getReportSaveTimes()) { int saveTime = Clock::timeMs() - m_saveStartTime; ++m_saveCount; m_totalSaveTime += saveTime; if (saveTime > m_maxSaveTime) m_maxSaveTime = saveTime; - DEBUG_REPORT_LOG(true,("Save completed in %i. (Average %i, max %i)\n", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); - LOG("SaveTimes",("Save completed in %i. (Average %i, max %i)", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); - - m_lastSaveTime = saveTime; + DEBUG_REPORT_LOG(true,("Save completed in %i. (Average %i, max %i)\n", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); + LOG("SaveTimes",("Save completed in %i. (Average %i, max %i)", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); + + m_lastSaveTime = saveTime; } - if (m_savingSnapshots.empty()) - { - // message Central Server that the current save cycle is complete - GenericValueTypeMessage const saveCompleteMessage("DatabaseSaveComplete", ++m_saveCounter); - DatabaseProcess::getInstance().sendToCentralServer(saveCompleteMessage, true); - LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); - } + LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); - { - // set the last save completion time (for the monitoring program) - time_t theTime = time(0); - m_lastSaveCompletionTime = ctime(&theTime); - } + // TODO: so do we send this for the other snapshot type or not? hrmph + // message Central Server that the current save cycle is complete + GenericValueTypeMessage const saveCompleteMessage("DatabaseSaveComplete", ++m_saveCounter); + DatabaseProcess::getInstance().sendToCentralServer(saveCompleteMessage, true); + LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); } - else - { - auto j=std::remove(m_savingCharacterSnapshots.begin(),m_savingCharacterSnapshots.end(),completedSnapshot); - - DEBUG_FATAL(i==m_savingCharacterSnapshots.end(),("Programmer bug: SaveCompleted() called with a snapshot that wasn't in m_savingSnapshots or m_savingCharacterSnapshots.")); - - if (j != m_savingCharacterSnapshots.end()) { - m_savingCharacterSnapshots.erase(j, m_savingCharacterSnapshots.end()); + + if (!found) { + for (auto i = m_savingCharacterSnapshots.begin(); i != m_savingCharacterSnapshots.end();) { + if (*i == completedSnapshot) { + i = m_savingCharacterSnapshots.erase(i); + found = true; + } else { + ++i; + } } - - if (completedSnapshot != nullptr) { - delete completedSnapshot; - completedSnapshot = nullptr; - } - + DEBUG_REPORT_LOG(ConfigServerDatabase::getReportSaveTimes(),("New character save completed\n")); } - pad.unlock(); + if (found && completedSnapshot != nullptr) { + delete completedSnapshot; + completedSnapshot = nullptr; + } } diff --git a/engine/server/library/serverDatabase/src/shared/Persister.h b/engine/server/library/serverDatabase/src/shared/Persister.h index 6f6ba2d6..31b09608 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.h +++ b/engine/server/library/serverDatabase/src/shared/Persister.h @@ -16,8 +16,6 @@ #include #include //TODO: remove when we clean up newCharacterLock hack -#include - #include "Unicode.h" #include "serverNetworkMessages/MessageToPayload.h" #include "sharedDatabaseInterface/DbModeQuery.h" @@ -99,8 +97,6 @@ class Persister : public MessageDispatch::Receiver DB::TaskQueue *m_newCharacterTaskQueue; private: - std::mutex pad; - struct PendingCharacter { uint32 stationId; diff --git a/engine/server/library/serverGame/src/shared/core/GameServer.cpp b/engine/server/library/serverGame/src/shared/core/GameServer.cpp index e760ae70..f48e37ad 100755 --- a/engine/server/library/serverGame/src/shared/core/GameServer.cpp +++ b/engine/server/library/serverGame/src/shared/core/GameServer.cpp @@ -4768,15 +4768,10 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse // ---------------------------------------------------------------------- // Set up the PlayerObject - - WARNING(true, ("Setting up the PlayerObject for %s", vrn.getName().c_str())); - ServerObject *playerServerObject = ServerWorld::createNewObject(ConfigServerGame::getPlayerObjectTemplate(), *newCharacterObject, false); PlayerObject *play = dynamic_cast(playerServerObject); if (play) { - WARNING(true, ("Player object is good...")); - play->setStationId(createMessage->getStationId()); play->setBornDate(); play->setSkillTemplate(createMessage->getSkillTemplate(), true); @@ -4811,8 +4806,6 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse WARNING_STRICT_FATAL(true,("Unable to create PlayerObject for new character %s.\n",newCharacterObject->getNetworkId().getValueString().c_str())); } - - WARNING(true, ("Setting scene and sending to DB for persist.")); newCharacterObject->setSceneIdOnThisAndContents(createMessage->getPlanetName()); // ---------------------------------------------------------------------- From 0995323201efb235496949112b87ed412d1c7f52 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 08:53:48 +0000 Subject: [PATCH 12/13] move these to before and after as they were before the changes --- .../serverDatabase/src/shared/Snapshot.cpp | 28 ++----------------- .../src/shared/core/SwgSnapshot.cpp | 14 ++++++++++ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Snapshot.cpp b/engine/server/library/serverDatabase/src/shared/Snapshot.cpp index 163c2225..c61520c0 100755 --- a/engine/server/library/serverDatabase/src/shared/Snapshot.cpp +++ b/engine/server/library/serverDatabase/src/shared/Snapshot.cpp @@ -69,34 +69,10 @@ bool Snapshot::saveToDB(DB::Session *session) { NOT_NULL(session); - m_isBeingSaved = true; - - CustomStepListType::iterator step; - for (step=m_customStepList.begin(); step !=m_customStepList.end(); ++step) - { - NOT_NULL(*step); - if (!(*step)->beforePersist(session)) { - m_isBeingSaved = false; - return false; - } + if (m_timestamp != 0 && !saveTimestamp(session)) { + return false; } - if (m_timestamp!=0) - if (! saveTimestamp(session)) { - m_isBeingSaved = false; - return false; - } - - for (step=m_customStepList.begin(); step !=m_customStepList.end(); ++step) - { - NOT_NULL(*step); - if (!(*step)->afterPersist(session)){ - m_isBeingSaved = false; - return false; - } - } - - m_isBeingSaved = false; return true; } // ---------------------------------------------------------------------- diff --git a/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp b/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp index e6f04dd2..a111e70c 100755 --- a/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp +++ b/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp @@ -94,6 +94,13 @@ bool SwgSnapshot::saveToDB(DB::Session *session) { session->setAutoCommitMode(false); + for (auto step = m_customStepList.begin(); step !=m_customStepList.end(); ++step) + { + if (!(*step)->beforePersist(session)) { + return false; + } + } + // save all the buffers if (!(m_objectTableBuffer.save(session))) { return false; } if (!(m_battlefieldMarkerObjectBuffer.save(session))) { return false; } @@ -131,6 +138,13 @@ bool SwgSnapshot::saveToDB(DB::Session *session) { if (!(m_waypointBuffer.save(session))) { return false; } if (!(m_weaponObjectBuffer.save(session))) { return false; } + for (auto step = m_customStepList.begin(); step !=m_customStepList.end(); ++step) + { + if (!(*step)->afterPersist(session)){ + return false; + } + } + // save the parent class if (!(Snapshot::saveToDB(session))) { return false; } From 4f347f0d22a898b620ea2897b6bfbc1a91e429b8 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 10:05:29 +0000 Subject: [PATCH 13/13] if this fixes our issue it's a one liner...wow --- engine/server/library/serverGame/src/shared/core/GameServer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/engine/server/library/serverGame/src/shared/core/GameServer.cpp b/engine/server/library/serverGame/src/shared/core/GameServer.cpp index f48e37ad..80009486 100755 --- a/engine/server/library/serverGame/src/shared/core/GameServer.cpp +++ b/engine/server/library/serverGame/src/shared/core/GameServer.cpp @@ -4799,6 +4799,8 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse } } } + + play->persist(); } else {