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] 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