From 33bb5aa8e5a7e00b9413f35c03809e0930262490 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Mon, 20 Feb 2023 22:32:29 -0600 Subject: [PATCH] [Database] Address deadlock in player events (#2974) * DB mutex testing * Mutex tweaks, native string escaping --- common/dbcore.cpp | 22 +++++++++++++------ common/dbcore.h | 4 ++++ common/events/player_event_logs.cpp | 3 ++- .../base/base_player_event_logs_repository.h | 12 +++++----- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/common/dbcore.cpp b/common/dbcore.cpp index 3aa62c402..64ab2f005 100644 --- a/common/dbcore.cpp +++ b/common/dbcore.cpp @@ -70,17 +70,20 @@ DBcore::~DBcore() // Sends the MySQL server a keepalive void DBcore::ping() { - if (!MDatabase.trylock()) { + if (!m_query_lock.try_lock()) { // well, if's it's locked, someone's using it. If someone's using it, it doesnt need a keepalive return; } mysql_ping(&mysql); - MDatabase.unlock(); + m_query_lock.unlock(); } MySQLRequestResult DBcore::QueryDatabase(std::string query, bool retryOnFailureOnce) { - return QueryDatabase(query.c_str(), query.length(), retryOnFailureOnce); + m_query_lock.lock(); + auto r = QueryDatabase(query.c_str(), query.length(), retryOnFailureOnce); + m_query_lock.unlock(); + return r; } bool DBcore::DoesTableExist(std::string table_name) @@ -95,15 +98,11 @@ MySQLRequestResult DBcore::QueryDatabase(const char *query, uint32 querylen, boo BenchTimer timer; timer.reset(); - LockMutex lock(&MDatabase); - // Reconnect if we are not connected before hand. if (pStatus != Connected) { Open(); } - - // request query. != 0 indicates some kind of error. if (mysql_real_query(&mysql, query, querylen) != 0) { unsigned int errorNumber = mysql_errno(&mysql); @@ -299,3 +298,12 @@ void DBcore::SetOriginHost(const std::string &origin_host) { DBcore::origin_host = origin_host; } + +std::string DBcore::Escape(const std::string& s) +{ + const std::size_t s_len = s.length(); + std::vector temp((s_len * 2) + 1, '\0'); + mysql_real_escape_string(&mysql, temp.data(), s.c_str(), s_len); + + return temp.data(); +} diff --git a/common/dbcore.h b/common/dbcore.h index 8e8e6b46b..90a5ca505 100644 --- a/common/dbcore.h +++ b/common/dbcore.h @@ -12,6 +12,7 @@ #include #include +#include class DBcore { public: @@ -27,6 +28,7 @@ public: void TransactionBegin(); void TransactionCommit(); void TransactionRollback(); + std::string Escape(const std::string& s); uint32 DoEscapeString(char *tobuf, const char *frombuf, uint32 fromlen); void ping(); MYSQL *getMySQL() { return &mysql; } @@ -57,6 +59,8 @@ private: Mutex MDatabase; eStatus pStatus; + std::mutex m_query_lock{}; + std::string origin_host; char *pHost; diff --git a/common/events/player_event_logs.cpp b/common/events/player_event_logs.cpp index c3c64a4c2..8e9872962 100644 --- a/common/events/player_event_logs.cpp +++ b/common/events/player_event_logs.cpp @@ -113,7 +113,9 @@ bool PlayerEventLogs::IsEventEnabled(PlayerEvent::EventType event) // this processes any current player events on the queue void PlayerEventLogs::ProcessBatchQueue() { + m_batch_queue_lock.lock(); if (m_record_batch_queue.empty()) { + m_batch_queue_lock.unlock(); return; } @@ -128,7 +130,6 @@ void PlayerEventLogs::ProcessBatchQueue() ); // empty - m_batch_queue_lock.lock(); m_record_batch_queue = {}; m_batch_queue_lock.unlock(); } diff --git a/common/repositories/base/base_player_event_logs_repository.h b/common/repositories/base/base_player_event_logs_repository.h index a32c8ed0e..8030303a9 100644 --- a/common/repositories/base/base_player_event_logs_repository.h +++ b/common/repositories/base/base_player_event_logs_repository.h @@ -240,8 +240,8 @@ public: v.push_back(columns[7] + " = " + std::to_string(e.z)); v.push_back(columns[8] + " = " + std::to_string(e.heading)); v.push_back(columns[9] + " = " + std::to_string(e.event_type_id)); - v.push_back(columns[10] + " = '" + Strings::Escape(e.event_type_name) + "'"); - v.push_back(columns[11] + " = '" + Strings::Escape(e.event_data) + "'"); + v.push_back(columns[10] + " = '" + db.Escape(e.event_type_name) + "'"); + v.push_back(columns[11] + " = '" + db.Escape(e.event_data) + "'"); v.push_back(columns[12] + " = FROM_UNIXTIME(" + (e.created_at > 0 ? std::to_string(e.created_at) : "null") + ")"); auto results = db.QueryDatabase( @@ -274,8 +274,8 @@ public: v.push_back(std::to_string(e.z)); v.push_back(std::to_string(e.heading)); v.push_back(std::to_string(e.event_type_id)); - v.push_back("'" + Strings::Escape(e.event_type_name) + "'"); - v.push_back("'" + Strings::Escape(e.event_data) + "'"); + v.push_back("'" + db.Escape(e.event_type_name) + "'"); + v.push_back("'" + db.Escape(e.event_data) + "'"); v.push_back("FROM_UNIXTIME(" + (e.created_at > 0 ? std::to_string(e.created_at) : "null") + ")"); auto results = db.QueryDatabase( @@ -316,8 +316,8 @@ public: v.push_back(std::to_string(e.z)); v.push_back(std::to_string(e.heading)); v.push_back(std::to_string(e.event_type_id)); - v.push_back("'" + Strings::Escape(e.event_type_name) + "'"); - v.push_back("'" + Strings::Escape(e.event_data) + "'"); + v.push_back("'" + db.Escape(e.event_type_name) + "'"); + v.push_back("'" + db.Escape(e.event_data) + "'"); v.push_back("FROM_UNIXTIME(" + (e.created_at > 0 ? std::to_string(e.created_at) : "null") + ")"); insert_chunks.push_back("(" + Strings::Implode(",", v) + ")");