From 0e47f66f21683a18f34282e07efa85cdac4fe0e0 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Jos=C3=A9=20Fonseca?= <jose.r.fonseca@gmail.com>
Date: Tue, 27 Dec 2011 20:00:47 +0000
Subject: [PATCH 1/1] Move mutex abstraction to os_thread.hpp.

---
 common/os.hpp                 |  4 ---
 common/os_posix.cpp           | 19 -------------
 common/os_thread.hpp          | 53 +++++++++++++++++++++++++++++++++++
 common/os_win32.cpp           | 23 ---------------
 common/trace_writer_local.cpp | 15 +++++-----
 common/trace_writer_local.hpp |  5 ++++
 6 files changed, 66 insertions(+), 53 deletions(-)

diff --git a/common/os.hpp b/common/os.hpp
index 6a3b8c8..caf9dc3 100644
--- a/common/os.hpp
+++ b/common/os.hpp
@@ -46,10 +46,6 @@
 
 namespace os {
 
-void acquireMutex(void);
-
-void releaseMutex(void);
-
 void log(const char *format, ...)
 #ifdef __GNUC__
     __attribute__ ((format (printf, 1, 2)))
diff --git a/common/os_posix.cpp b/common/os_posix.cpp
index 7dc2bb4..65c5404 100644
--- a/common/os_posix.cpp
+++ b/common/os_posix.cpp
@@ -32,7 +32,6 @@
 #include <unistd.h>
 #include <sys/time.h>
 #include <sys/wait.h>
-#include <pthread.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <signal.h>
@@ -58,24 +57,6 @@
 namespace os {
 
 
-static pthread_mutex_t 
-mutex = PTHREAD_MUTEX_INITIALIZER;
-
-
-void
-acquireMutex(void)
-{
-    pthread_mutex_lock(&mutex);
-}
-
-
-void
-releaseMutex(void)
-{
-    pthread_mutex_unlock(&mutex);
-}
-
-
 String
 getProcessName(void)
 {
diff --git a/common/os_thread.hpp b/common/os_thread.hpp
index 7349255..b17563f 100644
--- a/common/os_thread.hpp
+++ b/common/os_thread.hpp
@@ -32,6 +32,7 @@
 #ifndef _OS_THREAD_HPP_
 #define _OS_THREAD_HPP_
 
+
 #ifdef _WIN32
 #include <windows.h>
 #else
@@ -41,6 +42,58 @@
 namespace os {
 
 
+    class recursive_mutex
+    {
+    public:
+#ifdef _WIN32
+        typedef CRITICAL_SECTION native_handle_type;
+#else
+        typedef pthread_mutex_t native_handle_type;
+#endif
+
+        recursive_mutex(void) {
+#ifdef _WIN32
+            InitializeCriticalSection(&_native_handle);
+#else
+            pthread_mutexattr_t attr;
+            pthread_mutexattr_init(&attr);
+            pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+            pthread_mutex_init(&_native_handle, NULL);
+            pthread_mutexattr_destroy(&attr);
+#endif
+        }
+
+        ~recursive_mutex() {
+#ifdef _WIN32
+            DeleteCriticalSection(&_native_handle);
+#else
+            pthread_mutex_destroy(&_native_handle);
+#endif
+        }
+
+        inline void
+        lock(void) {
+#ifdef _WIN32
+            EnterCriticalSection(&_native_handle);
+#else
+            pthread_mutex_lock(&_native_handle);
+#endif
+        }
+
+        inline void
+        unlock(void) {
+#ifdef _WIN32
+            LeaveCriticalSection(&_native_handle);
+#else
+            pthread_mutex_unlock(&_native_handle);
+#endif
+        }
+
+    private:
+        native_handle_type _native_handle;
+    };
+
+
     template <typename T>
     class thread_specific_ptr
     {
diff --git a/common/os_win32.cpp b/common/os_win32.cpp
index 7e1a544..a020354 100644
--- a/common/os_win32.cpp
+++ b/common/os_win32.cpp
@@ -38,29 +38,6 @@
 namespace os {
 
 
-/* 
- * Trick from http://locklessinc.com/articles/pthreads_on_windows/
- */
-static CRITICAL_SECTION
-criticalSection = {
-    (PCRITICAL_SECTION_DEBUG)-1, -1, 0, 0, 0, 0
-};
-
-
-void
-acquireMutex(void)
-{
-    EnterCriticalSection(&criticalSection);
-}
-
-
-void
-releaseMutex(void)
-{
-    LeaveCriticalSection(&criticalSection);
-}
-
-
 String
 getProcessName(void)
 {
diff --git a/common/trace_writer_local.cpp b/common/trace_writer_local.cpp
index 52962c8..a3ab720 100644
--- a/common/trace_writer_local.cpp
+++ b/common/trace_writer_local.cpp
@@ -128,7 +128,7 @@ static unsigned next_thread_id = 0;
 static os::thread_specific_ptr<unsigned> thread_id_specific_ptr;
 
 unsigned LocalWriter::beginEnter(const FunctionSig *sig) {
-    os::acquireMutex();
+    mutex.lock();
     ++acquired;
 
     if (!m_file->isOpened()) {
@@ -152,11 +152,11 @@ unsigned LocalWriter::beginEnter(const FunctionSig *sig) {
 void LocalWriter::endEnter(void) {
     Writer::endEnter();
     --acquired;
-    os::releaseMutex();
+    mutex.unlock();
 }
 
 void LocalWriter::beginLeave(unsigned call) {
-    os::acquireMutex();
+    mutex.lock();
     ++acquired;
     Writer::beginLeave(call);
 }
@@ -164,16 +164,17 @@ void LocalWriter::beginLeave(unsigned call) {
 void LocalWriter::endLeave(void) {
     Writer::endLeave();
     --acquired;
-    os::releaseMutex();
+    mutex.unlock();
 }
 
 void LocalWriter::flush(void) {
     /*
      * Do nothing if the mutex is already acquired (e.g., if a segfault happen
-     * while writing the file) to prevent dead-lock.
+     * while writing the file) as state could be inconsistent, therefore yield
+     * inconsistent trace files and/or repeated segfaults till infinity.
      */
 
-    os::acquireMutex();
+    mutex.lock();
     if (acquired) {
         os::log("apitrace: ignoring exception while tracing\n");
     } else {
@@ -184,7 +185,7 @@ void LocalWriter::flush(void) {
         }
         --acquired;
     }
-    os::releaseMutex();
+    mutex.unlock();
 }
 
 
diff --git a/common/trace_writer_local.hpp b/common/trace_writer_local.hpp
index 7c70580..e54142f 100644
--- a/common/trace_writer_local.hpp
+++ b/common/trace_writer_local.hpp
@@ -31,6 +31,7 @@
 #define _TRACE_WRITER_LOCAL_HPP_
 
 
+#include "os_thread.hpp"
 #include "trace_writer.hpp"
 
 
@@ -52,6 +53,10 @@ namespace trace {
      */
     class LocalWriter : public Writer {
     protected:
+        /**
+         * We need a recursive mutex so that it doesn't dead lock when a segfault happens when the mutex is held.
+         */
+        os::recursive_mutex mutex;
         int acquired;
 
     public:
-- 
2.43.0