Skip to content

Commit

Permalink
Core/Database: Use RAII for resource management in MySQLConnection
Browse files Browse the repository at this point in the history 
* Prevents double deletion of MySQLConnection after errors
* The object stays valid after an error and will wait for a reconnect
* Also crash the server if 5 reconnects fail
* Corrects an issue where the server was crashed after one reconnect
  because mysql_thread_id was invoked with an invalid handle
  • Loading branch information
@Naios
Naios committed Mar 3, 2016
1 parent 09fa0ab commit 62815c6
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 37 deletions.
87 changes: 54 additions & 33 deletions src/server/database/Database/MySQLConnection.cpp
View file
Expand Up @@ -37,7 +37,6 @@ MySQLConnection::MySQLConnection(MySQLConnectionInfo& connInfo) :
m_reconnecting(false),
m_prepareError(false),
m_queue(NULL),
m_worker(NULL),
m_Mysql(NULL),
m_connectionInfo(connInfo),
m_connectionFlags(CONNECTION_SYNCH) { }
Expand All @@ -50,24 +49,26 @@ m_Mysql(NULL),
m_connectionInfo(connInfo),
m_connectionFlags(CONNECTION_ASYNC)
{
m_worker = new DatabaseWorker(m_queue, this);
m_worker = Trinity::make_unique<DatabaseWorker>(m_queue, this);
}

MySQLConnection::~MySQLConnection()
{
delete m_worker;

for (size_t i = 0; i < m_stmts.size(); ++i)
delete m_stmts[i];

if (m_Mysql)
mysql_close(m_Mysql);
Close();
}

void MySQLConnection::Close()
{
/// Only close us if we're not operating
delete this;
// Stop the worker thread before the statements are cleared
m_worker.reset();

m_stmts.clear();

if (m_Mysql)
{
mysql_close(m_Mysql);
m_Mysql = nullptr;
}
}

uint32 MySQLConnection::Open()
Expand Down Expand Up @@ -412,7 +413,7 @@ int MySQLConnection::ExecuteTransaction(SQLTransaction& transaction)
MySQLPreparedStatement* MySQLConnection::GetPreparedStatement(uint32 index)
{
ASSERT(index < m_stmts.size());
MySQLPreparedStatement* ret = m_stmts[index];
MySQLPreparedStatement* ret = m_stmts[index].get();
if (!ret)
TC_LOG_ERROR("sql.sql", "Could not fetch prepared statement %u on database `%s`, connection type: %s.",
index, m_connectionInfo.database.c_str(), (m_connectionFlags & CONNECTION_ASYNC) ? "asynchronous" : "synchronous");
Expand All @@ -424,16 +425,12 @@ void MySQLConnection::PrepareStatement(uint32 index, const char* sql, Connection
{
m_queries.insert(PreparedStatementMap::value_type(index, std::make_pair(sql, flags)));

// For reconnection case
if (m_reconnecting)
delete m_stmts[index];

// Check if specified query should be prepared on this connection
// i.e. don't prepare async statements on synchronous connections
// to save memory that will not be used.
if (!(m_connectionFlags & flags))
{
m_stmts[index] = NULL;
m_stmts[index].reset();
return;
}

Expand All @@ -455,8 +452,7 @@ void MySQLConnection::PrepareStatement(uint32 index, const char* sql, Connection
}
else
{
MySQLPreparedStatement* mStmt = new MySQLPreparedStatement(stmt);
m_stmts[index] = mStmt;
m_stmts[index] = Trinity::make_unique<MySQLPreparedStatement>(stmt);
}
}
}
Expand All @@ -477,7 +473,7 @@ PreparedResultSet* MySQLConnection::Query(PreparedStatement* stmt)
return new PreparedResultSet(stmt->m_stmt->GetSTMT(), result, rowCount, fieldCount);
}

