Merge pull request #288 from JeffersonLab/nbrei_eicrecon_fixes_2

Small fixes inspired by debugging eicrecon

src/libraries/JANA/JFactory.cc

@@ -14,7 +14,7 @@ void JFactory::Create(const std::shared_ptr<const JEvent>& event) {
 
     if (mStatus == Status::Uninitialized) {
         try {
-            std::call_once(mInitFlag, &JFactory::Init, this);
+            Init();
             mStatus = Status::Unprocessed;
         }
         catch (JException& ex) {

src/libraries/JANA/JFactory.h

@@ -202,9 +202,6 @@ class JFactory {
 
     CreationStatus mCreationStatus = CreationStatus::NotCreatedYet;
 
-    // Used to make sure Init is called only once
-    std::once_flag mInitFlag;
-
     // Common to components
     JEventLevel mLevel = JEventLevel::Event;
     std::string mPluginName;

src/libraries/JANA/Services/JComponentManager.cc

@@ -36,12 +36,13 @@ void JComponentManager::Init() {
     m_params->SetDefaultParameter("record_call_stack", m_enable_call_graph_recording, "Records a trace of who called each factory. Reduces performance but necessary for plugins such as janadot.");
     m_params->SetDefaultParameter("jana:nevents", m_nevents, "Max number of events that sources can emit");
     m_params->SetDefaultParameter("jana:nskip", m_nskip, "Number of events that sources should skip before starting emitting");
+    m_params->SetDefaultParameter("autoactivate", m_autoactivate, "List of factories to activate regardless of what the event processors request. Format is typename:tag,typename:tag");
     m_params->FilterParameters(m_default_tags, "DEFTAG:");
 
     // Look for factories to auto-activate
-    // Right now AutoActivator parameter won't show up in parameters list. Reconsider this.
-    if (JAutoActivator::IsRequested(m_params())) {
+    if (!m_autoactivate.empty()) {
         add(new JAutoActivator);
+        // JAutoActivator will re-parse the autoactivate list by itself
     }
 }
 

src/libraries/JANA/Services/JComponentManager.h

@@ -65,6 +65,7 @@ class JComponentManager : public JService {
 
     std::map<std::string, std::string> m_default_tags;
     bool m_enable_call_graph_recording = false;
+    std::string m_autoactivate;
 
     uint64_t m_nskip=0;
     uint64_t m_nevents=0;

src/libraries/JANA/Utils/JAutoActivator.cc

@@ -9,10 +9,6 @@ JAutoActivator::JAutoActivator() {
     SetTypeName("JAutoActivator");
 }
 
-bool JAutoActivator::IsRequested(JParameterManager& params) {
-    return params.Exists("autoactivate") && (!params.GetParameterValue<string>("autoactivate").empty());
-}
-
 void JAutoActivator::AddAutoActivatedFactory(string factory_name, string factory_tag) {
     m_auto_activated_factories.push_back({std::move(factory_name), std::move(factory_tag)});
 }
@@ -67,7 +63,8 @@ void JAutoActivator::Init() {
             }
         }
         catch (...) {
-            LOG << "Error parsing AUTOACTIVATE=" << autoactivate_conf << LOG_END;
+            LOG_ERROR(GetLogger()) << "Error parsing parameter 'autoactivate'. Found: " << autoactivate_conf << LOG_END;
+            throw JException("AutoActivator could not parse parameter 'autoactivate'");
         }
     }
 }
@@ -81,7 +78,8 @@ void JAutoActivator::Process(const std::shared_ptr<const JEvent> &event) {
             factory->Create(event); // This will do nothing if factory is already created
         }
         else {
-            LOG << "Warning: Could not find factory with name=" << name << ", tag=" << tag << LOG_END;
+            LOG_ERROR(GetLogger()) << "Could not find factory with typename=" << name << ", tag=" << tag << LOG_END;
+            throw JException("AutoActivator could not find factory with typename=%s, tag=%s", name.c_str(), tag.c_str());
         }
     }
 }

src/libraries/JANA/Utils/JAutoActivator.h

@@ -16,7 +16,6 @@ class JAutoActivator : public JEventProcessor {
 
 public:
     JAutoActivator();
-    static bool IsRequested(JParameterManager& params);
     static std::pair<std::string, std::string> Split(std::string factory_name);
     void AddAutoActivatedFactory(string factory_name, string factory_tag);
     void Init() override;

src/libraries/JANA/Utils/JProcessorMapping.cc

@@ -165,12 +165,11 @@ std::ostream& operator<<(std::ostream& os, const JProcessorMapping::LocalityStra
 
 std::ostream& operator<<(std::ostream& os, const JProcessorMapping& m) {
 
-    os << "NUMA Configuration" << std::endl;
-    os << "  Affinity strategy: " << m.m_affinity_strategy << std::endl;
-    os << "  Locality strategy: " << m.m_locality_strategy << std::endl;
+    os << "NUMA Configuration: " << "affinity=" << m.m_affinity_strategy << ", locality=" << m.m_locality_strategy;
     if (m.m_locality_strategy != JProcessorMapping::LocalityStrategy::Global) {
-        os << "  Location count: " << m.m_loc_count << std::endl;
+        os << " (" << m.m_loc_count << " locations)";
     }
+    os << std::endl;
 
     if (m.m_initialized) {
         JTablePrinter table;

src/programs/unit_tests/JFactoryTests.cc

@@ -197,5 +197,27 @@ TEST_CASE("JFactoryTests") {
         REQUIRE((*it)->data == 49);
         REQUIRE(sut.GetNumObjects() == 1);
     }
+
+    SECTION("Exception in JFactory::Process") {
+        LOG << "JFactoryTests: Exception in JFactory::Process" << LOG_END;
+        auto event = std::make_shared<JEvent>();
+        JFactoryTestExceptingFactory fac;
+        REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized);
+        REQUIRE_THROWS(fac.CreateAndGetData(event));
+
+        REQUIRE(fac.GetStatus() == JFactory::Status::Unprocessed);
+        REQUIRE_THROWS(fac.CreateAndGetData(event));
+    }
+
+    SECTION("Exception in JFactory::Init") {
+        LOG << "JFactoryTests: Exception in JFactory::Init" << LOG_END;
+        auto event = std::make_shared<JEvent>();
+        JFactoryTestExceptingInInitFactory fac;
+        REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized);
+        REQUIRE_THROWS(fac.CreateAndGetData(event));
+
+        REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized);
+        REQUIRE_THROWS(fac.CreateAndGetData(event));
+    }
 }
 

src/programs/unit_tests/JFactoryTests.h

@@ -50,6 +50,22 @@ struct JFactoryTestDummyFactory : public JFactoryT<JFactoryTestDummyObject> {
     }
 };
 
+struct JFactoryTestExceptingFactory : public JFactoryT<JFactoryTestDummyObject> {
+
+    void Process(const std::shared_ptr<const JEvent>&) override {
+        throw JException("This was never going to work!");
+    }
+};
+
+struct JFactoryTestExceptingInInitFactory : public JFactoryT<JFactoryTestDummyObject> {
+
+    void Init() override {
+        throw JException("This was never going to initialize even");
+    }
+    void Process(const std::shared_ptr<const JEvent>&) override {
+    }
+};
+
 
 struct JFactoryTestDummySource: public JEventSource {