More robust code in synchronization primitives and in JackMessageBuffer.

git-svn-id: http://subversion.jackaudio.org/jack/jack2/trunk/jackmp@4526 0c269be4-1314-0410-8aa9-9f06e86f4224

ChangeLog

@@ -35,6 +35,10 @@ Chris Caudle
   Jackdmp changes log
 ---------------------------
 
+2011-06-26 Stephane Letz  <letz@grame.fr>
+
+	* More robust code in synchronization primitives and in JackMessageBuffer.
+
 2011-07-29 Stephane Letz  <letz@grame.fr>
 
 	* New JackTimedDriver class to be used by JackDummyDriver, JackNetDriver and JackNetOneDriver classes.

common/JackAudioAdapterFactory.cpp

@@ -53,7 +53,7 @@ extern "C"
         Jack::JackAudioAdapter* adapter;
         jack_nframes_t buffer_size = jack_get_buffer_size(jack_client);
         jack_nframes_t sample_rate = jack_get_sample_rate(jack_client);
-        
+
         try {
 
     #ifdef __linux__
@@ -71,17 +71,16 @@ extern "C"
     #if defined(__sun__) || defined(sun)
             adapter = new Jack::JackAudioAdapter(jack_client, new Jack::JackOSSAdapter(buffer_size, sample_rate, params));
     #endif
-   
-           assert(adapter);
-           
-            if (adapter->Open() == 0)
+
+            assert(adapter);
+
+            if (adapter->Open() == 0) {
                 return 0;
-            else
-            {
+            } else {
                 delete adapter;
                 return 1;
             }
-         
+
         } catch (...) {
             return 1;
         }
@@ -94,13 +93,14 @@ extern "C"
         int res = 1;
         jack_driver_desc_t* desc = jack_get_descriptor();
 
-        Jack::JackArgParser parser ( load_init );
-        if ( parser.GetArgc() > 0 )
-            parse_params = parser.ParseParams ( desc, &params );
+        Jack::JackArgParser parser(load_init);
+        if (parser.GetArgc() > 0) {
+            parse_params = parser.ParseParams(desc, &params);
+        }
 
         if (parse_params) {
-            res = jack_internal_initialize ( jack_client, params );
-            parser.FreeParams ( params );
+            res = jack_internal_initialize(jack_client, params);
+            parser.FreeParams(params);
         }
         return res;
     }

common/JackException.h

