Re: [devel] [PATCH 1/1] base: Place TraceLog instance on heap memory V3 [#2860]

Sun, 27 May 2018 23:32:46 -0700 

Hi Minh,

yes, std::call_once or pthread_once should work.

/Thanks HansN

On 05/28/2018 05:08 AM, Minh Hon Chau wrote:

Hi Hans,
I think it can happen e.g if the traces in agent are enabled for more than 1 services. 

Maybe std::call_once can do?

Thanks
Minh
On 25/05/18 18:33, Hans Nordeback wrote:

Hi Minh,
ack, with a comment/question, if logtrace_init can be called concurrently, the new calls are not thread safe. If so,
(it is the same problem when e.g. making a singleton thread safe, where e.g. "double-checked locking" has been tried, but 

does not work).

Using pthread_once here should work.

/Thanks HansN

On 05/25/2018 05:58 AM, Minh Chau wrote:

When a process calls exit(), the exit_handler trigger
__do_global_dtor_aux then calls TraceLog destructor.
One of process thread is calling TraceLog::Log while
the destructor is called. This leads to a coredump.
The process (which could by any applications) calling
exit() first is responsible to make exit() thread-safe.
However, the TraceLog should also avoid the coredump
occurring at OpenSAF side.

Runtime allocate TraceLog pointer so that its memory
is located in heap. This allocation to avoid its
desctructor to be called as part of __do_global_dtor_aux.
---
  src/base/logtrace.cc        | 21 ++++++++++++---------
  src/base/logtrace_client.cc |  5 +++++
  src/base/logtrace_client.h  |  2 ++
  src/mds/mds_log.cc          | 15 ++++++++-------
  4 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index fd3d829..3d64e4d 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -43,8 +43,8 @@ bool enable_osaf_log = false;
    }  // namespace global
  -static TraceLog gl_trace;
