Hi Hans,

It is good to give an option to Mutex class not to abort. We can avoid the abort in mutex_unlock (as reported in coredump), but I feel the issue is still there.

We may hit a problem (segv?) with "mutex_->good()" since the other thread is wiping out the mutex_ in destructor, it is a matter of timing to happen I guess.

As we don't have (and don't want to have) any protection between two threads for the TraceLog, so the good one (I hope) is making one of those threads not to touch the TraceLog.

If you don't like to remove the destructor, another way is locating the gl_trace/gl_log to the HEAP?

Thanks,

Minh



On 23/05/18 20:50, Hans Nordeback wrote:
Change Mutex class to make it possible for caller to decide if abort
---
  src/base/logtrace_client.cc |  5 ++++-
  src/base/mutex.cc           |  2 +-
  src/base/mutex.h            | 22 +++++++++++++---------
  3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/base/logtrace_client.cc b/src/base/logtrace_client.cc
index 0dac6d389..f597c1ae3 100644
--- a/src/base/logtrace_client.cc
+++ b/src/base/logtrace_client.cc
@@ -76,7 +76,7 @@ bool TraceLog::Init(const char *msg_id, WriteMode mode) {
    msg_id_ = base::LogMessage::MsgId{msg_id};
    log_socket_ = new base::UnixClientSocket{Osaflog::kServerSocketPath,
      static_cast<base::UnixSocket::Mode>(mode)};
-  mutex_ = new base::Mutex{};
+  mutex_ = new base::Mutex{false};
return true;
  }
@@ -91,6 +91,9 @@ void TraceLog::Log(base::LogMessage::Severity severity, const 
char *fmt,
  void TraceLog::LogInternal(base::LogMessage::Severity severity, const char 
*fmt,
                             va_list ap) {
    base::Lock lock(*mutex_);
+
+  if (!mutex_->good()) return;
+
    uint32_t id = sequence_id_;
    sequence_id_ = id < kMaxSequenceId ? id + 1 : 1;
    buffer_.clear();
diff --git a/src/base/mutex.cc b/src/base/mutex.cc
index 5fa6ac55a..1627ac20b 100644
--- a/src/base/mutex.cc
+++ b/src/base/mutex.cc
@@ -20,7 +20,7 @@
namespace base { -Mutex::Mutex() : mutex_{} {
+Mutex::Mutex(bool abort) : abort_{abort}, mutex_{}, result_{0} {
    pthread_mutexattr_t attr;
    int result = pthread_mutexattr_init(&attr);
    if (result != 0) osaf_abort(result);
diff --git a/src/base/mutex.h b/src/base/mutex.h
index 7b3cee187..e3c54a711 100644
--- a/src/base/mutex.h
+++ b/src/base/mutex.h
@@ -31,30 +31,34 @@ namespace base {
  class Mutex {
   public:
    using NativeHandleType = pthread_mutex_t*;
-  Mutex();
+  Mutex(bool abort = true);
    ~Mutex();
    void Lock() {
-    int result = pthread_mutex_lock(&mutex_);
-    if (result != 0) osaf_abort(result);
+    result_ = pthread_mutex_lock(&mutex_);
+    if (abort_ && result_ != 0) osaf_abort(result_);
    }
    bool TryLock() {
-    int result = pthread_mutex_trylock(&mutex_);
-    if (result == 0) {
+    result_ = pthread_mutex_trylock(&mutex_);
+    if (result_ == 0) {
        return true;
-    } else if (result == EBUSY) {
+    } else if (result_ == EBUSY) {
        return false;
      } else {
-      osaf_abort(result);
+      if (abort_) osaf_abort(result_);
+      return false;
      }
    }
    void Unlock() {
-    int result = pthread_mutex_unlock(&mutex_);
-    if (result != 0) osaf_abort(result);
+    result_ = pthread_mutex_unlock(&mutex_);
+    if (abort_ && result_ != 0) osaf_abort(result_);
    }
    NativeHandleType native_handle() { return &mutex_; }
+ bool good() const {return result_ == 0;};
   private:
+  bool abort_;
    pthread_mutex_t mutex_;
+  int result_;
    DELETE_COPY_AND_MOVE_OPERATORS(Mutex);
  };


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to