When THREAD_TRACE_BUFFER enables, it uses by default a list 10240 trace string 
as
a circular buffer. There is a concern that how much extra memory of all threads
may use.

This patch makes THREAD_TRACE_BUFFER variable represent the number of trace 
string
in buffer, and each trace string has at most 256 character length.
This patch also includes a fix of incorrect increment of @index_ in the method
LogTraceBuffer::WriteToBuffer.
---
 src/amf/amfd/amfd.conf      |  6 +++++-
 src/base/logtrace.cc        | 16 +++++++++-------
 src/base/logtrace_buffer.cc | 41 ++++++++++++++++++++++++-----------------
 src/base/logtrace_buffer.h  |  3 +++
 4 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/src/amf/amfd/amfd.conf b/src/amf/amfd/amfd.conf
index 8b4fe75..d7827fe 100644
--- a/src/amf/amfd/amfd.conf
+++ b/src/amf/amfd/amfd.conf
@@ -28,4 +28,8 @@ export AVSV_HB_PERIOD=10000000000
 # THREAD_TRACE_BUFFER variable enables the tracing, writes the trace
 # to thread based buffer in circular fashion. The trace buffers will
 # be flushed to file if an abnormal end hits, i.e. LOG_ER is called
-# export THREAD_TRACE_BUFFER=1
+# The value of THREAD_TRACE_BUFFER indicates the number of trace string
+# in a buffer. The length of a trace string is at most 256 characters.
+# It can be disabled if set THREAD_TRACE_BUFFER as 0, the maximum value
+# can be set as 65535.
+# export THREAD_TRACE_BUFFER=10240
diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index 1dc1ae0..f755d71 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -42,7 +42,7 @@ char *msg_id;
 int logmask;
 const char* osaf_log_file = "osaf.log";
 bool enable_osaf_log = false;
-bool enable_thread_trace_buffer = false;
+size_t thread_trace_buffer_size = 0;
 
 }  // namespace global
 
@@ -56,7 +56,7 @@ LogTraceClient* gl_local_thread_trace = nullptr;
 std::once_flag init_flag;
 
 thread_local LogTraceBuffer gl_thread_buffer{gl_local_thread_trace,
-  LogTraceBuffer::kBufferSize_10K};
+  global::thread_trace_buffer_size};
 
 static pid_t gettid() { return syscall(SYS_gettid); }
 
@@ -107,7 +107,7 @@ void trace_output(const char *file, unsigned line, unsigned 
priority,
         static_cast<base::LogMessage::Severity>(priority), preamble, ap);
   }
   // thread trace
