From d1d5c5dd7a27fdc466c884b3024f93c3ccb60a0d Mon Sep 17 00:00:00 2001 From: brainiac Date: Sat, 4 Apr 2026 17:34:59 -0700 Subject: [PATCH] Switch database mutex to recursive because we copy it everywhere ... and I have no idea if any of the calls are re-entrant. yay. --- common/dbcore.cpp | 8 ++++---- common/dbcore.h | 7 ++++--- common/event/task_scheduler.cpp | 2 +- common/mysql_stmt.cpp | 2 +- common/mysql_stmt.h | 8 ++++---- world/cli/cli_database_concurrency.cpp | 2 +- world/world_boot.cpp | 2 +- zone/main.cpp | 2 +- 8 files changed, 17 insertions(+), 16 deletions(-) diff --git a/common/dbcore.cpp b/common/dbcore.cpp index d9985ce41..1cde3b30f 100644 --- a/common/dbcore.cpp +++ b/common/dbcore.cpp @@ -34,7 +34,7 @@ DBcore::DBcore() : mysql(mysql_init(nullptr)) - , m_mutex(std::make_shared()) + , m_mutex(std::make_shared()) { } @@ -220,12 +220,12 @@ bool DBcore::Open( bool DBcore::Open(uint32 *errnum, char *errbuf) { + // Expects m_mutex to already be locked. + if (errbuf) { errbuf[0] = 0; } - std::scoped_lock lock(*m_mutex); - if (GetStatus() == Connected) { return true; } @@ -294,7 +294,7 @@ std::string DBcore::Escape(const std::string& s) return temp.data(); } -void DBcore::SetMutex(const std::shared_ptr& mutex) +void DBcore::SetMutex(const std::shared_ptr& mutex) { DBcore::m_mutex = mutex; } diff --git a/common/dbcore.h b/common/dbcore.h index 09bad28de..b9b25d149 100644 --- a/common/dbcore.h +++ b/common/dbcore.h @@ -36,6 +36,8 @@ public: Closed, Connected, Error }; + using Mutex = std::recursive_mutex; + DBcore(); ~DBcore(); eStatus GetStatus() { return pStatus; } @@ -59,7 +61,7 @@ public: mysql = o.mysql; mysqlOwner = false; } - void SetMutex(const std::shared_ptr& mutex); + void SetMutex(const std::shared_ptr& mutex); // only safe on connections shared with other threads if results buffered // unsafe to use off main thread due to internal server logging @@ -86,8 +88,7 @@ private: bool mysqlOwner = true; eStatus pStatus = Closed; - std::shared_ptr m_mutex; - std::mutex m_query_lock; + std::shared_ptr m_mutex; std::string origin_host; diff --git a/common/event/task_scheduler.cpp b/common/event/task_scheduler.cpp index 7ee3894fd..2b0fd7ef9 100644 --- a/common/event/task_scheduler.cpp +++ b/common/event/task_scheduler.cpp @@ -95,7 +95,7 @@ void TaskScheduler::ProcessWork() m_data->cv.wait(lock, [this] { - return !m_data->running || m_data->tasks.empty(); + return !m_data->running || !m_data->tasks.empty(); }); if (!m_data->running) diff --git a/common/mysql_stmt.cpp b/common/mysql_stmt.cpp index a32133bd9..53a45b327 100644 --- a/common/mysql_stmt.cpp +++ b/common/mysql_stmt.cpp @@ -35,7 +35,7 @@ void PreparedStmt::StmtDeleter::operator()(MYSQL_STMT* stmt) noexcept mysql_stmt_close(stmt); } -PreparedStmt::PreparedStmt(MYSQL& mysql, std::string query, std::mutex& mutex, StmtOptions opts) +PreparedStmt::PreparedStmt(MYSQL& mysql, std::string query, DBcore::Mutex& mutex, StmtOptions opts) : m_stmt(mysql_stmt_init(&mysql), { mutex }) , m_query(std::move(query)) , m_options(opts) diff --git a/common/mysql_stmt.h b/common/mysql_stmt.h index 69bd97d41..2402d02e0 100644 --- a/common/mysql_stmt.h +++ b/common/mysql_stmt.h @@ -17,8 +17,8 @@ */ #pragma once +#include "common/dbcore.h" -#include "mysql.h" #include #include #include @@ -185,7 +185,7 @@ public: int64_t, uint64_t, float, double, bool, std::string_view, std::nullptr_t>; PreparedStmt() = delete; - PreparedStmt(MYSQL& mysql, std::string query, std::mutex& mutex, StmtOptions opts = {}); + PreparedStmt(MYSQL& mysql, std::string query, DBcore::Mutex& mutex, StmtOptions opts = {}); const std::string& GetQuery() const { return m_query; } StmtOptions GetOptions() const { return m_options; } @@ -221,7 +221,7 @@ private: struct StmtDeleter { - std::mutex& mutex; + DBcore::Mutex& mutex; void operator()(MYSQL_STMT* stmt) noexcept; }; @@ -235,7 +235,7 @@ private: std::string m_query; StmtOptions m_options = {}; bool m_need_bind = true; - std::mutex& m_mutex; // connection mutex + DBcore::Mutex& m_mutex; // connection mutex }; } // namespace mysql diff --git a/world/cli/cli_database_concurrency.cpp b/world/cli/cli_database_concurrency.cpp index 306407f58..6816d1c15 100644 --- a/world/cli/cli_database_concurrency.cpp +++ b/world/cli/cli_database_concurrency.cpp @@ -76,7 +76,7 @@ void WorldserverCLI::TestDatabaseConcurrency(int argc, char **argv, argh::parser return; } - std::shared_ptr sharedMutex = std::make_shared(); + std::shared_ptr sharedMutex = std::make_shared(); db.SetMutex(sharedMutex); diff --git a/world/world_boot.cpp b/world/world_boot.cpp index 97f4a172b..4bd6c79fd 100644 --- a/world/world_boot.cpp +++ b/world/world_boot.cpp @@ -180,7 +180,7 @@ bool WorldBoot::LoadDatabaseConnections() // when database and content_db share the same underlying mysql connection // it needs to be protected by a shared mutex otherwise we produce concurrency issues // when database actions are occurring in different threads - std::shared_ptr sharedMutex = std::make_shared(); + std::shared_ptr sharedMutex = std::make_shared(); database.SetMutex(sharedMutex); content_db.SetMutex(sharedMutex); } diff --git a/zone/main.cpp b/zone/main.cpp index 6f2c0bdc3..9aab5adf8 100644 --- a/zone/main.cpp +++ b/zone/main.cpp @@ -246,7 +246,7 @@ int main(int argc, char **argv) // when database and content_db share the same underlying mysql connection // it needs to be protected by a shared mutex otherwise we produce concurrency issues // when database actions are occurring in different threads - std::shared_ptr sharedMutex = std::make_shared(); + std::shared_ptr sharedMutex = std::make_shared(); database.SetMutex(sharedMutex); content_db.SetMutex(sharedMutex); }