Core/DBLayer: Proper core-side handling of MySQL errno 1213 to prevent a snowballeffect (until innodb_lock_wait_timeout)

src/server/game/Server/Protocol/Handlers/MailHandler.cpp

@@ -256,12 +256,8 @@ void WorldSession::HandleSendMail(WorldPacket & recv_data)
                 pl->MoveItemFromInventory(items[i]->GetBagSlot(), item->GetSlot(), true);
 
                 item->DeleteFromInventoryDB(trans);     // deletes item from character's inventory
+                item->SetOwnerGUID(rc);
                 item->SaveToDB(trans);                  // recursive and not have transaction guard into self, item not in inventory and can be save standalone
-                // owner in data will set at mail receive and item extracting
-                PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_SET_ITEM_OWNER);
-                stmt->setUInt32(0, GUID_LOPART(rc));
-                stmt->setUInt32(1, item->GetGUIDLow());
-                trans->Append(stmt);
 
                 draft.AddItem(item);
             }

src/server/shared/Database/DatabaseWorkerPool.h

@@ -370,6 +370,9 @@ class DatabaseWorkerPool
                 }
             }
 
+            // Clean up now.
+            transaction->Cleanup();
+
             con->Unlock();
         }
 

src/server/shared/Database/MySQLConnection.cpp

@@ -379,14 +379,16 @@ void MySQLConnection::CommitTransaction()
 
 bool MySQLConnection::ExecuteTransaction(SQLTransaction& transaction)
 {
-    std::queue<SQLElementData> &queries = transaction->m_queries;
+    std::list<SQLElementData> const& queries = transaction->m_queries;
     if (queries.empty())
         return false;
 
     BeginTransaction();
-    while (!queries.empty())
+
+    std::list<SQLElementData>::const_iterator itr;
+    for (itr = queries.begin(); itr != queries.end(); ++itr);
     {
-        SQLElementData data = queries.front();
+        SQLElementData const& data = *itr;
         switch (data.type)
         {
             case SQL_ELEMENT_PREPARED:
@@ -399,7 +401,6 @@ bool MySQLConnection::ExecuteTransaction(SQLTransaction& transaction)
                     RollbackTransaction();
                     return false;
                 }
-                delete data.element.stmt;
             }
             break;
             case SQL_ELEMENT_RAW:
@@ -412,13 +413,16 @@ bool MySQLConnection::ExecuteTransaction(SQLTransaction& transaction)
                     RollbackTransaction();
                     return false;
                 }
-                free((void*)const_cast<char*>(sql));
             }
             break;
         }
-        queries.pop();
     }
 
+    // we might encounter errors during certain queries, and depending on the kind of error
+    // we might want to restart the transaction. So to prevent data loss, we only clean up when it's all done.
+    // This is done in calling functions DatabaseWorkerPool<T>::DirectCommitTransaction and TransactionTask::Execute,
+    // and not while iterating over every element.
+ 
     CommitTransaction();
     return true;
 }
@@ -491,8 +495,6 @@ PreparedResultSet* MySQLConnection::Query(PreparedStatement* stmt)
 
 bool MySQLConnection::_HandleMySQLErrno(uint32 errNo)
 {
-    sLog->outSQLDriver("%s", __FUNCTION__);
-
     switch (errNo)
     {
         case 2006:  // "MySQL server has gone away"
@@ -521,8 +523,7 @@ bool MySQLConnection::_HandleMySQLErrno(uint32 errNo)
         }
 
         case 1213:      // "Deadlock found when trying to get lock; try restarting transaction"
-            return true;    // Implemented in TransactionTask::Execute and DatabaseWorkerPool<T>::DirectCommitTransaction
-
+            return false;    // Implemented in TransactionTask::Execute and DatabaseWorkerPool<T>::DirectCommitTransaction
         // Query related errors - skip query
         case 1058:      // "Column count doesn't match value count"
         case 1062:      // "Duplicate entry '%s' for key '%d'"

src/server/shared/Database/Transaction.cpp

@@ -24,7 +24,7 @@ void Transaction::Append(const char* sql)
     SQLElementData data;
     data.type = SQL_ELEMENT_RAW;
     data.element.query = strdup(sql);
-    m_queries.push(data);
+    m_queries.push_back(data);
 }
 
 void Transaction::PAppend(const char* sql, ...)
@@ -44,14 +44,18 @@ void Transaction::Append(PreparedStatement* stmt)
     SQLElementData data;
     data.type = SQL_ELEMENT_PREPARED;
     data.element.stmt = stmt;
-    m_queries.push(data);
+    m_queries.push_back(data);
 }
 
 void Transaction::Cleanup()
 {
+    // This might be called by explicit calls to Cleanup or by the auto-destructor
+    if (_cleanedUp) 
+        return;
+
     while (!m_queries.empty())
     {
-        SQLElementData data = m_queries.front();
+        SQLElementData const &data = m_queries.front();
         switch (data.type)
         {
             case SQL_ELEMENT_PREPARED:
@@ -61,8 +65,11 @@ void Transaction::Cleanup()
                 free((void*)(data.element.query));
             break;
         }
-        m_queries.pop();
+
+        m_queries.pop_front();
     }
+
+    _cleanedUp = true;
 }
 
 bool TransactionTask::Execute()
@@ -78,5 +85,8 @@ bool TransactionTask::Execute()
                 return true;
     }
 
+    // Clean up now.
+    m_trans->Cleanup();
+
     return false;
 }

src/server/shared/Database/Transaction.h

@@ -30,7 +30,7 @@ class Transaction
     friend class MySQLConnection;
 
     public:
-        Transaction() {}
+        Transaction() : _cleanedUp(false) {}
         ~Transaction() { Cleanup(); }
 
         void Append(PreparedStatement* statement);
@@ -41,7 +41,10 @@ class Transaction
 
     protected:
         void Cleanup();
-        std::queue<SQLElementData> m_queries;
+        std::list<SQLElementData> m_queries;
+
+    private:
+        bool _cleanedUp;
 
 };
 typedef ACE_Refcounted_Auto_Ptr<Transaction, ACE_Null_Mutex> SQLTransaction;