From 53482f729995889936ad8fba9fc5789d9cdba85a Mon Sep 17 00:00:00 2001 From: Shauren Date: Sat, 24 Oct 2020 17:37:24 +0200 Subject: Core/DBLayer: Fixed memory leaks with existing SQLQueryHolder uses and eliminated the possibilty of adding more in future (replaced manual memory management with smart pointers) --- src/server/database/Database/DatabaseEnvFwd.h | 4 +-- .../database/Database/DatabaseWorkerPool.cpp | 4 +-- src/server/database/Database/DatabaseWorkerPool.h | 2 +- src/server/database/Database/QueryHolder.cpp | 30 ++++++++-------------- src/server/database/Database/QueryHolder.h | 21 ++++++++------- src/server/database/Database/SQLOperation.h | 7 ----- 6 files changed, 28 insertions(+), 40 deletions(-) (limited to 'src/server/database/Database') diff --git a/src/server/database/Database/DatabaseEnvFwd.h b/src/server/database/Database/DatabaseEnvFwd.h index ba41655e213..c92c8092775 100644 --- a/src/server/database/Database/DatabaseEnvFwd.h +++ b/src/server/database/Database/DatabaseEnvFwd.h @@ -72,8 +72,8 @@ using LoginDatabaseTransaction = SQLTransaction; using WorldDatabaseTransaction = SQLTransaction; class SQLQueryHolderBase; -using QueryResultHolderFuture = std::future; -using QueryResultHolderPromise = std::promise; +using QueryResultHolderFuture = std::future; +using QueryResultHolderPromise = std::promise; template class SQLQueryHolder; diff --git a/src/server/database/Database/DatabaseWorkerPool.cpp b/src/server/database/Database/DatabaseWorkerPool.cpp index 8ef1e28d70f..c09f7c3e4b6 100644 --- a/src/server/database/Database/DatabaseWorkerPool.cpp +++ b/src/server/database/Database/DatabaseWorkerPool.cpp @@ -226,13 +226,13 @@ QueryCallback DatabaseWorkerPool::AsyncQuery(PreparedStatement* stmt) } template -SQLQueryHolderCallback DatabaseWorkerPool::DelayQueryHolder(SQLQueryHolder* holder) +SQLQueryHolderCallback DatabaseWorkerPool::DelayQueryHolder(std::shared_ptr> holder) { SQLQueryHolderTask* task = new SQLQueryHolderTask(holder); // Store future result before enqueueing - task might get already processed and deleted before returning from this method QueryResultHolderFuture result = task->GetFuture(); Enqueue(task); - return result; + return { std::move(holder), std::move(result) }; } template diff --git a/src/server/database/Database/DatabaseWorkerPool.h b/src/server/database/Database/DatabaseWorkerPool.h index f46c0e29c04..9852ded374e 100644 --- a/src/server/database/Database/DatabaseWorkerPool.h +++ b/src/server/database/Database/DatabaseWorkerPool.h @@ -160,7 +160,7 @@ class DatabaseWorkerPool //! return object as soon as the query is executed. //! The return value is then processed in ProcessQueryCallback methods. //! Any prepared statements added to this holder need to be prepared with the CONNECTION_ASYNC flag. - SQLQueryHolderCallback DelayQueryHolder(SQLQueryHolder* holder); + SQLQueryHolderCallback DelayQueryHolder(std::shared_ptr> holder); /** Transaction context methods. diff --git a/src/server/database/Database/QueryHolder.cpp b/src/server/database/Database/QueryHolder.cpp index eb9dbdcca22..a5c43b67c8a 100644 --- a/src/server/database/Database/QueryHolder.cpp +++ b/src/server/database/Database/QueryHolder.cpp @@ -15,10 +15,11 @@ * with this program. If not, see . */ -#include "MySQLConnection.h" #include "QueryHolder.h" -#include "PreparedStatement.h" +#include "Errors.h" #include "Log.h" +#include "MySQLConnection.h" +#include "PreparedStatement.h" #include "QueryResult.h" bool SQLQueryHolderBase::SetPreparedQueryImpl(size_t index, PreparedStatementBase* stmt) @@ -33,13 +34,13 @@ bool SQLQueryHolderBase::SetPreparedQueryImpl(size_t index, PreparedStatementBas return true; } -PreparedQueryResult SQLQueryHolderBase::GetPreparedResult(size_t index) +PreparedQueryResult SQLQueryHolderBase::GetPreparedResult(size_t index) const { // Don't call to this function if the index is of a prepared statement - if (index < m_queries.size()) - return m_queries[index].second; - else - return PreparedQueryResult(nullptr); + ASSERT(index < m_queries.size(), "Query holder result index out of range, tried to access index " SZFMTD " but there are only " SZFMTD " results", + index, m_queries.size()); + + return m_queries[index].second; } void SQLQueryHolderBase::SetPreparedResult(size_t index, PreparedResultSet* result) @@ -71,25 +72,16 @@ void SQLQueryHolderBase::SetSize(size_t size) m_queries.resize(size); } -SQLQueryHolderTask::~SQLQueryHolderTask() -{ - if (!m_executed) - delete m_holder; -} +SQLQueryHolderTask::~SQLQueryHolderTask() = default; bool SQLQueryHolderTask::Execute() { - m_executed = true; - - if (!m_holder) - return false; - /// execute all queries in the holder and pass the results for (size_t i = 0; i < m_holder->m_queries.size(); ++i) if (PreparedStatementBase* stmt = m_holder->m_queries[i].first) m_holder->SetPreparedResult(i, m_conn->Query(stmt)); - m_result.set_value(m_holder); + m_result.set_value(); return true; } @@ -97,7 +89,7 @@ bool SQLQueryHolderCallback::InvokeIfReady() { if (m_future.valid() && m_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) { - m_callback(m_future.get()); + m_callback(*m_holder); return true; } diff --git a/src/server/database/Database/QueryHolder.h b/src/server/database/Database/QueryHolder.h index f94eff49d2c..25687fedd81 100644 --- a/src/server/database/Database/QueryHolder.h +++ b/src/server/database/Database/QueryHolder.h @@ -19,6 +19,7 @@ #define _QUERYHOLDER_H #include "SQLOperation.h" +#include class TC_DATABASE_API SQLQueryHolderBase { @@ -26,10 +27,10 @@ class TC_DATABASE_API SQLQueryHolderBase private: std::vector> m_queries; public: - SQLQueryHolderBase() { } + SQLQueryHolderBase() = default; virtual ~SQLQueryHolderBase(); void SetSize(size_t size); - PreparedQueryResult GetPreparedResult(size_t index); + PreparedQueryResult GetPreparedResult(size_t index) const; void SetPreparedResult(size_t index, PreparedResultSet* result); protected: @@ -49,13 +50,12 @@ public: class TC_DATABASE_API SQLQueryHolderTask : public SQLOperation { private: - SQLQueryHolderBase* m_holder; + std::shared_ptr m_holder; QueryResultHolderPromise m_result; - bool m_executed; public: - SQLQueryHolderTask(SQLQueryHolderBase* holder) - : m_holder(holder), m_executed(false) { } + explicit SQLQueryHolderTask(std::shared_ptr holder) + : m_holder(std::move(holder)) { } ~SQLQueryHolderTask(); @@ -66,20 +66,23 @@ class TC_DATABASE_API SQLQueryHolderTask : public SQLOperation class TC_DATABASE_API SQLQueryHolderCallback { public: - SQLQueryHolderCallback(QueryResultHolderFuture&& future) : m_future(std::move(future)) { } + SQLQueryHolderCallback(std::shared_ptr&& holder, QueryResultHolderFuture&& future) + : m_holder(std::move(holder)), m_future(std::move(future)) { } + SQLQueryHolderCallback(SQLQueryHolderCallback&&) = default; SQLQueryHolderCallback& operator=(SQLQueryHolderCallback&&) = default; - void AfterComplete(std::function callback) & + void AfterComplete(std::function callback) & { m_callback = std::move(callback); } bool InvokeIfReady(); + std::shared_ptr m_holder; QueryResultHolderFuture m_future; - std::function m_callback; + std::function m_callback; }; #endif diff --git a/src/server/database/Database/SQLOperation.h b/src/server/database/Database/SQLOperation.h index 97f1b396cea..2e85f72c17c 100644 --- a/src/server/database/Database/SQLOperation.h +++ b/src/server/database/Database/SQLOperation.h @@ -42,13 +42,6 @@ struct SQLElementData SQLElementDataType type; }; -//- For ambigious resultsets -union SQLResultSetUnion -{ - PreparedResultSet* presult; - ResultSet* qresult; -}; - class MySQLConnection; class TC_DATABASE_API SQLOperation -- cgit v1.2.3