Merge pull request #391 from JeffersonLab/nbrei_gluex_fixes

Adjust event processor deletion order

src/libraries/JANA/Services/JComponentManager.cc

@@ -16,21 +16,33 @@ JComponentManager::JComponentManager() {
 
 JComponentManager::~JComponentManager() {
 
-    for (auto* src : m_evt_srces) {
-        delete src;
-    }
-    for (auto* proc : m_evt_procs) {
-        delete proc;
+    for (auto* src_gen : m_src_gens) {
+        delete src_gen;
     }
     for (auto* fac_gen : m_fac_gens) {
         delete fac_gen;
     }
-    for (auto* src_gen : m_src_gens) {
-        delete src_gen;
+
+    for (auto* src : m_evt_srces) {
+        LOG_TRACE(GetLogger()) << "Destroying source with type=" << src->GetTypeName();
+        delete src;
     }
     for (auto* unfolder : m_unfolders) {
+        LOG_TRACE(GetLogger()) << "Destroying unfolder with type=" << unfolder->GetTypeName();
         delete unfolder;
     }
+
+    // The order of deletion here sadly matters, because GlueX likes to fill
+    // histograms inside component destructors. Thus event processors must be destroyed after 
+    // all other components, and the last component to be destroyed should be the _first_ event 
+    // processor, because that's the one that is usually provided by the main program and handles 
+    // the ROOT output file. We may eventually want to move all ROOT file operations to a separate
+    // JService which would be closed and destroyed after all other components.
+
+    for (int i=m_evt_procs.size()-1; i >= 0; --i) {
+        LOG_TRACE(GetLogger()) << "Destroying processor with type=" << m_evt_procs[i]->GetTypeName();
+        delete m_evt_procs[i];
+    }
 }
 
 void JComponentManager::Init() {

src/libraries/JANA/Topology/JTopologyBuilder.cc

@@ -28,9 +28,6 @@ JTopologyBuilder::~JTopologyBuilder() {
     for (auto pool : pools) {
         delete pool;
     }
-    if (event_pool != nullptr) {
-        delete event_pool;
-    }
 }
 
 std::string JTopologyBuilder::print_topology() {
@@ -92,8 +89,6 @@ void JTopologyBuilder::create_topology() {
     mapping.initialize(static_cast<JProcessorMapping::AffinityStrategy>(m_affinity),
                        static_cast<JProcessorMapping::LocalityStrategy>(m_locality));
 
-    event_pool = new JEventPool(m_components, m_max_inflight_events, m_location_count);
-
     if (m_configure_topology) {
         m_configure_topology(*this);
         LOG_WARN(GetLogger()) << "Found custom topology configurator! Modified arrow topology is: \n" << print_topology() << LOG_END;
@@ -246,6 +241,7 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par
     // --------------------------
     JEventPool* pool_at_level = new JEventPool(m_components, m_max_inflight_events, m_location_count, current_level);
     pools.push_back(pool_at_level); // Hand over ownership of the pool to the topology
+    LOG_INFO(GetLogger()) << "Created event pool with level=" << current_level << " and size=" << m_max_inflight_events;
 
     // --------------------------
     // 1. Source

src/libraries/JANA/Topology/JTopologyBuilder.h

@@ -40,7 +40,6 @@ class JTopologyBuilder : public JService {
 
     // Things that probably shouldn't be here
     std::function<void(JTopologyBuilder&)> m_configure_topology;
-    JEventPool* event_pool = nullptr; // TODO: Move into pools eventually
     JProcessorMapping mapping;
 
 public: