The C++ std provides unique_lock which supports all ScopeLock features. Use the 
standard lock instead of rewriting it.
---
 src/log/agent/lga_agent.cc  | 59 +++++++++++++------------------------
 src/log/agent/lga_agent.h   | 11 +++----
 src/log/agent/lga_client.cc | 32 +++++---------------
 src/log/agent/lga_client.h  |  3 +-
 src/log/agent/lga_common.h  | 19 ------------
 src/log/agent/lga_state.cc  |  9 +++---
 src/log/agent/lga_util.cc   |  8 ++---
 7 files changed, 43 insertions(+), 98 deletions(-)

diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
index 6c8d927a5..0f9aad91f 100644
--- a/src/log/agent/lga_agent.cc
+++ b/src/log/agent/lga_agent.cc
@@ -136,25 +136,6 @@ ScopeData::~ScopeData() {
 
//------------------------------------------------------------------------------
 LogAgent::LogAgent() {
   client_list_.clear();
-  // There is high risk of calling one @LogClient method
-  // in the body of other @LogClient methods, such case would cause deadlock
-  // even they are in the same thread context.
-  // To avoid such risk, use RECURSIVE MUTEX for @LogClient
-  pthread_mutexattr_t attr;
-  int result = pthread_mutexattr_init(&attr);
-  assert(result == 0 && "Failed to init mutex attribute");
-
-  result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
-  assert(result == 0 && "Failed to set mutex type");
-
-  result = pthread_mutex_init(&mutex_, nullptr);
-  assert(result == 0 && "Failed to init `mutex_`");
-
-  pthread_mutexattr_destroy(&attr);
-
-  // Initialize @get_delete_obj_sync_mutex_
-  result = pthread_mutex_init(&get_delete_obj_sync_mutex_, nullptr);
-  assert(result == 0 && "Failed to init `get_delete_obj_sync_mutex_`");
 }
 
 void LogAgent::PopulateOpenParams(
@@ -201,7 +182,7 @@ LogStreamInfo* 
LogAgent::SearchLogStreamInfoByHandle(SaLogStreamHandleT h) {
 // Search for client in the @client_list_ based on client @handle
 LogClient* LogAgent::SearchClientByHandle(uint32_t handle) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   LogClient* client = nullptr;
   for (const auto& c : client_list_) {
     if (c != nullptr && c->GetHandle() == handle) {
@@ -215,7 +196,7 @@ LogClient* LogAgent::SearchClientByHandle(uint32_t handle) {
 // Search for client in the @client_list_ based on client @id
 LogClient* LogAgent::SearchClientById(uint32_t id) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   LogClient* client = nullptr;
   for (const auto& c : client_list_) {
     if (c != nullptr && c->GetClientId() == id) {
@@ -230,7 +211,7 @@ LogClient* LogAgent::SearchClientById(uint32_t id) {
 // This method should be called in @saLogFinalize() context
 void LogAgent::RemoveLogClient(LogClient** client) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   assert(*client != nullptr);
   auto it = std::remove(client_list_.begin(), client_list_.end(), *client);
   if (it != client_list_.end()) {
@@ -246,7 +227,7 @@ void LogAgent::RemoveLogClient(LogClient** client) {
 
 void LogAgent::RemoveAllLogClients() {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   for (auto& client : client_list_) {
     if (client == nullptr) continue;
     delete client;
@@ -257,7 +238,7 @@ void LogAgent::RemoveAllLogClients() {
 // This method should be called in @saLogInitialize() context
 void LogAgent::AddLogClient(LogClient* client) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   assert(client != nullptr);
   client_list_.push_back(client);
 }
@@ -265,7 +246,7 @@ void LogAgent::AddLogClient(LogClient* client) {
 // Do recover all log clients in list @client_list_
 bool LogAgent::RecoverAllLogClients() {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   for (auto& client : client_list_) {
     // We just get SC absence in MDS thread
     // Don't try to recover the client list.
@@ -287,7 +268,7 @@ bool LogAgent::RecoverAllLogClients() {
 
 void LogAgent::NoLogServer() {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   // When SC is absent, surely, no active SC.
   // Then reset LOG server destination address.
   atomic_data_.lgs_mds_dest = 0;
@@ -444,7 +425,7 @@ SaAisErrorT LogAgent::saLogSelectionObjectGet(
   }
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     // Get log client from the global list
     client = SearchClientByHandle(logHandle);
@@ -495,7 +476,7 @@ SaAisErrorT LogAgent::saLogDispatch(SaLogHandleT logHandle,
   }
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     // Get log client from the global list
     client = SearchClientByHandle(logHandle);
@@ -583,7 +564,7 @@ SaAisErrorT LogAgent::saLogFinalize(SaLogHandleT logHandle) 
{
   ScopeData data{&client_data, &is_locked};
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     // Get log client from the global list
     client = SearchClientByHandle(logHandle);
@@ -633,7 +614,7 @@ SaAisErrorT LogAgent::saLogFinalize(SaLogHandleT logHandle) 
{
       TRACE("%s lga_state = Recovery state (1)", __func__);
       if (client->is_client_initialized() == false) {
         TRACE("\t Client is not initialized. Remove it from database");
-        ScopeLock critical_section(get_delete_obj_sync_mutex_);
+        std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
         RemoveLogClient(&client);
         return ais_rc;
       }
@@ -647,7 +628,7 @@ SaAisErrorT LogAgent::saLogFinalize(SaLogHandleT logHandle) 
{
   if (ais_rc == SA_AIS_OK || ais_rc == SA_AIS_ERR_BAD_HANDLE) {
     TRACE("%s delete_one_client", __func__);
     if (true) {
-      ScopeLock critical_section(get_delete_obj_sync_mutex_);
+      std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
       RemoveLogClient(&client);
     }
   }
@@ -840,7 +821,7 @@ SaAisErrorT LogAgent::saLogStreamOpen_2(
   if (ais_rc != SA_AIS_OK) return ais_rc;
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     // Get log client from the global list
     client = SearchClientByHandle(logHandle);
@@ -891,7 +872,7 @@ SaAisErrorT LogAgent::saLogStreamOpen_2(
 
     // Remove the client from the database if recovery failed
     if (client->is_recovery_done() == false) {
-      ScopeLock critical_section(get_delete_obj_sync_mutex_);
+      std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
       // Make sure that client is just removed from the list
       // if no other users use it
       if (client->FetchRefCounter() == 1) {
@@ -1145,7 +1126,7 @@ SaAisErrorT 
LogAgent::saLogWriteLogAsync(SaLogStreamHandleT logStreamHandle,
   ScopeData data{&client_data, &stream_data, &is_locked};
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     // Retrieve the @LogStreamInfo based on @logStreamHandle
     stream = SearchLogStreamInfoByHandle(logStreamHandle);
@@ -1267,7 +1248,7 @@ SaAisErrorT 
LogAgent::saLogWriteLogAsync(SaLogStreamHandleT logStreamHandle,
 
     // Remove the client from the database if recovery failed
     if (client->is_recovery_done() == false) {
-      ScopeLock critical_section(get_delete_obj_sync_mutex_);
+      std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
       // Make sure that client is just removed from the list
       // if no other users use it
       if (client->FetchRefCounter() == 1) {
@@ -1324,7 +1305,7 @@ SaAisErrorT LogAgent::saLogStreamClose(SaLogStreamHandleT 
logStreamHandle) {
   ScopeData data{&client_data, &stream_data, &is_locked};
 
   if (true) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
 
     stream = SearchLogStreamInfoByHandle(logStreamHandle);
     if (stream == nullptr) {
@@ -1372,7 +1353,7 @@ SaAisErrorT LogAgent::saLogStreamClose(SaLogStreamHandleT 
logStreamHandle) {
     // No server is available. Remove the stream from client database.
     // Server side will manage to release resources of this stream when up.
     TRACE("%s No server", __func__);
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
     client->RemoveLogStreamInfo(&stream);
     ais_rc = SA_AIS_OK;
     return ais_rc;
@@ -1397,7 +1378,7 @@ SaAisErrorT LogAgent::saLogStreamClose(SaLogStreamHandleT 
logStreamHandle) {
 
     // Remove the client from the database if recovery failed
     if (client->is_recovery_done() == false) {
-      ScopeLock critical_section(get_delete_obj_sync_mutex_);
+      std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
       // Make sure that client is just removed from the list
       // if no other users use it
       if (client->FetchRefCounter() == 1) {
@@ -1445,7 +1426,7 @@ SaAisErrorT LogAgent::saLogStreamClose(SaLogStreamHandleT 
logStreamHandle) {
   }
 
   if (ais_rc == SA_AIS_OK) {
-    ScopeLock critical_section(get_delete_obj_sync_mutex_);
+    std::unique_lock<std::mutex> critical_section(critial_section_mutex_);
     client->RemoveLogStreamInfo(&stream);
   }
 
diff --git a/src/log/agent/lga_agent.h b/src/log/agent/lga_agent.h
index 0c32ea33b..2ca40f505 100644
--- a/src/log/agent/lga_agent.h
+++ b/src/log/agent/lga_agent.h
@@ -20,6 +20,7 @@
 
 #include <atomic>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <vector>
 #include <saAis.h>
@@ -264,7 +265,7 @@ class LogAgent {
   // Used to synchronize adding/removing @client to/from @client_list_
   // MUST use this mutex whenever accessing to client from @client_list_
   // This mutex is RECURSIVE.
-  pthread_mutex_t mutex_;
+  std::recursive_mutex mutex_;
 
   // Protect the short critical section: Search<xx>ByHandle() to
   // FetchAndUpdateObjectState(). Avoid race condition b/w
@@ -275,8 +276,8 @@ class LogAgent {
   // and updating @ref_counter_.
   // NOTE: Use native mutex instead of base::Mutex because base::Mutex
   // has coredump in MDS thread when unlock the mutex. Not yet analyze.
-  // base::Mutex get_delete_obj_sync_mutex_;
-  pthread_mutex_t get_delete_obj_sync_mutex_;
+  // base::Mutex critial_section_mutex_;
+  std::mutex critial_section_mutex_;
 
   // Hold list of current log clients
   std::vector<LogClient*> client_list_;
@@ -370,11 +371,11 @@ inline bool LogAgent::waiting_log_server_up() const {
 }
 
 inline void LogAgent::EnterCriticalSection() {
-  osaf_mutex_lock_ordie(&get_delete_obj_sync_mutex_);
+  critial_section_mutex_.lock();
 }
 
 inline void LogAgent::LeaveCriticalSection() {
-  osaf_mutex_unlock_ordie(&get_delete_obj_sync_mutex_);
+  critial_section_mutex_.unlock();
 }
 
 #endif  // SRC_LOG_AGENT_LGA_AGENT_H_
diff --git a/src/log/agent/lga_client.cc b/src/log/agent/lga_client.cc
index 2eb37a0f7..56f1100c5 100644
--- a/src/log/agent/lga_client.cc
+++ b/src/log/agent/lga_client.cc
@@ -59,22 +59,6 @@ LogClient::LogClient(const SaLogCallbacksT* cb, uint32_t id, 
SaVersionT ver)
   if ((rc = m_NCS_IPC_ATTACH(&mailbox_)) != NCSCC_RC_SUCCESS) {
     assert(rc != NCSCC_RC_SUCCESS && "Attach the mbx failed");
   }
-
-  // As there is high risk of calling of one @LogClient method
-  // in the body of other method, such case would cause deadlock
-  // even they are in the same thread context.
-  // To avoid such risk, use RECURSIVE MUTEX for @LogClient
-  pthread_mutexattr_t attr;
-  int result = pthread_mutexattr_init(&attr);
-  assert(result == 0 && "Failed to init mutex attribute");
-
-  result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
-  assert(result == 0 && "Failed to set mutex type");
-
-  result = pthread_mutex_init(&mutex_, nullptr);
-  assert(result == 0 && "Failed to init mutex");
-
-  pthread_mutexattr_destroy(&attr);
 }
 
 LogClient::~LogClient() {
@@ -98,8 +82,6 @@ LogClient::~LogClient() {
   m_NCS_IPC_DETACH(&mailbox_, LogClient::ClearMailBox, nullptr);
   // Free the allocated mailbox for this client
   m_NCS_IPC_RELEASE(&mailbox_, nullptr);
-
-  pthread_mutex_destroy(&mutex_);
 }
 
 bool LogClient::ClearMailBox(NCSCONTEXT arg, NCSCONTEXT msg) {
@@ -142,7 +124,7 @@ void LogClient::InvokeCallback(const lgsv_msg_t* msg) {
       if (callbacks_.saLogFilterSetCallback) {
         LogStreamInfo* stream;
         stream = SearchLogStreamInfoById(cbk_info->lgs_stream_id);
-        ScopeLock scopeLock(mutex_);
+        std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
         if (stream != nullptr) {
           callbacks_.saLogFilterSetCallback(
               stream->GetHandle(), 
cbk_info->serverity_filter_cbk.log_severity);
@@ -257,7 +239,7 @@ void LogClient::AddLogStreamInfo(LogStreamInfo* stream,
                                  SaLogStreamOpenFlagsT flag,
                                  SaLogHeaderTypeT htype) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   assert(stream != nullptr);
   // Give additional info to @stream
   stream->SetOpenFlags(flag);
@@ -270,7 +252,7 @@ void LogClient::AddLogStreamInfo(LogStreamInfo* stream,
 // and should be called after getting @saLogStreamClose() or @saLogFinalize().
 void LogClient::RemoveLogStreamInfo(LogStreamInfo** stream) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   assert(*stream != nullptr);
   auto i = std::remove(stream_list_.begin(), stream_list_.end(), *stream);
   if (i != stream_list_.end()) {
@@ -282,7 +264,7 @@ void LogClient::RemoveLogStreamInfo(LogStreamInfo** stream) 
{
 
 LogStreamInfo* LogClient::SearchLogStreamInfoById(uint32_t id) {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   LogStreamInfo* stream = nullptr;
   for (auto const& s : stream_list_) {
     if (s != nullptr && s->GetStreamId() == id) {
@@ -357,7 +339,7 @@ bool LogClient::RecoverMe() {
     return false;
   }
 
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   for (const auto& stream : stream_list_) {
     if (stream == nullptr) continue;
     TRACE("Recover client = %d, stream = %d", client_id_,
@@ -376,7 +358,7 @@ bool LogClient::RecoverMe() {
 
 bool LogClient::HaveLogStreamInUse() {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   for (const auto& s : stream_list_) {
     if ((s != nullptr) && (s->ref_counter_object_.ref_counter() > 0))
       return true;
@@ -386,7 +368,7 @@ bool LogClient::HaveLogStreamInUse() {
 
 void LogClient::NoLogServer() {
   TRACE_ENTER();
-  ScopeLock scopeLock(mutex_);
+  std::unique_lock<std::recursive_mutex> scopeLock(mutex_);
   // No SCs, no valid client id.
   client_id_ = 0;
   atomic_data_.recovered_flag = false;
diff --git a/src/log/agent/lga_client.h b/src/log/agent/lga_client.h
index e6e2c911e..db220cb8a 100644
--- a/src/log/agent/lga_client.h
+++ b/src/log/agent/lga_client.h
@@ -22,6 +22,7 @@
 #include <vector>
 #include <list>
 #include <atomic>
+#include <mutex>
 #include <saLog.h>
 #include "base/mutex.h"
 #include "mds/mds_papi.h"
@@ -287,7 +288,7 @@ class LogClient {
   // Used to synchronize adding/removing to/from @stream_list_
   // MUST use this mutex whenever accessing to stream from @stream_list_
   // Using native mutex as base::Mutex does not support RECURSIVE.
-  pthread_mutex_t mutex_;
+  std::recursive_mutex mutex_;
 
   // Hold information how many thread are referring to this object
   // Refer to `RefCounter` class for more info.
diff --git a/src/log/agent/lga_common.h b/src/log/agent/lga_common.h
index 87bd9e717..0467eaea8 100644
--- a/src/log/agent/lga_common.h
+++ b/src/log/agent/lga_common.h
@@ -32,23 +32,4 @@ enum class LogServerState {
   kHasActiveLogServer
 };
 
-//>
-// Introduce its own scope lock mutex because @base::Lock does not support
-// RECURSIVE MUTEX.
-//<
-class ScopeLock {
- public:
-  explicit ScopeLock(pthread_mutex_t& m) : mutex_{&m} { Lock(); };
-  ~ScopeLock() { UnLock(); }
-
- private:
-  void Lock() { osaf_mutex_lock_ordie(mutex_); }
-  void UnLock() { osaf_mutex_unlock_ordie(mutex_); }
-
- private:
-  pthread_mutex_t* mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(ScopeLock);
-};
-
 #endif  // SRC_LOG_AGENT_LGA_COMMON_H_
diff --git a/src/log/agent/lga_state.cc b/src/log/agent/lga_state.cc
index 970fe27d2..51b8f3680 100644
--- a/src/log/agent/lga_state.cc
+++ b/src/log/agent/lga_state.cc
@@ -256,20 +256,19 @@ void lga_recovery2_unlock(void) { 
osaf_mutex_unlock_ordie(&lga_recov2_lock); }
 
 typedef struct {
   RecoveryState state;
-  pthread_mutex_t lock;
+  std::mutex lock;
 } lga_state_s;
 
-static lga_state_s lga_state = {.state = RecoveryState::kNormal,
-                                .lock = PTHREAD_MUTEX_INITIALIZER};
+static lga_state_s lga_state = {.state = RecoveryState::kNormal};
 
 static void set_lga_recovery_state(RecoveryState state) {
-  ScopeLock lock(lga_state.lock);
+  std::unique_lock<std::mutex> lock(lga_state.lock);
   lga_state.state = state;
 }
 
 bool is_lga_recovery_state(RecoveryState state) {
   bool rc = false;
-  ScopeLock lock(lga_state.lock);
+  std::unique_lock<std::mutex> lock(lga_state.lock);
   if (state == lga_state.state) rc = true;
   return rc;
 }
diff --git a/src/log/agent/lga_util.cc b/src/log/agent/lga_util.cc
index 0bb91aaee..6241b9ed6 100644
--- a/src/log/agent/lga_util.cc
+++ b/src/log/agent/lga_util.cc
@@ -30,7 +30,7 @@
 #include "log/agent/lga_common.h"
 
 // Variables used during startup/shutdown only
-static pthread_mutex_t init_lock = PTHREAD_MUTEX_INITIALIZER;
+static std::mutex init_lock;
 static unsigned int client_counter = 0;
 
 /**
@@ -81,7 +81,7 @@ static unsigned int lga_create() {
  */
 unsigned int lga_startup() {
   unsigned int rc = NCSCC_RC_SUCCESS;
-  ScopeLock lock(init_lock);
+  std::unique_lock<std::mutex> lock(init_lock);
   std::atomic<MDS_HDL>& mds_hdl = LogAgent::instance()->atomic_get_mds_hdl();
 
   TRACE_ENTER();
@@ -109,7 +109,7 @@ done:
  * The function help to trace number of clients
  */
 void lga_increase_user_counter(void) {
-  ScopeLock lock(init_lock);
+  std::unique_lock<std::mutex> lock(init_lock);
 
     ++client_counter;
     TRACE_2("client_counter: %u", client_counter);
@@ -121,7 +121,7 @@ void lga_increase_user_counter(void) {
  */
 void lga_decrease_user_counter(void) {
   TRACE_ENTER2("client_counter: %u", client_counter);
-  ScopeLock lock(init_lock);
+  std::unique_lock<std::mutex> lock(init_lock);
 
   if (client_counter > 0)
     --client_counter;
-- 
2.17.1



_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to