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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