bool MySQLConnection::_HandleMySQLErrno(uint32 errNo)
bool MySQLConnection::_HandleMySQLErrno(uint32 errNo, uint8 attempts /*= 5*/)
{
switch (errNo)
{
Expand All @@ -486,34 +482,59 @@ bool MySQLConnection::_HandleMySQLErrno(uint32 errNo)
case CR_INVALID_CONN_HANDLE:
case CR_SERVER_LOST_EXTENDED:
{
if (m_Mysql)
{
TC_LOG_ERROR("sql.sql", "Lost the connection to the MySQL server!");

mysql_close(GetHandle());
m_Mysql = nullptr;
}

/*no break*/
}
case CR_CONN_HOST_ERROR:
{
TC_LOG_INFO("sql.sql", "Attempting to reconnect to the MySQL server...");

m_reconnecting = true;
uint64 oldThreadId = mysql_thread_id(GetHandle());
mysql_close(GetHandle());

uint32 const lErrno = Open();
if (!lErrno)
{
// Don't remove 'this' pointer unless you want to skip loading all prepared statements...
if (!this->PrepareStatements())
{
TC_LOG_ERROR("sql.sql", "Could not re-prepare statements!");
Close();
return false;
TC_LOG_FATAL("sql.sql", "Could not re-prepare statements!");
std::this_thread::sleep_for(std::chrono::seconds(10));
std::abort();

This comment has been minimized.

Sign in to view

Copy link
@jackpoz

jackpoz Mar 3, 2016

Member

what's the behavior of std::abort() with WEH ?

This comment has been minimized.

Sign in to view

Copy link
@Shauren

Shauren Mar 3, 2016

Member

No log. This is why you cannot use assert() macro from header and need our custom one - crt calls abort() from there.

This comment has been minimized.

Sign in to view

Copy link
@jackpoz

jackpoz Mar 3, 2016

Member

can we redefine abort() and std::abort() as static_assert("Never call this function, use ASSERT("Reason") instead") ?
}

TC_LOG_INFO("sql.sql", "Connection to the MySQL server is active.");
if (oldThreadId != mysql_thread_id(GetHandle()))
TC_LOG_INFO("sql.sql", "Successfully reconnected to %s @%s:%s (%s).",
m_connectionInfo.database.c_str(), m_connectionInfo.host.c_str(), m_connectionInfo.port_or_socket.c_str(),
(m_connectionFlags & CONNECTION_ASYNC) ? "asynchronous" : "synchronous");
TC_LOG_INFO("sql.sql", "Successfully reconnected to %s @%s:%s (%s).",
m_connectionInfo.database.c_str(), m_connectionInfo.host.c_str(), m_connectionInfo.port_or_socket.c_str(),
(m_connectionFlags & CONNECTION_ASYNC) ? "asynchronous" : "synchronous");

m_reconnecting = false;
return true;
}

// It's possible this attempted reconnect throws 2006 at us. To prevent crazy recursive calls, sleep here.
std::this_thread::sleep_for(std::chrono::seconds(3)); // Sleep 3 seconds
return _HandleMySQLErrno(lErrno); // Call self (recursive)
if ((--attempts) == 0)
{
// Shut down the server when the mysql server isn't
// reachable for some time
TC_LOG_FATAL("sql.sql", "Failed to reconnect to the MySQL server, "
"terminating the server to prevent data corruption!");

// We could also initiate a shutdown through using std::raise(SIGTERM)
std::this_thread::sleep_for(std::chrono::seconds(10));
std::abort();
}
else
{
// It's possible this attempted reconnect throws 2006 at us.
// To prevent crazy recursive calls, sleep here.
std::this_thread::sleep_for(std::chrono::seconds(3)); // Sleep 3 seconds
return _HandleMySQLErrno(lErrno, attempts); // Call self (recursive)
}
}

case ER_LOCK_DEADLOCK:
Expand Down
8 changes: 4 additions & 4 deletions src/server/database/Database/MySQLConnection.h
View file
Expand Up @@ -116,18 +116,18 @@ class MySQLConnection
virtual void DoPrepareStatements() = 0;

protected:
std::vector<MySQLPreparedStatement*> m_stmts; //! PreparedStatements storage
std::vector<std::unique_ptr<MySQLPreparedStatement>> m_stmts; //! PreparedStatements storage
PreparedStatementMap m_queries; //! Query storage
bool m_reconnecting; //! Are we reconnecting?
bool m_prepareError; //! Was there any error while preparing statements?

private:
bool _HandleMySQLErrno(uint32 errNo);
bool _HandleMySQLErrno(uint32 errNo, uint8 attempts = 5);

private:
ProducerConsumerQueue<SQLOperation*>* m_queue; //! Queue shared with other asynchronous connections.
DatabaseWorker* m_worker; //! Core worker task.
MYSQL * m_Mysql; //! MySQL Handle.
std::unique_ptr<DatabaseWorker> m_worker; //! Core worker task.
MYSQL* m_Mysql; //! MySQL Handle.
MySQLConnectionInfo& m_connectionInfo; //! Connection info (used for logging)
ConnectionFlags m_connectionFlags; //! Connection flags (for preparing relevant statements)
std::mutex m_Mutex;
Expand Down

0 comments on commit 62815c6

Please sign in to comment.