There is a race in using singleton-static class object b/w mds thread
and application thread - caller of exit() api.
This patch still uses singleton but making the instance shared_ptr
to ensure the resource will not be destroyed if it is being used.
---
src/log/agent/lga_agent.cc | 7 ++-----
src/log/agent/lga_agent.h | 21 ++++++++++++++-------
src/log/agent/lga_api.cc | 20 ++++++++++----------
src/log/agent/lga_mds.cc | 32 ++++++++++++++++----------------
src/log/agent/lga_state.cc | 2 +-
src/log/agent/lga_util.cc | 10 +++++-----
6 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
index bf9caa935..1000bb3fd 100644
--- a/src/log/agent/lga_agent.cc
+++ b/src/log/agent/lga_agent.cc
@@ -108,7 +108,7 @@ ScopeData::~ScopeData() {
recovery2_unlock(is_locked_);
}
- LogAgent::instance().EnterCriticalSection();
+ LogAgent::instance()->EnterCriticalSection();
LogClient* client = client_data_->client;
bool* is_updated = client_data_->is_updated;
RefCounter::Degree client_degree = client_data_->value;
@@ -128,15 +128,12 @@ ScopeData::~ScopeData() {
stream->RestoreRefCounter(caller, stream_degree, *stream_is_updated);
}
}
- LogAgent::instance().LeaveCriticalSection();
+ LogAgent::instance()->LeaveCriticalSection();
}
//------------------------------------------------------------------------------
// LogAgent
//------------------------------------------------------------------------------
-// Singleton represents LOG agent.
-LogAgent LogAgent::me_;
-
LogAgent::LogAgent() {
client_list_.clear();
// There is high risk of calling one @LogClient method
diff --git a/src/log/agent/lga_agent.h b/src/log/agent/lga_agent.h
index 0049da054..0c32ea33b 100644
--- a/src/log/agent/lga_agent.h
+++ b/src/log/agent/lga_agent.h
@@ -19,12 +19,14 @@
#define SRC_LOG_AGENT_LGA_AGENT_H_
#include <atomic>
+#include <memory>
#include <string>
#include <vector>
-#include "mds/mds_papi.h"
#include <saAis.h>
-#include "base/macros.h"
#include <saLog.h>
+
+#include "base/macros.h"
+#include "mds/mds_papi.h"
#include "log/common/lgsv_msg.h"
#include "log/common/lgsv_defs.h"
#include "log/agent/lga_common.h"
@@ -80,7 +82,15 @@ class LogClient;
//<
class LogAgent {
public:
- static LogAgent& instance() { return me_; }
+ static std::shared_ptr<LogAgent>& instance() {
+ // Ensure this static singleton instance is only destroyed when
+ // no one is using it. Note that: static data can be destroyed
+ // in log application thread which calls exit() libc. So, introducing
+ // shared_ptr<> to avoid races among threads.
+ static std::shared_ptr<LogAgent> me =
+ std::shared_ptr<LogAgent>{new LogAgent()};
+ return me;
+ }
//<
// C++ APIs wrapper for corresponding C LOG Agent APIs
@@ -158,11 +168,11 @@ class LogAgent {
// Introduce these public interface for MDS thread use.
void EnterCriticalSection();
void LeaveCriticalSection();
+ ~LogAgent() {}
private:
// Not allow to create @LogAgent object, except the singleton object @me_.
LogAgent();
- ~LogAgent() {}
// True if there is no Active SC, otherwise false
bool no_active_log_server() const;
@@ -274,9 +284,6 @@ class LogAgent {
// LGS LGA sync params
NCS_SEL_OBJ lgs_sync_sel_;
- // Singleton represents LOG Agent in LOG application process
- static LogAgent me_;
-
DELETE_COPY_AND_MOVE_OPERATORS(LogAgent);
};
diff --git a/src/log/agent/lga_api.cc b/src/log/agent/lga_api.cc
index 37a1cc650..97dbf1d31 100644
--- a/src/log/agent/lga_api.cc
+++ b/src/log/agent/lga_api.cc
@@ -39,7 +39,7 @@
SaAisErrorT saLogInitialize(SaLogHandleT* logHandle,
const SaLogCallbacksT* callbacks,
SaVersionT* version) {
- return LogAgent::instance().saLogInitialize(logHandle, callbacks, version);
+ return LogAgent::instance()->saLogInitialize(logHandle, callbacks, version);
}
/***************************************************************************
@@ -69,7 +69,7 @@ SaAisErrorT saLogInitialize(SaLogHandleT* logHandle,
***************************************************************************/
SaAisErrorT saLogSelectionObjectGet(SaLogHandleT logHandle,
SaSelectionObjectT* selectionObject) {
- return LogAgent::instance().saLogSelectionObjectGet(logHandle,
+ return LogAgent::instance()->saLogSelectionObjectGet(logHandle,
selectionObject);
}
@@ -96,7 +96,7 @@ SaAisErrorT saLogSelectionObjectGet(SaLogHandleT logHandle,
***************************************************************************/
SaAisErrorT saLogDispatch(SaLogHandleT logHandle,
SaDispatchFlagsT dispatchFlags) {
- return LogAgent::instance().saLogDispatch(logHandle, dispatchFlags);
+ return LogAgent::instance()->saLogDispatch(logHandle, dispatchFlags);
}
/***************************************************************************
@@ -122,7 +122,7 @@ SaAisErrorT saLogDispatch(SaLogHandleT logHandle,
*
***************************************************************************/
SaAisErrorT saLogFinalize(SaLogHandleT logHandle) {
- return LogAgent::instance().saLogFinalize(logHandle);
+ return LogAgent::instance()->saLogFinalize(logHandle);
}
/**
@@ -142,7 +142,7 @@ SaAisErrorT saLogStreamOpen_2(
const SaLogFileCreateAttributesT_2* logFileCreateAttributes,
SaLogStreamOpenFlagsT logStreamOpenFlags, SaTimeT timeOut,
SaLogStreamHandleT* logStreamHandle) {
- return LogAgent::instance().saLogStreamOpen_2(
+ return LogAgent::instance()->saLogStreamOpen_2(
logHandle, logStreamName, logFileCreateAttributes, logStreamOpenFlags,
timeOut, logStreamHandle);
}
@@ -161,7 +161,7 @@ SaAisErrorT saLogStreamOpenAsync_2(
SaLogHandleT logHandle, const SaNameT* logStreamName,
const SaLogFileCreateAttributesT_2* logFileCreateAttributes,
SaLogStreamOpenFlagsT logstreamOpenFlags, SaInvocationT invocation) {
- return LogAgent::instance().saLogStreamOpenAsync_2(
+ return LogAgent::instance()->saLogStreamOpenAsync_2(
logHandle, logStreamName, logFileCreateAttributes, logstreamOpenFlags,
invocation);
}
@@ -176,7 +176,7 @@ SaAisErrorT saLogStreamOpenAsync_2(
*/
SaAisErrorT saLogWriteLog(SaLogStreamHandleT logStreamHandle, SaTimeT timeOut,
const SaLogRecordT* logRecord) {
- return LogAgent::instance().saLogWriteLog(logStreamHandle, timeOut,
+ return LogAgent::instance()->saLogWriteLog(logStreamHandle, timeOut,
logRecord);
}
@@ -193,7 +193,7 @@ SaAisErrorT saLogWriteLogAsync(SaLogStreamHandleT
logStreamHandle,
SaInvocationT invocation,
SaLogAckFlagsT ackFlags,
const SaLogRecordT* logRecord) {
- return LogAgent::instance().saLogWriteLogAsync(logStreamHandle, invocation,
+ return LogAgent::instance()->saLogWriteLogAsync(logStreamHandle, invocation,
ackFlags, logRecord);
}
/**
@@ -204,7 +204,7 @@ SaAisErrorT saLogWriteLogAsync(SaLogStreamHandleT
logStreamHandle,
* @return SaAisErrorT
*/
SaAisErrorT saLogStreamClose(SaLogStreamHandleT logStreamHandle) {
- return LogAgent::instance().saLogStreamClose(logStreamHandle);
+ return LogAgent::instance()->saLogStreamClose(logStreamHandle);
}
/**
@@ -217,5 +217,5 @@ SaAisErrorT saLogStreamClose(SaLogStreamHandleT
logStreamHandle) {
*/
SaAisErrorT saLogLimitGet(SaLogHandleT logHandle, SaLogLimitIdT limitId,
SaLimitValueT* limitValue) {
- return LogAgent::instance().saLogLimitGet(logHandle, limitId, limitValue);
+ return LogAgent::instance()->saLogLimitGet(logHandle, limitId, limitValue);
}
diff --git a/src/log/agent/lga_mds.cc b/src/log/agent/lga_mds.cc
index ca949577f..07db48d2c 100644
--- a/src/log/agent/lga_mds.cc
+++ b/src/log/agent/lga_mds.cc
@@ -516,9 +516,9 @@ static uint32_t lga_lgs_msg_proc(lgsv_msg_t *lgsv_msg,
// Lookup the hdl rec by client_id
LogClient *client = nullptr;
uint32_t id = lgsv_msg->info.cbk_info.lgs_client_id;
- LogAgent::instance().EnterCriticalSection();
- if (nullptr == (client = LogAgent::instance().SearchClientById(id))) {
- LogAgent::instance().LeaveCriticalSection();
+ LogAgent::instance()->EnterCriticalSection();
+ if (nullptr == (client = LogAgent::instance()->SearchClientById(id))) {
+ LogAgent::instance()->LeaveCriticalSection();
TRACE("regid not found");
lga_msg_destroy(lgsv_msg);
return NCSCC_RC_FAILURE;
@@ -527,12 +527,12 @@ static uint32_t lga_lgs_msg_proc(lgsv_msg_t *lgsv_msg,
// @client is being deleted in other thread. DO NOT touch this.
bool updated = false;
if (client->FetchAndIncreaseRefCounter(__func__, &updated) == -1) {
- LogAgent::instance().LeaveCriticalSection();
+ LogAgent::instance()->LeaveCriticalSection();
lga_msg_destroy(lgsv_msg);
TRACE_LEAVE();
return rc;
}
- LogAgent::instance().LeaveCriticalSection();
+ LogAgent::instance()->LeaveCriticalSection();
switch (lgsv_msg->type) {
case LGSV_LGS_CBK_MSG:
@@ -554,7 +554,7 @@ static uint32_t lga_lgs_msg_proc(lgsv_msg_t *lgsv_msg,
TRACE_2("LGSV_CLM_NODE_STATUS_CALLBACK clm_node_status: %d", status);
std::atomic<SaClmClusterChangesT> &clm_node_state =
- LogAgent::instance().atomic_get_clm_node_state();
+ LogAgent::instance()->atomic_get_clm_node_state();
clm_node_state =
lgsv_msg->info.cbk_info.clm_node_status_cbk.clm_node_status;
// A client becomes stale if Node loses CLM Membership.
@@ -644,7 +644,7 @@ static uint32_t lga_mds_svc_evt(struct ncsmds_callback_info
*mds_cb_info) {
TRACE("%s\t NCSMDS_NO_ACTIVE", __func__);
// This is a temporary server down e.g. during switch/fail over
if (mds_cb_info->info.svc_evt.i_svc_id == NCSMDS_SVC_ID_LGS) {
- LogAgent::instance().NoActiveLogServer();
+ LogAgent::instance()->NoActiveLogServer();
}
break;
@@ -658,7 +658,7 @@ static uint32_t lga_mds_svc_evt(struct ncsmds_callback_info
*mds_cb_info) {
// Notify to LOG Agent that no LOG server exist.
// In turn, it will inform to all its log clients
// and all log streams belong to each log client.
- LogAgent::instance().NoLogServer();
+ LogAgent::instance()->NoLogServer();
// Stop the recovery thread if it is running
lga_no_server_state_set();
}
@@ -671,11 +671,11 @@ static uint32_t lga_mds_svc_evt(struct
ncsmds_callback_info *mds_cb_info) {
TRACE("%s\t NCSMDS_UP", __func__);
// Inform to LOG agent that LOG server is up from headless
// and provide it the LOG server destination address too.
- LogAgent::instance().HasActiveLogServer(
+ LogAgent::instance()->HasActiveLogServer(
mds_cb_info->info.svc_evt.i_dest);
- if (LogAgent::instance().waiting_log_server_up() == true) {
+ if (LogAgent::instance()->waiting_log_server_up() == true) {
// Signal waiting thread
- m_NCS_SEL_OBJ_IND(LogAgent::instance().get_lgs_sync_sel());
+ m_NCS_SEL_OBJ_IND(LogAgent::instance()->get_lgs_sync_sel());
}
// Start recovery
lga_serv_recov1state_set();
@@ -1236,7 +1236,7 @@ uint32_t lga_mds_init() {
NCSMDS_INFO mds_info;
uint32_t rc = NCSCC_RC_SUCCESS;
MDS_SVC_ID svc = NCSMDS_SVC_ID_LGS;
- std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance().atomic_get_mds_hdl();
+ std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance()->atomic_get_mds_hdl();
TRACE_ENTER();
// Create the ADEST for LGA and get the pwe hdl
@@ -1307,9 +1307,9 @@ uint32_t lga_mds_msg_sync_send(lgsv_msg_t *i_msg,
lgsv_msg_t **o_msg,
SaTimeT timeout, uint32_t prio) {
NCSMDS_INFO mds_info;
uint32_t rc = NCSCC_RC_SUCCESS;
- std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance().atomic_get_mds_hdl();
+ std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance()->atomic_get_mds_hdl();
std::atomic<MDS_DEST> &lgs_mds_dest =
- LogAgent::instance().atomic_get_lgs_mds_dest();
+ LogAgent::instance()->atomic_get_lgs_mds_dest();
TRACE_ENTER();
@@ -1358,9 +1358,9 @@ uint32_t lga_mds_msg_sync_send(lgsv_msg_t *i_msg,
lgsv_msg_t **o_msg,
******************************************************************************/
uint32_t lga_mds_msg_async_send(lgsv_msg_t *i_msg, uint32_t prio) {
NCSMDS_INFO mds_info;
- std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance().atomic_get_mds_hdl();
+ std::atomic<MDS_HDL> &mds_hdl = LogAgent::instance()->atomic_get_mds_hdl();
std::atomic<MDS_DEST> &lgs_mds_dest =
- LogAgent::instance().atomic_get_lgs_mds_dest();
+ LogAgent::instance()->atomic_get_lgs_mds_dest();
TRACE_ENTER();
osafassert(i_msg != nullptr);
diff --git a/src/log/agent/lga_state.cc b/src/log/agent/lga_state.cc
index 05c7a85ee..970fe27d2 100644
--- a/src/log/agent/lga_state.cc
+++ b/src/log/agent/lga_state.cc
@@ -97,7 +97,7 @@ static void *recovery2_thread(void *dummy) {
// Recover all log clients. During this time, any call of LOG APIs
// return TRY_AGAIN until the recovery is done.
- LogAgent::instance().RecoverAllLogClients();
+ LogAgent::instance()->RecoverAllLogClients();
// All clients are recovered (or removed). Or recovery is aborted
// Change to not recovering state RecoveryState::kNormal
diff --git a/src/log/agent/lga_util.cc b/src/log/agent/lga_util.cc
index f4d0f970a..0bb91aaee 100644
--- a/src/log/agent/lga_util.cc
+++ b/src/log/agent/lga_util.cc
@@ -40,17 +40,17 @@ static unsigned int lga_create() {
unsigned int rc = NCSCC_RC_SUCCESS;
// Create and init sel obj for mds sync
- NCS_SEL_OBJ* lgs_sync_sel = LogAgent::instance().get_lgs_sync_sel();
+ NCS_SEL_OBJ* lgs_sync_sel = LogAgent::instance()->get_lgs_sync_sel();
m_NCS_SEL_OBJ_CREATE(lgs_sync_sel);
std::atomic<bool>& lgs_sync_wait =
- LogAgent::instance().atomic_get_lgs_sync_wait();
+ LogAgent::instance()->atomic_get_lgs_sync_wait();
lgs_sync_wait = true;
// register with MDS
if ((NCSCC_RC_SUCCESS != (rc = lga_mds_init()))) {
rc = NCSCC_RC_FAILURE;
// Delete the lga init instances
- LogAgent::instance().RemoveAllLogClients();
+ LogAgent::instance()->RemoveAllLogClients();
return rc;
}
@@ -64,7 +64,7 @@ static unsigned int lga_create() {
lgs_sync_wait = false;
std::atomic<SaClmClusterChangesT>& clm_state =
- LogAgent::instance().atomic_get_clm_node_state();
+ LogAgent::instance()->atomic_get_clm_node_state();
clm_state = SA_CLM_NODE_JOINED;
// No longer needed
@@ -82,7 +82,7 @@ static unsigned int lga_create() {
unsigned int lga_startup() {
unsigned int rc = NCSCC_RC_SUCCESS;
ScopeLock lock(init_lock);
- std::atomic<MDS_HDL>& mds_hdl = LogAgent::instance().atomic_get_mds_hdl();
+ std::atomic<MDS_HDL>& mds_hdl = LogAgent::instance()->atomic_get_mds_hdl();
TRACE_ENTER();
--
2.19.2
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