-  if (global::enable_thread_trace_buffer == true &&
+  if (global::thread_trace_buffer_size > 0 &&
       (category == CAT_TRACE_ENTER || category == CAT_TRACE_LEAVE)) {
     // reuse @entry if legacy trace is enabled
     if (!entry) {
@@ -131,7 +131,7 @@ void log_output(const char *file, unsigned line, unsigned 
priority,
   LogTraceClient::Log(gl_remote_osaflog,
       static_cast<base::LogMessage::Severity>(priority), preamble, ap);
   // Flush the thread buffer for logging error or lower
-  if (global::enable_thread_trace_buffer == true && gl_local_thread_trace &&
+  if (global::thread_trace_buffer_size > 0 && gl_local_thread_trace &&
       static_cast<base::LogMessage::Severity>(priority) <=
       base::LogMessage::Severity::kErr) {
     gl_thread_buffer.RequestFlush();
@@ -187,7 +187,7 @@ void logtrace_trace(const char *file, unsigned line, 
unsigned category,
   va_list ap;
 
   if (is_logtrace_enabled(category) == false &&
-      global::enable_thread_trace_buffer == false) return;
+      global::thread_trace_buffer_size == 0) return;
 
   va_start(ap, format);
   trace_output(file, line, LOG_DEBUG, category, format, ap);
@@ -198,6 +198,7 @@ static void logtrace_init_interal(const char *pathname, 
unsigned mask,
     int* result_init) {
   bool result = false;
   char *tmp = nullptr;
+  uint16_t th_buffer_size;
   if (global::msg_id != nullptr) goto done;
   tmp = strdup(pathname);
   if (tmp != nullptr) {
@@ -218,8 +219,9 @@ static void logtrace_init_interal(const char *pathname, 
unsigned mask,
           LogTraceClient::kRemoteBlocking);
     }
   }
-  if (base::GetEnv("THREAD_TRACE_BUFFER", uint32_t{0}) == 1) {
-    global::enable_thread_trace_buffer = true;
+  th_buffer_size = base::GetEnv("THREAD_TRACE_BUFFER", uint16_t{0});
+  if (th_buffer_size > 0) {
+    global::thread_trace_buffer_size = th_buffer_size;
     if (!gl_local_thread_trace) {
       gl_local_thread_trace = new LogTraceClient(global::msg_id,
           LogTraceClient::kLocalBuffer);
diff --git a/src/base/logtrace_buffer.cc b/src/base/logtrace_buffer.cc
index e945dab..9a48bc3 100644
--- a/src/base/logtrace_buffer.cc
+++ b/src/base/logtrace_buffer.cc
@@ -29,33 +29,40 @@ LogTraceBuffer::LogTraceBuffer(LogTraceClient* owner, 
size_t buffer_size) :
   buffer_size_(buffer_size),
   index_(0),
   flush_required_(false) {
-  std::string log_file_name;
-  tid_ = syscall(SYS_gettid);
-  vector_.resize(buffer_size_);
-  if (owner_) {
-    owner_->AddExternalBuffer(tid_, this);
-    log_file_name = std::string(owner_->app_name()) + "_" + owner_->proc_id()
-        + "_" + std::to_string(tid_);
-  } else {
-    log_file_name = std::string(getpid() + "_" + std::to_string(tid_));
+  // ensure we don't allocate any memory when THREAD_TRACE_BUFFER is disabled
+  if (buffer_size_ > 0) {
+    std::string log_file_name;
+    tid_ = syscall(SYS_gettid);
+    vector_.resize(buffer_size_);
+    if (owner_) {
+      owner_->AddExternalBuffer(tid_, this);
+      log_file_name = std::string(owner_->app_name()) + "_" + owner_->proc_id()
+          + "_" + std::to_string(tid_);
+    } else {
+      log_file_name = std::string(getpid() + "_" + std::to_string(tid_));
+    }
+    log_writer_ = new LogWriter{log_file_name, 10,
+      LogWriter::kMaxFileSize_10MB};
   }
-  log_writer_ = new LogWriter{log_file_name, 10, LogWriter::kMaxFileSize_10MB};
 }
 
 LogTraceBuffer::~LogTraceBuffer() {
   if (owner_) {
     owner_->RemoveExternalBuffer(tid_);
   }
-  delete log_writer_;
+  if (log_writer_) delete log_writer_;
 }
 
 void LogTraceBuffer::WriteToBuffer(std::string trace) {
-  vector_[index_] = kLogTraceString + trace;
-  // add break line char
-  if (trace.at(trace.length() - 1) != '\n')
-    vector_[index_] += "\n";
-
-  if (index_++ == buffer_size_) index_ = 0;
+  size_t length = trace.length();
+  if (length > kMaxTraceString) length = kMaxTraceString;
+  vector_[index_] = std::string(kLogTraceString) +
+      std::string(trace.c_str(), length);
+  length = vector_[index_].length();
+  if (trace.at(trace.length() - 1) != '\n') {
+    vector_[index_].replace(length - 1, 1, "\n");
+  }
+  if (++index_ == buffer_size_) index_ = 0;
   if (flush_required_) FlushBuffer();
 }
 void LogTraceBuffer::RequestFlush() {
diff --git a/src/base/logtrace_buffer.h b/src/base/logtrace_buffer.h
index 3ba8c63..18b5bad 100644
--- a/src/base/logtrace_buffer.h
+++ b/src/base/logtrace_buffer.h
@@ -37,12 +37,15 @@ class LogTraceBuffer {
   // Identified string, added at beginning of logtrace. This string
   // can be used as searching key for the log trace records in coredump file
   constexpr static const char* kLogTraceString = "1qaz2wsx";
+  // Maximum characters per trace string
+  static const uint32_t kMaxTraceString = 256 - strlen(kLogTraceString);
   LogTraceBuffer(LogTraceClient* owner, size_t buffer_size);
   ~LogTraceBuffer();
   void WriteToBuffer(std::string trace);
   bool FlushBuffer();
   void RequestFlush();
   void SetFlush(const bool flush) { flush_required_ = flush; }
+
  private:
   LogTraceClient* owner_;
   const size_t buffer_size_;
-- 
2.7.4


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