You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by sz...@apache.org on 2022/10/07 15:31:41 UTC

[nifi-minifi-cpp] branch main updated: MINIFICPP-1945 Fix memory leak in ConsumeWindowsEventLog

This is an automated email from the ASF dual-hosted git repository.

szaszm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git


The following commit(s) were added to refs/heads/main by this push:
     new d93b84e5a MINIFICPP-1945 Fix memory leak in ConsumeWindowsEventLog
d93b84e5a is described below

commit d93b84e5ae5c8cc2b475a5bcb8934443eb0f4a58
Author: Martin Zink <ma...@apache.org>
AuthorDate: Fri Oct 7 15:34:41 2022 +0200

    MINIFICPP-1945 Fix memory leak in ConsumeWindowsEventLog
    
    MINIFICPP-1946 CWEL reschedule duplicates header_names member fix
    
    Closes #1426
    Signed-off-by: Marton Szasz <sz...@apache.org>
---
 .../windows-event-log/ConsumeWindowsEventLog.cpp   | 38 ++++++-------
 .../windows-event-log/ConsumeWindowsEventLog.h     |  2 +-
 .../windows-event-log/wel/WindowsEventLog.cpp      | 19 ++-----
 extensions/windows-event-log/wel/WindowsEventLog.h | 64 +++++++++-------------
 4 files changed, 52 insertions(+), 71 deletions(-)

