[Opalvoip-svn] SF.net SVN: opalvoip:[34840] ptlib/trunk

[<rjo...@us...>]

Revision: 34840
          http://sourceforge.net/p/opalvoip/code/34840
Author:   rjongbloed
Date:     2016-05-27 06:18:43 +0000 (Fri, 27 May 2016)
Log Message:
-----------
Merged revision(s) 34834-34839 from ptlib/branches/v2_16:
Added PThread::WaitAndDelete() to handle thread termination management which is very common repeated code. Also, to aid debugging, this new function will, when termination exceeds a timeout, do stack walks of the thread being terminated and the thread waiting on termination.

Modified Paths:
--------------
    ptlib/trunk/include/ptlib/thread.h
    ptlib/trunk/src/ptclib/cli.cxx
    ptlib/trunk/src/ptclib/vxml.cxx
    ptlib/trunk/src/ptlib/common/osutils.cxx
    ptlib/trunk/src/ptlib/msos/vfw.cxx

Property Changed:
----------------
    ptlib/trunk/

Index: ptlib/trunk
===================================================================
--- ptlib/trunk	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk	2016-05-27 06:18:43 UTC (rev 34840)

Property changes on: ptlib/trunk
___________________________________________________________________
Modified: svn:mergeinfo
## -6,7 +6,7 ##
 /ptlib/branches/v2_10:25177-29189,32921,32947
 /ptlib/branches/v2_12:28485-31603
 /ptlib/branches/v2_14:31501-33720
-/ptlib/branches/v2_16:34085-34833
+/ptlib/branches/v2_16:34085-34839
 /ptlib/branches/v2_2:20746,20791,20827,22014,22942
 /ptlib/branches/v2_4:21086,21094,21147,21160,21185,21281,21296,21305,21322,21337,21363,21467,21471-21472,21506,21508,21623,21695,21744,21746,21763,22241,22958,23045-23046,23061,23066,23712
 /ptlib/branches/v2_6:22195,22243,22295,22304,22311,22317,22320,22356,22458,22509,22587,22601-22602,22611,22629,22633,22673,22681,22729,22731-22732,22736,22742,22848,22960,22992,23161,23163,23167,23169,23177,23239,23291,23298,23336,23429,23595,23823,23827,23873,24816
\ No newline at end of property
Modified: ptlib/trunk/include/ptlib/thread.h
===================================================================
--- ptlib/trunk/include/ptlib/thread.h	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk/include/ptlib/thread.h	2016-05-27 06:18:43 UTC (rev 34840)
@@ -187,6 +187,27 @@
       const PTimeInterval & maxWait  ///< Maximum time to wait for termination.
     ) const;
 