@@ -29,6 +29,13 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 namespace Jack
 {
 
+#define	ThrowIf(inCondition, inException)                                               \
+			if(inCondition)																\
+			{																			\
+				throw(inException);														\
+			}
+
+
 /*!
 \brief Exception base class.
 */
@@ -52,8 +59,9 @@ class SERVER_EXPORT JackException : public std::runtime_error {
         void PrintMessage()
         {
             std::string str = what();
-            if (str != "")
+            if (str != "") {
                 jack_info(str.c_str());
+            }
         }
 };
 
@@ -62,9 +70,9 @@ class SERVER_EXPORT JackException : public std::runtime_error {
  */
 
 class SERVER_EXPORT JackTemporaryException : public JackException {
-    
+
     public:
-        
+
         JackTemporaryException(const std::string& msg) : JackException(msg)
         {}
         JackTemporaryException(char* msg) : JackException(msg)
@@ -74,15 +82,15 @@ class SERVER_EXPORT JackTemporaryException : public JackException {
         JackTemporaryException() : JackException("")
         {}
 };
-    
+
 /*!
- \brief 
+ \brief
  */
 
 class SERVER_EXPORT JackQuitException : public JackException {
-    
+
     public:
-        
+
         JackQuitException(const std::string& msg) : JackException(msg)
         {}
         JackQuitException(char* msg) : JackException(msg)
@@ -92,7 +100,7 @@ class SERVER_EXPORT JackQuitException : public JackException {
         JackQuitException() : JackException("")
         {}
 };
-    
+
 /*!
 \brief Exception possibly thrown by Net slaves.
 */

common/JackMessageBuffer.cpp

@@ -48,11 +48,16 @@ void JackMessageBuffer::Stop()
     } else {
         jack_log("no message buffer overruns");
     }
-    fGuard.Lock();
-    fRunning = false;
-    fGuard.Signal();
-    fGuard.Unlock();
-    fThread.Stop();
+
+    if (fGuard.Lock()) {
+        fRunning = false;
+        fGuard.Signal();
+        fGuard.Unlock();
+        fThread.Stop();
+    } else {
+        fThread.Kill();
+    }
+
     Flush();
 }
 
@@ -80,19 +85,22 @@ void JackMessageBuffer::AddMessage(int level, const char *message)
 bool JackMessageBuffer::Execute()
 {
     while (fRunning) {
-        fGuard.Lock();
-        fGuard.Wait();
-        /* the client asked for all threads to run a thread
-        initialization callback, which includes us.
-        */
-        if (fInit) {
-            fInit(fInitArg);
-            fInit = NULL;
-            /* and we're done */
-            fGuard.Signal();
+        if (fGuard.Lock()) {
+            fGuard.Wait();
+            /* the client asked for all threads to run a thread
+            initialization callback, which includes us.
+            */
+            if (fInit) {
+                fInit(fInitArg);
+                fInit = NULL;
+                /* and we're done */
+                fGuard.Signal();
+            }
+            Flush();
+            fGuard.Unlock();
+        } else {
+            jack_error("JackMessageBuffer::Execute lock cannot be taken");
         }
-        Flush();
-        fGuard.Unlock();
     }
     return false;
 }
@@ -126,16 +134,19 @@ void JackMessageBufferAdd(int level, const char *message)
 
 void JackMessageBuffer::SetInitCallback(JackThreadInitCallback callback, void *arg)
 {
-    fGuard.Lock();
-    /* set up the callback */
-    fInitArg = arg;
-    fInit = callback;
-    /* wake msg buffer thread */
-    fGuard.Signal();
-    /* wait for it to be done */
-    fGuard.Wait();
-    /* and we're done */
-    fGuard.Unlock();
+    if (fGuard.Lock()) {
+        /* set up the callback */
+        fInitArg = arg;
+        fInit = callback;
+        /* wake msg buffer thread */
+        fGuard.Signal();
+        /* wait for it to be done */
+        fGuard.Wait();
+        /* and we're done */
+        fGuard.Unlock();
+    } else {
+        jack_error("JackMessageBuffer::SetInitCallback lock cannot be taken");
+    }
 }
 
 };

common/JackMessageBuffer.h

@@ -10,19 +10,19 @@
  *  Copyright (C) 2004 Rui Nuno Capela, Steve Harris
  *  Copyright (C) 2008 Nedko Arnaudov
  *  Copyright (C) 2008 Grame
- *  
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU Lesser General Public License as published by
  *  the Free Software Foundation; either version 2.1 of the License, or
  *  (at your option) any later version.
- *  
+ *
  *  This program is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU Lesser General Public License for more details.
- *  
+ *
  *  You should have received a copy of the GNU Lesser General Public License
- *  along with this program; if not, write to the Free Software 
+ *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *
  */
@@ -56,7 +56,7 @@ class JackMessageBuffer : public JackRunnableInterface
 {
 
     private:
-    
+
         JackThreadInitCallback fInit;
         void* fInitArg;
         JackMessage fBuffers[MB_BUFFERS];
@@ -68,15 +68,15 @@ class JackMessageBuffer : public JackRunnableInterface
         bool fRunning;
 
         void Flush();
-    
+
         void Start();
         void Stop();
- 	    
+
     public:
-    
-        JackMessageBuffer();     
+
+        JackMessageBuffer();
         ~JackMessageBuffer();
-         
+
         // JackRunnableInterface interface
         bool Execute();
 
@@ -90,16 +90,16 @@ class JackMessageBuffer : public JackRunnableInterface
 };
 
 #ifdef __cplusplus
-extern "C" 
+extern "C"
 {
 #endif
 
 void JackMessageBufferAdd(int level, const char *message);
 
 #ifdef __cplusplus
-} 
+}
 #endif
 
 };
 
-#endif 
+#endif

posix/JackPosixMutex.h

@@ -22,10 +22,11 @@
 #ifndef __JackPosixMutex__
 #define __JackPosixMutex__
 
