commit 18f2a7b59bd0845d97d1432f8f4f7b24b27edf05
author Marton Szasz <szaszm@cloudera.com> Wed Jan 08 19:49:11 2020 +0100
committer Arpad Boda <aboda@apache.org> Fri Jan 10 10:50:44 2020 +0000
tree 4fd2b480c65b52db580d4d2ca0b52611d90dc861
parent aa42957a2e227df41510047cece3cd606dc1cb6a

MINIFICPP-1116 - Fix error reporting in C2Agent

- Missing CoapProtocol will now result in a fatal error (exception)
instead of segfault. Error reporting is improved.
- C2Agent::protocol_ is now properly deleted
- remove dead code
- avoid unnecessary string copies in loop

Signed-off-by: Arpad Boda <aboda@apache.org>

This closes #706

libminifi/include/c2/C2Agent.h

diff --git a/libminifi/include/c2/C2Agent.h b/libminifi/include/c2/C2Agent.h
index c3945fb..f84c131 100644
--- a/libminifi/include/c2/C2Agent.h
+++ b/libminifi/include/c2/C2Agent.h
@@ -57,7 +57,8 @@
 
   C2Agent(const std::shared_ptr<core::controller::ControllerServiceProvider> &controller, const std::shared_ptr<state::StateMonitor> &updateSink, const std::shared_ptr<Configure> &configure);
 
-  virtual ~C2Agent() {
+  virtual ~C2Agent() noexcept {
+    delete protocol_.load();
   }
 
   /**

libminifi/src/c2/C2Agent.cpp

diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp
index 544e752..7c4b6c2 100644
--- a/libminifi/src/c2/C2Agent.cpp
+++ b/libminifi/src/c2/C2Agent.cpp
@@ -40,12 +40,13 @@
 
 C2Agent::C2Agent(const std::shared_ptr<core::controller::ControllerServiceProvider> &controller, const std::shared_ptr<state::StateMonitor> &updateSink,
                  const std::shared_ptr<Configure> &configuration)
-    : controller_(controller),
+    : heart_beat_period_(3000),
+      max_c2_responses(5),
       update_sink_(updateSink),
       update_service_(nullptr),
+      controller_(controller),
       configuration_(configuration),
-      heart_beat_period_(3000),
-      max_c2_responses(5),
+      protocol_(nullptr),
       logger_(logging::LoggerFactory<C2Agent>::getLogger()) {
   allow_updates_ = true;
 
@@ -154,25 +155,24 @@
       clazz = "CoapProtocol";
     }
     logger_->log_info("Class is %s", clazz);
-    auto protocol = core::ClassLoader::getDefaultClassLoader().instantiateRaw(clazz, clazz);
 
+    auto protocol = core::ClassLoader::getDefaultClassLoader().instantiateRaw(clazz, clazz);
     if (protocol == nullptr) {
-      logger_->log_info("Class %s not found", clazz);
+      logger_->log_warn("Class %s not found", clazz);
       protocol = core::ClassLoader::getDefaultClassLoader().instantiateRaw("CoapProtocol", "CoapProtocol");
       if (!protocol) {
-        logger_->log_info("Attempted to load CoapProtocol. To enable C2, please specify an active protocol for this agent.");
-        return;
-      } else {
-        logger_->log_info("Class is CoapProtocol");
+        const char* errmsg = "Attempted to load CoapProtocol. To enable C2, please specify an active protocol for this agent.";
+        logger_->log_error(errmsg);
+        throw minifi::Exception{ minifi::GENERAL_EXCEPTION, errmsg };
       }
+
+      logger_->log_info("Class is CoapProtocol");
     }
-    C2Protocol *old_protocol = protocol_.exchange(dynamic_cast<C2Protocol*>(protocol));
+
+    // Since !reconfigure, the call comes from the ctor and protocol_ is null, therefore no delete is necessary
+    protocol_.exchange(dynamic_cast<C2Protocol *>(protocol));
 
     protocol_.load()->initialize(controller_, configuration_);
-
-    if (reconfigure && old_protocol != nullptr) {
-      delete old_protocol;
-    }
   } else {
     protocol_.load()->update(configure);
   }
@@ -232,7 +232,7 @@
   if (configure->get("nifi.c2.agent.heartbeat.reporter.classes", "c2.agent.heartbeat.reporter.classes", heartbeat_reporters)) {
     std::vector<std::string> reporters = utils::StringUtils::split(heartbeat_reporters, ",");
     std::lock_guard<std::mutex> lock(heartbeat_mutex);
-    for (auto reporter : reporters) {
+    for (const auto& reporter : reporters) {
       auto heartbeat_reporter_obj = core::ClassLoader::getDefaultClassLoader().instantiate(reporter, reporter);
       if (heartbeat_reporter_obj == nullptr) {
         logger_->log_debug("Could not instantiate %s", reporter);
@@ -248,7 +248,7 @@
   if (configure->get("nifi.c2.agent.trigger.classes", "c2.agent.trigger.classes", trigger_classes)) {
     std::vector<std::string> triggers = utils::StringUtils::split(trigger_classes, ",");
     std::lock_guard<std::mutex> lock(heartbeat_mutex);
-    for (auto trigger : triggers) {
+    for (const auto& trigger : triggers) {
       auto trigger_obj = core::ClassLoader::getDefaultClassLoader().instantiate(trigger, trigger);
       if (trigger_obj == nullptr) {
         logger_->log_debug("Could not instantiate %s", trigger);