+    /**Wait for thread termination and delete object.
+       The thread pointer is set to NULL within an optional mutex. The mutex is
+       released before waiting for thread termination, which avoids deadlock
+       scenarios. The \p lock parameter indicates if the mutex is already locked
+       on entry and should not be locked again, preventing the unlock from
+       occurring due to nested mutex semantics.
+
+       If the \p maxTime timeout occurs, the thread is preemptively destroyed
+       which is very likely to cause many issues, see PThread::Terminate() so
+       a PAssert is raised when this condition occurs.
+
+       @return
+       <code>true</code> if the thread forcibly terminated, <code>false</code> if orderly exit.
+      */
+    static bool WaitAndDelete(
+      PThread * & thread,                    ///< Thread to delete
+      const PTimeInterval & maxWait = 10000, ///< Maximum time to wait for termination.
+      PMutex * mutex = NULL,                 ///< Optional mutex to protect setting of thread variable
+      bool lock = true                       ///< Mutex should be locked.
+    );
+
     /** Suspend or resume the thread.
     
        If <code>susp</code> is <code>true</code> this increments an internal count of

Modified: ptlib/trunk/src/ptclib/cli.cxx
===================================================================
--- ptlib/trunk/src/ptclib/cli.cxx	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk/src/ptclib/cli.cxx	2016-05-27 06:18:43 UTC (rev 34840)
@@ -125,12 +125,7 @@
   PChannel * base = GetBaseReadChannel();
   if (base != NULL)
     base->Close();
-
-  if (m_thread != NULL && PThread::Current() != m_thread) {
-    m_thread->WaitForTermination(10000);
-    delete m_thread;
-    m_thread = NULL;
-  }
+  PThread::WaitAndDelete(m_thread);
 }
 
 
@@ -1117,13 +1112,8 @@
 void PCLISocket::Stop()
 {
   m_listenSocket.Close();
+  PThread::WaitAndDelete(m_thread);
 
-  if (m_thread != NULL && PThread::Current() != m_thread) {
-    m_thread->WaitForTermination(10000);
-    delete m_thread;
-    m_thread = NULL;
-  }
-
   PCLI::Stop();
 }
 

Modified: ptlib/trunk/src/ptclib/vxml.cxx
===================================================================
--- ptlib/trunk/src/ptclib/vxml.cxx	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk/src/ptclib/vxml.cxx	2016-05-27 06:18:43 UTC (rev 34840)
@@ -1095,30 +1095,15 @@
 
 PBoolean PVXMLSession::Close()
 {
-  PThread * thread = NULL;
-
   m_sessionMutex.Wait();
 
   LoadGrammar(NULL);
 
-  if (PThread::Current() != m_vxmlThread) {
-    thread = m_vxmlThread;
-    m_vxmlThread = NULL;
-  }
+  // Stop condition for thread
+  m_abortVXML = true;
+  Trigger();
+  PThread::WaitAndDelete(m_vxmlThread, 10000, &m_sessionMutex, false);
 
-  m_sessionMutex.Signal();
-
-  if (thread != NULL) {
-    PTRACE(3, "VXML\tClosing session, fast forwarding through script");
-
-    // Stop condition for thread
-    m_abortVXML = true;
-    Trigger();
-
-    PAssert(thread->WaitForTermination(10000), "VXML thread did not exit in time.");
-    delete thread;
-  }
-
   return PIndirectChannel::Close();
 }
 

Modified: ptlib/trunk/src/ptlib/common/osutils.cxx
===================================================================
--- ptlib/trunk/src/ptlib/common/osutils.cxx	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk/src/ptlib/common/osutils.cxx	2016-05-27 06:18:43 UTC (rev 34840)
@@ -2817,6 +2817,55 @@
 }
 
 
+bool PThread::WaitAndDelete(PThread * & threadToDelete, const PTimeInterval & maxWait, PMutex * mutex, bool lock)
+{
+  if (mutex != NULL && lock)
+    mutex->Wait();
+
+  PThread * thread = threadToDelete;
+  threadToDelete = NULL;
+
+  if (mutex != NULL)
+    mutex->Signal();
+
+  if (thread == NULL)
+    return false;
+
+  if (PThread::Current() == thread) {
+    // Is me!
+    thread->SetAutoDelete();
+    return false;
+  }
+
+  if (thread->IsSuspended()) {
+    // Ended before it started
+    delete thread;
+    return false;
+  }
+
+  PTRACE(4, thread, "Waiting for thread " << *thread << " to terminate in " << maxWait << " seconds");
+  if (thread->WaitForTermination(maxWait)) {
+    // Orderly exit
+    delete thread;
+    return false;
+  }
+
+  ostringstream strm;
+  strm << "Thread \"" << *thread << "\""
+#if PTRACING
+          "\n";
+  PTrace::WalkStack(strm, thread->GetThreadId());
+  strm << "  "
+#endif
+          " failed to terminate in " << maxWait << " seconds";
+  PAssertAlways(strm.str().c_str());
+
+  delete thread;
+
+  return true;
+}
+
+
 #define RELEASE_THREAD_LOCAL_STORAGE 1
 #if RELEASE_THREAD_LOCAL_STORAGE
 static std::set<PThread::LocalStorageBase*> s_ThreadLocalStorage;

Modified: ptlib/trunk/src/ptlib/msos/vfw.cxx
===================================================================
--- ptlib/trunk/src/ptlib/msos/vfw.cxx	2016-05-27 06:09:28 UTC (rev 34839)
+++ ptlib/trunk/src/ptlib/msos/vfw.cxx	2016-05-27 06:18:43 UTC (rev 34840)
@@ -1399,11 +1399,7 @@
 
   SendMessage(m_hWnd, WM_CLOSE, 0, 0);
 
-  if (m_thread != NULL) {
-    m_thread->WaitForTermination(3000);
-    delete m_thread;
-    m_thread = NULL;
-  }
+  PThread::WaitAndDelete(m_thread, 3000);
 
   return true;
 }

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.