[8328] Fixed problem with crash at startup in result destroy anti-freeze thread runnable. Author: VladimirMangos

    * Destroy runnable only if no references.
    * Some code cleanups

--HG--
branch : trunk

4 files changed, 49 insertions, 17 deletions

diff --git a/src/shared/Threading.cpp b/src/shared/Threading.cpp
index 25bd3120c4a..652f468772f 100644
--- a/src/shared/Threading.cpp
+++ b/src/shared/Threading.cpp
@@ -103,6 +103,10 @@ Thread::Thread() : m_task(0), m_iThreadId(0), m_hThreadHandle(0)
 
 Thread::Thread(Runnable* instance) : m_task(instance), m_iThreadId(0), m_hThreadHandle(0)
 {
+    // register reference to m_task to prevent it deeltion until destructor
+    if (m_task)
+        m_task->incReference();
+
     bool _start = start();
     ASSERT (_start);
 }
@@ -111,8 +115,9 @@ Thread::~Thread()
 {
     //Wait();
 
-    // deleted runnable object (owned by Thread)
-    delete m_task;
+    // deleted runnable object (if no other references)
+    if (m_task)
+        m_task->decReference();
 }
 
 //initialize Thread's class static member
@@ -121,15 +126,20 @@ ThreadPriority Thread::m_TpEnum;
 
 bool Thread::start()
 {
-    if(m_task == 0 || m_iThreadId != 0)
+    if (m_task == 0 || m_iThreadId != 0)
         return false;
 
-    return (ACE_Thread::spawn(&Thread::ThreadTask, (void*)m_task, THREADFLAG, &m_iThreadId, &m_hThreadHandle) == 0);
+    bool res = (ACE_Thread::spawn(&Thread::ThreadTask, (void*)m_task, THREADFLAG, &m_iThreadId, &m_hThreadHandle) == 0);
+
+    if (res)
+        m_task->incReference();
+
+    return res;
 }
 
 bool Thread::wait()
 {
-    if(!m_hThreadHandle || !m_task)
+    if (!m_hThreadHandle || !m_task)
         return false;
 
     ACE_THR_FUNC_RETURN _value = ACE_THR_FUNC_RETURN(-1);
@@ -143,7 +153,17 @@ bool Thread::wait()
 
 void Thread::destroy()
 {
-    ACE_Thread::kill(m_iThreadId, -1);
+    if (!m_iThreadId || !m_task)
+        return;
+
+    if (ACE_Thread::kill(m_iThreadId, -1) != 0)
+        return;
+
+    m_iThreadId = 0;
+    m_hThreadHandle = 0;
+
+    // reference set at ACE_Thread::spawn
+    m_task->decReference();
 }
 
 void Thread::suspend()
@@ -161,6 +181,9 @@ ACE_THR_FUNC_RETURN Thread::ThreadTask(void * param)
     Runnable * _task = (Runnable*)param;
     _task->run();
 
+    // task execution complete, free referecne added at 
+    _task->decReference();
+
     return (ACE_THR_FUNC_RETURN)0;
 }
 
diff --git a/src/shared/Threading.h b/src/shared/Threading.h
index ab423696cd1..6c3f3724bc1 100644
--- a/src/shared/Threading.h
+++ b/src/shared/Threading.h
@@ -21,6 +21,7 @@
 
 #include <ace/Thread.h>
 #include <ace/TSS_T.h>
+#include "ace/Atomic_Op.h"
 #include <assert.h>
 #include "Errors.h"
 
@@ -32,6 +33,15 @@ namespace ACE_Based
         public:
             virtual ~Runnable() {}
             virtual void run() = 0;
+
+            void incReference() { ++m_refs; }
+            void decReference()
+            {
+                if(!--m_refs)
+                    delete this;
+            }
+        private:
+            ACE_Atomic_Op<ACE_Thread_Mutex, int> m_refs;
     };
 
     enum Priority
diff --git a/src/trinitycore/Master.cpp b/src/trinitycore/Master.cpp
index b9bf375628b..76fdbd661de 100644
--- a/src/trinitycore/Master.cpp
+++ b/src/trinitycore/Master.cpp
@@ -238,8 +238,8 @@ int Master::Run()
     _HookSignals();
 
     ///- Launch WorldRunnable thread
-    ACE_Based::Thread t(new WorldRunnable);
-    t.setPriority(ACE_Based::Highest);
+    ACE_Based::Thread world_thread(new WorldRunnable);
+    world_thread.setPriority(ACE_Based::Highest);
 
     // set server online
     loginDatabase.PExecute("UPDATE realmlist SET color = 0, population = 0 WHERE id = '%d'",realmID);
@@ -256,7 +256,7 @@ int Master::Run()
         cliThread = new ACE_Based::Thread(new CliRunnable);
     }
 
-    ACE_Based::Thread td2(new RARunnable);
+    ACE_Based::Thread rar_thread(new RARunnable);
 
     ///- Handle affinity for multiple processors and process priority on Windows
     #ifdef WIN32
@@ -312,13 +312,12 @@ int Master::Run()
     uint32 loopCounter = 0;
 
     ///- Start up freeze catcher thread
-    uint32 freeze_delay = sConfig.GetIntDefault("MaxCoreStuckTime", 0);
-    if(freeze_delay)
+    if(uint32 freeze_delay = sConfig.GetIntDefault("MaxCoreStuckTime", 0))
     {
         FreezeDetectorRunnable *fdr = new FreezeDetectorRunnable();
         fdr->SetDelayTime(freeze_delay*1000);
-        ACE_Based::Thread t(fdr);
-        t.setPriority(ACE_Based::Highest);
+        ACE_Based::Thread freeze_thread(fdr);
+        freeze_thread.setPriority(ACE_Based::Highest);
     }
 
     ///- Launch the world listener socket
@@ -342,8 +341,8 @@ int Master::Run()
 
     // when the main thread closes the singletons get unloaded
     // since worldrunnable uses them, it will crash if unloaded after master
-    t.wait();
-    td2.wait ();
+    world_thread.wait();
+    rar_thread.wait ();
 
     ///- Clean database before leaving
     clearOnlineAccounts();
@@ -355,7 +354,7 @@ int Master::Run()
 
     sLog.outString( "Halting process..." );
 
-    if(cliThread)
+    if (cliThread)
     {
         #ifdef WIN32
 
diff --git a/src/trinityrealm/AuthSocket.h b/src/trinityrealm/AuthSocket.h
index 43b880a1e74..ecfdd159000 100644
--- a/src/trinityrealm/AuthSocket.h
+++ b/src/trinityrealm/AuthSocket.h
@@ -60,7 +60,7 @@ class AuthSocket: public TcpSocket
         void _SetVSFields(const std::string& rI);
 
         FILE *pPatch;
-    ACE_Thread_Mutex patcherLock;
+        ACE_Thread_Mutex patcherLock;
         bool IsLag();
 
     private: