Push up mutex changes

common/dbcore.cpp

@@ -43,7 +43,7 @@ DBcore::DBcore()
 	pCompress       = false;
 	pSSL            = false;
 	pStatus         = Closed;
-
+	m_mutex         = new Mutex;
 }
 
 DBcore::~DBcore()
@@ -66,12 +66,12 @@ DBcore::~DBcore()
 // Sends the MySQL server a keepalive
 void DBcore::ping()
 {
-	if (!MDatabase.trylock()) {
+	if (!m_mutex->trylock()) {
 		// 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_mutex->unlock();
 }
 
 MySQLRequestResult DBcore::QueryDatabase(std::string query, bool retryOnFailureOnce)
@@ -92,7 +92,7 @@ MySQLRequestResult DBcore::QueryDatabase(const char *query, uint32 querylen, boo
 	BenchTimer timer;
 	timer.reset();
 
-	LockMutex lock(&MDatabase);
+	LockMutex lock(m_mutex);
 
 	// Reconnect if we are not connected before hand.
 	if (pStatus != Connected) {
@@ -217,7 +217,7 @@ bool DBcore::Open(
 	bool iSSL
 )
 {
-	LockMutex lock(&MDatabase);
+	LockMutex lock(m_mutex);
 	safe_delete_array(pHost);
 	safe_delete_array(pUser);
 	safe_delete_array(pPassword);
@@ -237,7 +237,7 @@ bool DBcore::Open(uint32 *errnum, char *errbuf)
 	if (errbuf) {
 		errbuf[0] = 0;
 	}
-	LockMutex lock(&MDatabase);
+	LockMutex lock(m_mutex);
 	if (GetStatus() == Connected) {
 		return true;
 	}
@@ -298,3 +298,10 @@ std::string DBcore::Escape(const std::string& s)
 
 	return temp.data();
 }
+
+void DBcore::SetMutex(Mutex *mutex)
+{
+	safe_delete(m_mutex);
+
+	DBcore::m_mutex = mutex;
+}

common/dbcore.h

@@ -37,7 +37,12 @@ class DBcore {
 
 	bool DoesTableExist(std::string table_name);
 
-	void SetMySQL(const DBcore& o) { mysql = o.mysql; mysqlOwner = false; }
+	void SetMySQL(const DBcore &o)
+	{
+		mysql      = o.mysql;
+		mysqlOwner = false;
+	}
+	void SetMutex(Mutex *mutex);
 
 protected:
 	bool Open(
@@ -56,8 +61,8 @@ class DBcore {
 	bool Open(uint32 *errnum = nullptr, char *errbuf = nullptr);
 
 	MYSQL*  mysql;
-	bool	mysqlOwner;
-	Mutex   MDatabase;
+	bool    mysqlOwner;
+	Mutex   *m_mutex;
 	eStatus pStatus;
 
 	std::mutex m_query_lock{};

world/cli/database_concurrency.cpp

@@ -0,0 +1,73 @@
+#include <thread>
+#include "../../common/repositories/zone_repository.h"
+#include "../../common/eqemu_config.h"
+#include <signal.h>
+
+Database db;
+Database db2;
+
+volatile sig_atomic_t stop;
+void inthand(int signum) {
+	stop = 1;
+}
+
+[[noreturn]] void DatabaseTest()
+{
+	while (true) {
+		LogInfo("DatabaseTest Query");
+		db.QueryDatabase("SELECT 1");
+	}
+}
+
+[[noreturn]] void DatabaseTestSecondConnection()
+{
+	while (true) {
+		LogInfo("DatabaseTest Query");
+		db2.QueryDatabase("SELECT 1");
+	}
+}
+
+
+void WorldserverCLI::TestDatabaseConcurrency(int argc, char **argv, argh::parser &cmd, std::string &description)
+{
+	description = "Test command to test database concurrency";
+
+	if (cmd[{"-h", "--help"}]) {
+		return;
+	}
+
+	signal(SIGINT, inthand);
+
+	LogInfo("Database test");
+
+	auto mutex = new Mutex;
+
+	auto c = EQEmuConfig::get();
+	LogInfo("Connecting to MySQL");
+	if (!db.Connect(
+		c->DatabaseHost.c_str(),
+		c->DatabaseUsername.c_str(),
+		c->DatabasePassword.c_str(),
+		c->DatabaseDB.c_str(),
+		c->DatabasePort
+	)) {
+		LogError("Cannot continue without a database connection");
+		return;
+	}
+
+	db.SetMutex(mutex);
+
+	db2.SetMySQL(db);
+
+	db2.SetMutex(mutex);
+
+	std::thread(DatabaseTest).detach();
+	std::thread(DatabaseTest).detach();
+	std::thread(DatabaseTestSecondConnection).detach();
+
+	while (!stop) {
+
+	}
+
+	safe_delete(mutex);
+}

world/main.cpp

@@ -478,6 +478,8 @@ int main(int argc, char **argv)
 	LogInfo("Signaling HTTP service to stop");
 	LogSys.CloseFileLogs();
 
+	WorldBoot::Shutdown();
+
 	return 0;
 }
 

world/world_boot.cpp

@@ -30,6 +30,8 @@
 extern ZSList      zoneserver_list;
 extern WorldConfig Config;
 
+auto mutex = new Mutex;
+
 void WorldBoot::GMSayHookCallBackProcessWorld(uint16 log_category, const char *func, std::string message)
 {
 	// we don't want to loop up with chat messages
@@ -136,9 +138,7 @@ bool WorldBoot::LoadDatabaseConnections()
 		return false;
 	}
 
-	/**
-	 * Multi-tenancy: Content database
-	 */
+	// Multi-tenancy - content database
 	if (!c->ContentDbHost.empty()) {
 		if (!content_db.Connect(
 			c->ContentDbHost.c_str(),
@@ -154,6 +154,11 @@ bool WorldBoot::LoadDatabaseConnections()
 	}
 	else {
 		content_db.SetMySQL(database);
+		// 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
+		database.SetMutex(mutex);
+		content_db.SetMutex(mutex);
 	}
 
 	return true;
@@ -652,3 +657,8 @@ void WorldBoot::CheckForPossibleConfigurationIssues()
 	}
 }
 
+void WorldBoot::Shutdown()
+{
+	safe_delete(mutex);
+}
+

world/world_boot.h

@@ -15,6 +15,7 @@ class WorldBoot {
 	static void RegisterLoginservers();
 	static bool DatabaseLoadRoutines(int argc, char **argv);
 	static void CheckForPossibleConfigurationIssues();
+	static void Shutdown();
 };
 
 

world/world_server_cli.cpp

@@ -31,10 +31,12 @@ void WorldserverCLI::CommandHandler(int argc, char **argv)
 	function_map["test:expansion"]              = &WorldserverCLI::ExpansionTestCommand;
 	function_map["test:repository"]             = &WorldserverCLI::TestRepository;
 	function_map["test:repository2"]            = &WorldserverCLI::TestRepository2;
+	function_map["test:db-concurrency"]         = &WorldserverCLI::TestDatabaseConcurrency;
 
 	EQEmuCommand::HandleMenu(function_map, cmd, argc, argv);
 }
 
+#include "cli/database_concurrency.cpp"
 #include "cli/copy_character.cpp"
 #include "cli/database_dump.cpp"
 #include "cli/database_get_schema.cpp"

world/world_server_cli.h

@@ -18,6 +18,7 @@ class WorldserverCLI {
 	static void ExpansionTestCommand(int argc, char **argv, argh::parser &cmd, std::string &description);
 	static void TestRepository(int argc, char **argv, argh::parser &cmd, std::string &description);
 	static void TestRepository2(int argc, char **argv, argh::parser &cmd, std::string &description);
+	static void TestDatabaseConcurrency(int argc, char **argv, argh::parser &cmd, std::string &description);
 };
 
 

zone/main.cpp

@@ -231,6 +231,8 @@ int main(int argc, char** argv) {
 		worldserver.SetLauncherName("NONE");
 	}
 
+	auto mutex = new Mutex;
+
 	LogInfo("Connecting to MySQL");
 	if (!database.Connect(
 		Config->DatabaseHost.c_str(),
@@ -242,9 +244,7 @@ int main(int argc, char** argv) {
 		return 1;
 	}
 
-	/**
-	 * Multi-tenancy: Content Database
-	 */
+	// Multi-tenancy: Content Database
 	if (!Config->ContentDbHost.empty()) {
 		if (!content_db.Connect(
 			Config->ContentDbHost.c_str() ,
@@ -259,6 +259,11 @@ int main(int argc, char** argv) {
 		}
 	} else {
 		content_db.SetMySQL(database);
+		// 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
+		database.SetMutex(mutex);
+		content_db.SetMutex(mutex);
 	}
 
 	/* Register Log System and Settings */
@@ -613,6 +618,9 @@ int main(int argc, char** argv) {
 	safe_delete(parse);
 	LogInfo("Proper zone shutdown complete.");
 	LogSys.CloseFileLogs();
+
+	safe_delete(mutex);
+
 	return 0;
 }