diff --git a/extensions/windows-event-log/ConsumeWindowsEventLog.cpp b/extensions/windows-event-log/ConsumeWindowsEventLog.cpp
index 3447ccd8c..a787470fc 100644
--- a/extensions/windows-event-log/ConsumeWindowsEventLog.cpp
+++ b/extensions/windows-event-log/ConsumeWindowsEventLog.cpp
@@ -236,23 +236,23 @@ void ConsumeWindowsEventLog::onSchedule(const std::shared_ptr<core::ProcessConte
   context->getProperty(EventHeaderDelimiter.getName(), header_delimiter_);
   context->getProperty(BatchCommitSize.getName(), batch_commit_size_);
 
-  std::string header;
-  context->getProperty(EventHeader.getName(), header);
-
-  auto keyValueSplit = utils::StringUtils::split(header, ",");
-  for (const auto &kv : keyValueSplit) {
-    auto splitKeyAndValue = utils::StringUtils::split(kv, "=");
-    if (splitKeyAndValue.size() == 2) {
-      auto key = utils::StringUtils::trim(splitKeyAndValue.at(0));
-      auto value = utils::StringUtils::trim(splitKeyAndValue.at(1));
-      if (!insertHeaderName(header_names_, key, value)) {
-        logger_->log_error("%s is an invalid key for the header map", key);
+  header_names_.clear();
+  if (auto header = context->getProperty(EventHeader)) {
+    auto keyValueSplit = utils::StringUtils::split(*header, ",");
+    for (const auto &kv : keyValueSplit) {
+      auto splitKeyAndValue = utils::StringUtils::split(kv, "=");
+      if (splitKeyAndValue.size() == 2) {
+        auto key = utils::StringUtils::trim(splitKeyAndValue.at(0));
+        auto value = utils::StringUtils::trim(splitKeyAndValue.at(1));
+        if (!insertHeaderName(header_names_, key, value)) {
+          logger_->log_error("%s is an invalid key for the header map", key);
+        }
+      } else if (splitKeyAndValue.size() == 1) {
+        auto key = utils::StringUtils::trim(splitKeyAndValue.at(0));
+        if (!insertHeaderName(header_names_, key, "")) {
+          logger_->log_error("%s is an invalid key for the header map", key);
+        }
       }
-    } else if (splitKeyAndValue.size() == 1) {
-     auto key = utils::StringUtils::trim(splitKeyAndValue.at(0));
-     if (!insertHeaderName(header_names_, key, "")) {
-       logger_->log_error("%s is an invalid key for the header map", key);
-     }
     }
   }
 
@@ -435,7 +435,7 @@ void ConsumeWindowsEventLog::onTrigger(const std::shared_ptr<core::ProcessContex
   }
 }
 
-wel::WindowsEventLogHandler ConsumeWindowsEventLog::getEventLogHandler(const std::string & name) {
+wel::WindowsEventLogHandler& ConsumeWindowsEventLog::getEventLogHandler(const std::string & name) {
   logger_->log_trace("Getting Event Log Handler corresponding to %s", name.c_str());
   auto provider = providers_.find(name);
   if (provider != std::end(providers_)) {
@@ -447,7 +447,7 @@ wel::WindowsEventLogHandler ConsumeWindowsEventLog::getEventLogHandler(const std
   LPCWSTR widechar = temp_wstring.c_str();
 
   providers_[name] = wel::WindowsEventLogHandler(EvtOpenPublisherMetadata(NULL, widechar, NULL, 0, 0));
-  logger_->log_trace("Not found the handler -> created handler for %s", name.c_str());
+  logger_->log_info("Handler not found for %s, creating. Number of cached handlers: %zu", name, providers_.size());
   return providers_[name];
 }
 
@@ -591,7 +591,7 @@ bool ConsumeWindowsEventLog::createEventRender(EVT_HANDLE hEvent, EventRender& e
   if (output_.plaintext) {
     logger_->log_trace("Writing event in plain text");
 
-    auto handler = getEventLogHandler(providerName);
+    auto& handler = getEventLogHandler(providerName);
     auto message = handler.getEventMessage(hEvent);
 
     if (!message.empty()) {
diff --git a/extensions/windows-event-log/ConsumeWindowsEventLog.h b/extensions/windows-event-log/ConsumeWindowsEventLog.h
index 651d92f3d..536f0a280 100644
--- a/extensions/windows-event-log/ConsumeWindowsEventLog.h
+++ b/extensions/windows-event-log/ConsumeWindowsEventLog.h
@@ -118,7 +118,7 @@ class ConsumeWindowsEventLog : public core::Processor {
  protected:
   void refreshTimeZoneData();
   void putEventRenderFlowFileToSession(const EventRender& eventRender, core::ProcessSession& session) const;
-  wel::WindowsEventLogHandler getEventLogHandler(const std::string & name);
+  wel::WindowsEventLogHandler& getEventLogHandler(const std::string & name);
   bool insertHeaderName(wel::METADATA_NAMES &header, const std::string &key, const std::string &value) const;
   void LogWindowsError(std::string error = "Error") const;
   bool createEventRender(EVT_HANDLE eventHandle, EventRender& eventRender);
diff --git a/extensions/windows-event-log/wel/WindowsEventLog.cpp b/extensions/windows-event-log/wel/WindowsEventLog.cpp
index 58876321e..1fa624872 100644
--- a/extensions/windows-event-log/wel/WindowsEventLog.cpp
+++ b/extensions/windows-event-log/wel/WindowsEventLog.cpp
@@ -26,11 +26,7 @@
 #include "utils/Deleters.h"
 #include "utils/gsl.h"
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace wel {
+namespace org::apache::nifi::minifi::wel {
 
 void WindowsEventLogMetadataImpl::renderMetadata() {
   DWORD status = ERROR_SUCCESS;
@@ -171,7 +167,7 @@ std::string WindowsEventLogHandler::getEventMessage(EVT_HANDLE eventHandle) cons
   DWORD num_chars_used = 0;
   DWORD status = 0;
 
-  EvtFormatMessage(metadata_provider_, eventHandle, 0, 0, NULL, EvtFormatMessageEvent, num_chars_in_buffer, buffer.get(), &num_chars_used);
+  EvtFormatMessage(metadata_provider_.get(), eventHandle, 0, 0, NULL, EvtFormatMessageEvent, num_chars_in_buffer, buffer.get(), &num_chars_used);
   if (num_chars_used == 0) {
     return returnValue;
   }
@@ -189,7 +185,7 @@ std::string WindowsEventLogHandler::getEventMessage(EVT_HANDLE eventHandle) cons
       return returnValue;
     }
 
-    EvtFormatMessage(metadata_provider_, eventHandle, 0, 0, NULL, EvtFormatMessageEvent, num_chars_in_buffer, buffer.get(), &num_chars_used);
+    EvtFormatMessage(metadata_provider_.get(), eventHandle, 0, 0, NULL, EvtFormatMessageEvent, num_chars_in_buffer, buffer.get(), &num_chars_used);
   }
 
   if (ERROR_EVT_MESSAGE_NOT_FOUND == status || ERROR_EVT_MESSAGE_ID_NOT_FOUND == status) {
@@ -213,12 +209,7 @@ std::string WindowsEventLogHeader::createDefaultDelimiter(size_t max, size_t len
 }
 
 EVT_HANDLE WindowsEventLogHandler::getMetadata() const {
-  return metadata_provider_;
+  return metadata_provider_.get();
 }
 
-} /* namespace wel */
-} /* namespace minifi */
-} /* namespace nifi */
-} /* namespace apache */
-} /* namespace org */
-
+}  // namespace org::apache::nifi::minifi::wel
diff --git a/extensions/windows-event-log/wel/WindowsEventLog.h b/extensions/windows-event-log/wel/WindowsEventLog.h
index c550e5f53..4eb44c5b3 100644
--- a/extensions/windows-event-log/wel/WindowsEventLog.h
+++ b/extensions/windows-event-log/wel/WindowsEventLog.h
@@ -36,31 +36,27 @@
 #include "core/ProcessSession.h"
 #include "utils/OsUtils.h"
 #include "FlowFileRecord.h"
+#include "UniqueEvtHandle.h"
 
 #include "pugixml.hpp"
 
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace wel {
-
-
-  enum METADATA {
-    LOG_NAME,
-    SOURCE,
-    TIME_CREATED,
-    EVENTID,
-    OPCODE,
-    EVENT_RECORDID,
-    EVENT_TYPE,
-    TASK_CATEGORY,
-    LEVEL,
-    KEYWORDS,
-    USER,
-    COMPUTER,
-    UNKNOWN
-  };
+namespace org::apache::nifi::minifi::wel {
+
+enum METADATA {
+  LOG_NAME,
+  SOURCE,
+  TIME_CREATED,
+  EVENTID,
+  OPCODE,
+  EVENT_RECORDID,
+  EVENT_TYPE,
+  TASK_CATEGORY,
+  LEVEL,
+  KEYWORDS,
+  USER,
+  COMPUTER,
+  UNKNOWN
+};
 
 
 // this is a continuous enum so we can rely on the array
@@ -77,11 +73,10 @@ class WindowsEventLogHandler {
 
   std::string getEventMessage(EVT_HANDLE eventHandle) const;
 
-
   EVT_HANDLE getMetadata() const;
 
  private:
-  EVT_HANDLE metadata_provider_;
+  unique_evt_handle metadata_provider_;
 };
 
 class WindowsEventLogMetadata {
@@ -93,9 +88,9 @@ class WindowsEventLogMetadata {
 
   static std::string getMetadataString(METADATA val) {
     static std::map<METADATA, std::string> map = {
-        {LOG_NAME, "LOG_NAME" },
+        {LOG_NAME, "LOG_NAME"},
         {SOURCE, "SOURCE"},
-        {TIME_CREATED, "TIME_CREATED" },
+        {TIME_CREATED, "TIME_CREATED"},
         {EVENTID, "EVENTID"},
         {OPCODE, "OPCODE"},
         {EVENT_RECORDID, "EVENT_RECORDID"},
@@ -110,11 +105,11 @@ class WindowsEventLogMetadata {
     return map[val];
   }
 
-  static METADATA getMetadataFromString(const std::string &val) {
+  static METADATA getMetadataFromString(const std::string& val) {
     static std::map<std::string, METADATA> map = {
         {"LOG_NAME", LOG_NAME},
         {"SOURCE", SOURCE},
-        {"TIME_CREATED", TIME_CREATED },
+        {"TIME_CREATED", TIME_CREATED},
         {"EVENTID", EVENTID},
         {"OPCODE", OPCODE},
         {"EVENT_RECORDID", EVENT_RECORDID},
@@ -176,7 +171,7 @@ class WindowsEventLogHeader {
  public:
   explicit WindowsEventLogHeader(METADATA_NAMES header_names) : header_names_(header_names) {}
 
-  void setDelimiter(const std::string &delim);
+  void setDelimiter(const std::string& delim);
 
   template<typename MetadataCollection>
   std::string getEventHeader(const MetadataCollection& metadata_collection) const;
@@ -192,11 +187,11 @@ template<typename MetadataCollection>
 std::string WindowsEventLogHeader::getEventHeader(const MetadataCollection& metadata_collection) const {
   std::stringstream eventHeader;
   size_t max = 1;
-  for (const auto &option : header_names_) {
+  for (const auto& option : header_names_) {
     max = (std::max(max, option.second.size()));
   }
   ++max;  // increment by one to get space.
-  for (const auto &option : header_names_) {
+  for (const auto& option : header_names_) {
     auto name = option.second;
     if (!name.empty()) {
       eventHeader << name << (delimiter_.empty() ? createDefaultDelimiter(max, name.size()) : delimiter_);
@@ -207,9 +202,4 @@ std::string WindowsEventLogHeader::getEventHeader(const MetadataCollection& meta
   return eventHeader.str();
 }
 
-} /* namespace wel */
-} /* namespace minifi */
-} /* namespace nifi */
-} /* namespace apache */
-} /* namespace org */
-
+}  // namespace org::apache::nifi::minifi::wel