-static TraceLog gl_osaflog;
+TraceLog* gl_trace = nullptr;
+TraceLog* gl_osaflog = nullptr;
    static pid_t gettid() { return syscall(SYS_gettid); }
  @@ -88,8 +88,8 @@ void trace_output(const char *file, unsigned line, unsigned priority, 
    if (strncmp(file, "src/", 4) == 0) file += 4;
    snprintf(preamble, sizeof(preamble), "%d:%s:%u %s %s", gettid(), file, line, 
             global::prefix_name[priority + category], format);
- gl_trace.Log(static_cast<base::LogMessage::Severity>(priority), preamble, 
-                  ap);
+  TraceLog::Log(gl_trace, static_cast<base::LogMessage::Severity>(priority), 
+      preamble, ap);
  }
    void log_output(const char *file, unsigned line, unsigned priority,
@@ -101,8 +101,8 @@ void log_output(const char *file, unsigned line, unsigned priority, 
    if (strncmp(file, "src/", 4) == 0) file += 4;
    snprintf(preamble, sizeof(preamble), "%d:%s:%u %s %s", gettid(), file, line, 
             global::prefix_name[priority + category], format);
- gl_osaflog.Log(static_cast<base::LogMessage::Severity>(priority), preamble, 
-                  ap);
+  TraceLog::Log(gl_osaflog, static_cast<base::LogMessage::Severity>(priority), 
+      preamble, ap);
  }
    void logtrace_log(const char *file, unsigned line, int priority,
@@ -176,11 +176,13 @@ int logtrace_init(const char *, const char *pathname, unsigned mask) { 
      global::msg_id = nullptr;
    }
    if (result && mask != 0) {
-    result = gl_trace.Init(global::msg_id, TraceLog::kBlocking);
+    if (!gl_trace) gl_trace = new TraceLog();
+    result = gl_trace->Init(global::msg_id, TraceLog::kBlocking);
    }
    if (base::GetEnv("OSAF_LOCAL_NODE_LOG", uint32_t{0}) == 1) {
      global::enable_osaf_log = true;
-    gl_osaflog.Init(global::osaf_log_file, TraceLog::kBlocking);
+    if (!gl_osaflog) gl_osaflog = new TraceLog();
+    gl_osaflog->Init(global::osaf_log_file, TraceLog::kBlocking);
    }
    if (result) {
      syslog(LOG_INFO, "logtrace: trace enabled to file '%s', mask=0x%x", 
@@ -221,7 +223,8 @@ int trace_category_set(unsigned mask) {
    if (global::category_mask == 0) {
      syslog(LOG_INFO, "logtrace: trace disabled");
    } else {
-    gl_trace.Init(global::msg_id, TraceLog::kBlocking);
+    if (!gl_trace) gl_trace = new TraceLog();
+    gl_trace->Init(global::msg_id, TraceLog::kBlocking);
      syslog(LOG_INFO, "logtrace: trace enabled to file %s, mask=0x%x",
             global::msg_id, global::category_mask);
    }
diff --git a/src/base/logtrace_client.cc b/src/base/logtrace_client.cc
index 0dac6d3..a9d82e2 100644
--- a/src/base/logtrace_client.cc
+++ b/src/base/logtrace_client.cc
@@ -81,6 +81,11 @@ bool TraceLog::Init(const char *msg_id, WriteMode mode) { 
    return true;
  }
  +void TraceLog::Log(TraceLog* tracelog, base::LogMessage::Severity severity, 
+      const char *fmt, va_list ap) {
+  if (tracelog != nullptr) tracelog->Log(severity, fmt, ap);
+}
+
  void TraceLog::Log(base::LogMessage::Severity severity, const char *fmt, 
                     va_list ap) {
    if (log_socket_ != nullptr && mutex_ != nullptr) {
diff --git a/src/base/logtrace_client.h b/src/base/logtrace_client.h
index eac31d0..21c7d2e 100644
--- a/src/base/logtrace_client.h
+++ b/src/base/logtrace_client.h
@@ -32,6 +32,8 @@ class TraceLog {
      kNonblocking = base::UnixSocket::Mode::kNonblocking,
    };
    bool Init(const char *msg_id, WriteMode mode);
+  static void Log(TraceLog* tracelog, base::LogMessage::Severity severity, 
+      const char *fmt, va_list ap);
    void Log(base::LogMessage::Severity severity, const char *fmt,
                    va_list ap);
    TraceLog();
diff --git a/src/mds/mds_log.cc b/src/mds/mds_log.cc
index 0792975..24bc398 100644
--- a/src/mds/mds_log.cc
+++ b/src/mds/mds_log.cc
@@ -29,7 +29,7 @@
  #include "mds/mds_papi.h"
    int gl_mds_log_level = 3;
-static TraceLog gl_mds_log;
+TraceLog* gl_mds_log = nullptr;
/******************************************************************************* 
   * Funtion Name   :    mds_log_init
@@ -40,7 +40,8 @@ static TraceLog gl_mds_log;
   *
*******************************************************************************/ 
  uint32_t mds_log_init(const char *) {
-  if (!gl_mds_log.Init("mds.log", TraceLog::kNonblocking)) {
+  if (!gl_mds_log) gl_mds_log = new TraceLog();
+  if (!gl_mds_log->Init("mds.log", TraceLog::kNonblocking)) {
      return NCSCC_RC_FAILURE;
    }
    tzset();
@@ -61,7 +62,7 @@ void log_mds_critical(const char *fmt, ...) {
    if (gl_mds_log_level < NCSMDS_LC_CRITICAL) return;
    va_list ap;
    va_start(ap, fmt);
-  gl_mds_log.Log(base::LogMessage::Severity::kCrit, fmt, ap);
+  TraceLog::Log(gl_mds_log, base::LogMessage::Severity::kCrit, fmt, ap); 
    va_end(ap);
  }
  @@ -77,7 +78,7 @@ void log_mds_err(const char *fmt, ...) {
    if (gl_mds_log_level < NCSMDS_LC_ERR) return;
    va_list ap;
    va_start(ap, fmt);
-  gl_mds_log.Log(base::LogMessage::Severity::kErr, fmt, ap);
+  TraceLog::Log(gl_mds_log, base::LogMessage::Severity::kErr, fmt, ap); 
    va_end(ap);
  }
  @@ -93,7 +94,7 @@ void log_mds_notify(const char *fmt, ...) {
    if (gl_mds_log_level < NCSMDS_LC_NOTIFY) return;
    va_list ap;
    va_start(ap, fmt);
-  gl_mds_log.Log(base::LogMessage::Severity::kNotice, fmt, ap);
+  TraceLog::Log(gl_mds_log, base::LogMessage::Severity::kNotice, fmt, ap); 
    va_end(ap);
  }
  @@ -109,7 +110,7 @@ void log_mds_info(const char *fmt, ...) {
    if (gl_mds_log_level < NCSMDS_LC_INFO) return;
    va_list ap;
    va_start(ap, fmt);
-  gl_mds_log.Log(base::LogMessage::Severity::kInfo, fmt, ap);
+  TraceLog::Log(gl_mds_log, base::LogMessage::Severity::kInfo, fmt, ap); 
    va_end(ap);
  }
  @@ -126,6 +127,6 @@ void log_mds_dbg(const char *fmt, ...) {
    if (gl_mds_log_level < NCSMDS_LC_DBG) return;
    va_list ap;
    va_start(ap, fmt);
-  gl_mds_log.Log(base::LogMessage::Severity::kDebug, fmt, ap);
+  TraceLog::Log(gl_mds_log, base::LogMessage::Severity::kDebug, fmt, ap); 
    va_end(ap);
  }








------------------------------------------------------------------------------
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