From 1a5c914792b587086a4acd1c6a92d7482ffb3cc2 Mon Sep 17 00:00:00 2001
From: Nathan Brei <nbrei@jlab.org>
Date: Thu, 5 Dec 2024 15:23:26 -0500
Subject: [PATCH 1/2] Remove obsolete `event_pool` data member from
 JTopologyBuilder

---
 src/libraries/JANA/Topology/JTopologyBuilder.cc | 5 -----
 src/libraries/JANA/Topology/JTopologyBuilder.h  | 1 -
 2 files changed, 6 deletions(-)

diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc
index 451b7fd82..2060fe178 100644
--- a/src/libraries/JANA/Topology/JTopologyBuilder.cc
+++ b/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;
diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.h b/src/libraries/JANA/Topology/JTopologyBuilder.h
index 685efc09f..59a5c8bd0 100644
--- a/src/libraries/JANA/Topology/JTopologyBuilder.h
+++ b/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:

From 2605036d16379907f96eda8d4a2165b38e230472 Mon Sep 17 00:00:00 2001
From: Nathan Brei <nbrei@jlab.org>
Date: Thu, 5 Dec 2024 16:25:42 -0500
Subject: [PATCH 2/2] Adjust component deletion order

This is in response to GlueX filling histograms in component destructors
---
 .../JANA/Services/JComponentManager.cc        | 26 ++++++++++++++-----
 .../JANA/Topology/JTopologyBuilder.cc         |  1 +
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/libraries/JANA/Services/JComponentManager.cc b/src/libraries/JANA/Services/JComponentManager.cc
index e7fe17fd4..3c381bd51 100644
--- a/src/libraries/JANA/Services/JComponentManager.cc
+++ b/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() {
diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc
index 2060fe178..e3e1cca17 100644
--- a/src/libraries/JANA/Topology/JTopologyBuilder.cc
+++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc
@@ -241,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