+#include "JackError.h"
+#include "JackException.h"
 #include <pthread.h>
 #include <stdio.h>
 #include <assert.h>
-#include "JackError.h"
 
 namespace Jack
 {
@@ -33,19 +34,20 @@ namespace Jack
 \brief Mutex abstraction.
 */
 
-
 class JackBasePosixMutex
 {
 
     protected:
 
         pthread_mutex_t fMutex;
+        pthread_t fOwner;
 
     public:
 
-        JackBasePosixMutex()
+        JackBasePosixMutex():fOwner(0)
         {
-            pthread_mutex_init(&fMutex, NULL);
+            int res = pthread_mutex_init(&fMutex, NULL);
+            ThrowIf(res != 0, JackException("JackBasePosixMutex: could not init the mutex"));
         }
 
         virtual ~JackBasePosixMutex()
@@ -53,30 +55,65 @@ class JackBasePosixMutex
             pthread_mutex_destroy(&fMutex);
         }
 
-        void Lock()
+        bool Lock()
         {
-            int res = pthread_mutex_lock(&fMutex);
-            if (res != 0)
-                jack_log("JackBasePosixMutex::Lock res = %d", res);
+            pthread_t current_thread = pthread_self();
+
+            if (!pthread_equal(current_thread, fOwner)) {
+                int res = pthread_mutex_lock(&fMutex);
+                if (res == 0) {
+                    fOwner = current_thread;
+                    return true;
+                } else {
+                    jack_error("JackBasePosixMutex::Lock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackBasePosixMutex::Lock mutex already locked by thread = %d", current_thread);
+                return false;
+            }
         }
 
         bool Trylock()
         {
-            return (pthread_mutex_trylock(&fMutex) == 0);
+            pthread_t current_thread = pthread_self();
+
+            if (!pthread_equal(current_thread, fOwner)) {
+                int res = pthread_mutex_trylock(&fMutex);
+                if (res == 0) {
+                    fOwner = current_thread;
+                    return true;
+                } else {
+                    jack_error("JackBasePosixMutex::Trylock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackBasePosixMutex::Trylock mutex already locked by thread = %d", current_thread);
+                return false;
+            }
         }
 
-        void Unlock()
+        bool Unlock()
         {
-            int res = pthread_mutex_unlock(&fMutex);
-            if (res != 0)
-                jack_log("JackBasePosixMutex::Unlock res = %d", res);
+            if (pthread_equal(pthread_self(), fOwner)) {
+                fOwner = 0;
+                int res = pthread_mutex_unlock(&fMutex);
+                if (res == 0) {
+                    return true;
+                } else {
+                    jack_error("JackBasePosixMutex::Unlock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackBasePosixMutex::Unlock mutex not locked by thread = %d owner %d", pthread_self(), fOwner);
+                return false;
+            }
         }
 
 };
 
 class JackPosixMutex
 {
-
     protected:
 
         pthread_mutex_t fMutex;
@@ -89,13 +126,12 @@ class JackPosixMutex
             pthread_mutexattr_t mutex_attr;
             int res;
             res = pthread_mutexattr_init(&mutex_attr);
-            assert(res == 0);
+            ThrowIf(res != 0, JackException("JackBasePosixMutex: could not init the mutex attribute"));
             res = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE);
-            assert(res == 0);
+            ThrowIf(res != 0, JackException("JackBasePosixMutex: could not settype the mutex"));
             res = pthread_mutex_init(&fMutex, &mutex_attr);
-            assert(res == 0);
-            res = pthread_mutexattr_destroy(&mutex_attr);
-            assert(res == 0);
+            ThrowIf(res != 0, JackException("JackBasePosixMutex: could not init the mutex"));
+            pthread_mutexattr_destroy(&mutex_attr);
         }
 
         virtual ~JackPosixMutex()
@@ -106,8 +142,9 @@ class JackPosixMutex
         bool Lock()
         {
             int res = pthread_mutex_lock(&fMutex);
-            if (res != 0)
+            if (res != 0) {
                 jack_log("JackPosixMutex::Lock res = %d", res);
+            }
             return (res == 0);
         }
 
@@ -119,8 +156,9 @@ class JackPosixMutex
         bool Unlock()
         {
             int res = pthread_mutex_unlock(&fMutex);
-            if (res != 0)
+            if (res != 0) {
                 jack_log("JackPosixMutex::Unlock res = %d", res);
+            }
             return (res == 0);
         }
 

posix/JackProcessSync.cpp

@@ -12,7 +12,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU Lesser General Public License for more details.
 
 You should have received a copy of the GNU Lesser General Public License
-along with this program; if not, write to the Free Software 
+along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
 */
@@ -30,6 +30,7 @@ void JackProcessSync::Signal()
         jack_error("JackProcessSync::Signal error err = %s", strerror(res));
 }
 
+// TO DO : check thread consistency?
 void JackProcessSync::LockedSignal()
 {
     int res = pthread_mutex_lock(&fMutex);
@@ -50,6 +51,7 @@ void JackProcessSync::SignalAll()
         jack_error("JackProcessSync::SignalAll error err = %s", strerror(res));
 }
 
+// TO DO : check thread consistency?
 void JackProcessSync::LockedSignalAll()
 {
     int res = pthread_mutex_lock(&fMutex);
@@ -65,11 +67,18 @@ void JackProcessSync::LockedSignalAll()
 
 void JackProcessSync::Wait()
 {
-    int res;
-    if ((res = pthread_cond_wait(&fCond, &fMutex)) != 0)
+    ThrowIf(!pthread_equal(pthread_self(), fOwner), JackException("JackProcessSync::Wait: a thread has to have locked a mutex before it can wait"));
+    fOwner = 0;
+
+    int res = pthread_cond_wait(&fCond, &fMutex);
+    if (res != 0) {
         jack_error("JackProcessSync::Wait error err = %s", strerror(res));
- }
+    } else {
+        fOwner = pthread_self();
+    }
+}
 
+// TO DO : check thread consistency?
 void JackProcessSync::LockedWait()
 {
     int res;
@@ -85,6 +94,9 @@ void JackProcessSync::LockedWait()
 
 bool JackProcessSync::TimedWait(long usec)
 {
+    ThrowIf(!pthread_equal(pthread_self(), fOwner), JackException("JackProcessSync::TimedWait: a thread has to have locked a mutex before it can wait"));
+    fOwner = 0;
+
     struct timeval T0, T1;
     timespec time;
     struct timeval now;
@@ -97,17 +109,22 @@ bool JackProcessSync::TimedWait(long usec)
     unsigned int next_date_usec = now.tv_usec + usec;
     time.tv_sec = now.tv_sec + (next_date_usec / 1000000);
     time.tv_nsec = (next_date_usec % 1000000) * 1000;
+
     res = pthread_cond_timedwait(&fCond, &fMutex, &time);
-    if (res != 0)
+    if (res != 0) {
         jack_error("JackProcessSync::TimedWait error usec = %ld err = %s", usec, strerror(res));
+    } else {
+        fOwner = pthread_self();
+    }
 
     gettimeofday(&T1, 0);
     jack_log("JackProcessSync::TimedWait finished delta = %5.1lf",
              (1e6 * T1.tv_sec - 1e6 * T0.tv_sec + T1.tv_usec - T0.tv_usec));
-    
+
     return (res == 0);
 }
 
+// TO DO : check thread consistency?
 bool JackProcessSync::LockedTimedWait(long usec)
 {
     struct timeval T0, T1;
@@ -118,7 +135,7 @@ bool JackProcessSync::LockedTimedWait(long usec)
     res1 = pthread_mutex_lock(&fMutex);
     if (res1 != 0)
         jack_error("JackProcessSync::LockedTimedWait error err = %s", usec, strerror(res1));
-        
+
     jack_log("JackProcessSync::TimedWait time out = %ld", usec);
     gettimeofday(&T0, 0);
 
@@ -134,10 +151,10 @@ bool JackProcessSync::LockedTimedWait(long usec)
     res1 = pthread_mutex_unlock(&fMutex);
     if (res1 != 0)
         jack_error("JackProcessSync::LockedTimedWait error err = %s", usec, strerror(res1));
-        
+
     jack_log("JackProcessSync::TimedWait finished delta = %5.1lf",
              (1e6 * T1.tv_sec - 1e6 * T0.tv_sec + T1.tv_usec - T0.tv_usec));
-    
+
     return (res2 == 0);
 }
 

posix/JackProcessSync.h

@@ -12,7 +12,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU Lesser General Public License for more details.
 
 You should have received a copy of the GNU Lesser General Public License
-along with this program; if not, write to the Free Software 
+along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
 */
@@ -22,6 +22,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
 #include "JackPlatformPlug.h"
 #include "JackPosixMutex.h"
+#include "JackException.h"
 #include <sys/time.h>
 #include <unistd.h>
 
@@ -43,7 +44,8 @@ class JackProcessSync : public JackBasePosixMutex
 
         JackProcessSync():JackBasePosixMutex()
         {
-            pthread_cond_init(&fCond, NULL);
+            int res = pthread_cond_init(&fCond, NULL);
+            ThrowIf(res != 0, JackException("JackBasePosixMutex: could not init the cond variable"));
         }
 
         virtual ~JackProcessSync()
@@ -53,16 +55,16 @@ class JackProcessSync : public JackBasePosixMutex
 
         bool TimedWait(long usec);
         bool LockedTimedWait(long usec);
-        
+
         void Wait();
         void LockedWait();
-         
+
         void Signal();
         void LockedSignal();
-         
+
         void SignalAll();
         void LockedSignalAll();
-  
+
 };
 
 } // end of namespace

windows/JackWinMutex.h

@@ -1,20 +1,20 @@
 /*
  Copyright (C) 2004-2008 Grame
- 
+
  This program is free software; you can redistribute it and/or modify
  it under the terms of the GNU Lesser General Public License as published by
  the Free Software Foundation; either version 2.1 of the License, or
  (at your option) any later version.
- 
+
  This program is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  GNU Lesser General Public License for more details.
- 
+
  You should have received a copy of the GNU Lesser General Public License
  along with this program; if not, write to the Free Software
  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- 
+
  */
 
 
@@ -28,6 +28,81 @@ namespace Jack
 /*!
 \brief Mutex abstraction.
 */
+class JackBaseWinMutex
+{
+
+    protected:
+
+        HANDLE fMutex;
+        DWORD fOwner;
+
+    public:
+
+        JackBaseWinMutex():fOwner(0)
+        {
+            // In recursive mode by default
+            fMutex = (HANDLE)CreateMutex(0, FALSE, 0);
+            ThrowIf(fMutex == 0, JackException("JackWinMutex: could not init the mutex"));
+        }
+
+        virtual ~JackBaseWinMutex()
+        {
+            CloseHandle(fMutex);
+        }
+
+        bool Lock()
+        {
+            if (fOwner != GetCurrentThreadId()) {
+                DWORD res = WaitForSingleObject(fMutex, INFINITE);
+                if (res == WAIT_OBJECT_0) {
+                    fOwner = GetCurrentThreadId();
+                    return true;
+                } else {
+                    jack_log("JackWinMutex::Lock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackWinMutex::Lock mutex already locked by thread = %d", GetCurrentThreadId());
+                return false;
+            }
+        }
+
+        bool Trylock()
+        {
+            if (fOwner != GetCurrentThreadId()) {
+                DWORD res = WaitForSingleObject(fMutex, 0);
+                if (res == WAIT_OBJECT_0) {
+                    fOwner = GetCurrentThreadId();
+                    return true;
+                } else {
+                    jack_log("JackWinMutex::Trylock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackWinMutex::Trylock mutex already locked by thread = %d", GetCurrentThreadId());
+                return false;
+            }
+        }
+
+        bool Unlock()
+        {
+            if (fOwner == GetCurrentThreadId()) {
+                fOwner = 0;
+                int res = ReleaseMutex(fMutex);
+                if (res != 0) {
+                    return true;
+                } else {
+                    jack_log("JackWinMutex::Unlock res = %d", res);
+                    return false;
+                }
+            } else {
+                jack_error("JackWinMutex::Unlock mutex not locked by thread = %d", GetCurrentThreadId());
+                return false;
+            }
+        }
+
+};
+
 class JackWinMutex
 {
 
@@ -65,6 +140,7 @@ class JackWinMutex
 
 };
 
+
 } // namespace
 
 #endif