From bcd908c68267d557f8c95920f415a11572ad3c04 Mon Sep 17 00:00:00 2001
From: "P.-A. Loizeau" <p.-a.loizeau@gsi.de>
Date: Thu, 21 Jul 2022 17:04:41 +0200
Subject: [PATCH] Fix segfault at exit of unpacking macros

- Prevent FairrootManager from touching the source object trhogh its pointer after the run is over
- Explicitely delete the dynamically allocated Run and Source objects in the macro (give up share ownership of config objects)
- Give up ownership of the config objects, which triggers their destruction before reaching end of the macro (last owner of shared pointer)

Problem was that the FairrootManager was never going out of scope and was not calling the Source destructor.
So all shared pointers and the source objects were destroyed only when going out of scope in the ROOT session, at which time ROOT already deleted some TF1's, THx's and Canvases without asking anybody (global Lists).
The source and config destructors were then calling destructors for their members and member's members, where we tried to cleanup the memory we assigned but which ROOT already deleted.
---
 macro/beamtime/mcbm2022/mcbm_unp_event.C | 20 ++++++++
 macro/run/.gitignore                     |  1 +
 macro/run/run_unpack_online.C            | 61 +++++++++++++++++-------
 macro/run/run_unpack_online_bmon.C       | 16 +++++++
 macro/run/run_unpack_tsa.C               | 23 ++++++++-
 macro/run/run_unpack_tsa_bmon.C          | 16 +++++++
 reco/steer/CbmRecoUnpack.h               |  2 +-
 reco/steer/CbmSourceTsArchive.h          |  5 +-
 8 files changed, 125 insertions(+), 19 deletions(-)

diff --git a/macro/beamtime/mcbm2022/mcbm_unp_event.C b/macro/beamtime/mcbm2022/mcbm_unp_event.C
index 0a1188ef37..f2bb0ef9a9 100644
--- a/macro/beamtime/mcbm2022/mcbm_unp_event.C
+++ b/macro/beamtime/mcbm2022/mcbm_unp_event.C
@@ -210,6 +210,9 @@ Bool_t mcbm_unp_event(bool bBmoninTof = false,
     }
 
     stsconfig->SetWalkMap(walkMap);
+    walkMap.clear();
+    delete parMod;
+    delete parAsic;
   }
   // -------------
 
@@ -499,6 +502,23 @@ Bool_t mcbm_unp_event(bool bBmoninTof = false,
   std::cout << "After CpuTime = " << timer.CpuTime() << " s RealTime = " << timer.RealTime() << " s." << std::endl;
   // ------------------------------------------------------------------------
 
+  // --   Release all shared pointers to config before ROOT destroys things -
+  // => We need to destroy things by hand because run->Finish calls (trhought the FairRootManager) Source->Close which
+  //    does call the Source destructor, so due to share pointer things stay alive until out of macro scope...
+  run->SetSource(nullptr);
+  delete run;
+  delete source;
+
+  bmonconfig.reset();
+  stsconfig.reset();
+  muchconfig.reset();
+  trd1Dconfig.reset();
+  trdfasp2dconfig.reset();
+  tofconfig.reset();
+  richconfig.reset();
+  psdconfig.reset();
+  // ------------------------------------------------------------------------
+
   return kTRUE;
 }  // End of main macro function
 
diff --git a/macro/run/.gitignore b/macro/run/.gitignore
index 70c6a75444..6979d2cf3b 100644
--- a/macro/run/.gitignore
+++ b/macro/run/.gitignore
@@ -1,3 +1,4 @@
 all_*.par
 data/*.root
 data/qa/*.root
+CbmRecoUnpack.perf.root
diff --git a/macro/run/run_unpack_online.C b/macro/run/run_unpack_online.C
index e3ef0f8a44..5ddcb0235f 100644
--- a/macro/run/run_unpack_online.C
+++ b/macro/run/run_unpack_online.C
@@ -140,7 +140,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     if (2160 <= runid) {
       richconfig->SetSystemTimeOffset(50);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       richconfig->SetSystemTimeOffset(100);  // [ns] value to be updated
     }
     if (runid == 1588) richconfig->MaskDiRICH(0x7150);
@@ -169,7 +169,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     if (2160 <= runid) {
       stsconfig->SetSystemTimeOffset(-1075);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       stsconfig->SetSystemTimeOffset(-970);  // [ns] value to be updated
     }
 
@@ -211,14 +211,18 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     int sensor, asic;
     std::ifstream asicTimeWalk_par(Form("%s/mStsAsicTimeWalk.par", parfilesbasepathSts.data()));
     while (asicTimeWalk_par >> std::hex >> sensor >> std::dec >> asic >> p0 >> p1 >> p2 >> p3) {
-      std::cout << Form("Setting time-walk parametersfor: module %x, ASIC %u\n", sensor, asic);
+      // std::cout << Form("Setting time-walk parameters for: module %x, ASIC %u\n", sensor, asic);
       parAsic->SetWalkCoef({p0, p1, p2, p3});
 
       if (walkMap.find(sensor) == walkMap.end()) { walkMap[sensor] = CbmStsParModule(*parMod); }
       walkMap[sensor].SetAsic(asic, *parAsic);
+      // std::cout << Form("Done with time-walk parameters for: module %x, ASIC %u\n", sensor, asic);
     }
 
     stsconfig->SetWalkMap(walkMap);
+    walkMap.clear();
+    delete parMod;
+    delete parAsic;
   }
   // -------------
 
@@ -236,13 +240,21 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
       /// Starting to use CRI Based MUCH setup with 2GEM and 1 RPC since 09/03/2022 Carbon run
       muchconfig->SetParFileName("mMuchParUpto26032022.par");
     }
-    else if (2350 <= runid && runid <= 2367) {
-      /// First nickel runs
-      muchconfig->SetParFileName("mMuchParNickel_23052022.par");
+    else if (2163 <= runid && runid <= 2291) {
+      ///
+      muchconfig->SetParFileName("mMuchParUpto03042022.par");
     }
-    else if (2367 < runid) {
+    else if (2311 <= runid && runid <= 2315) {
+      ///
+      muchconfig->SetParFileName("mMuchParUpto10042022.par");
+    }
+    else if (2316 <= runid && runid <= 2366) {
+      ///
+      muchconfig->SetParFileName("mMuchParUpto23052022.par");
+    }
+    else if (2367 <= runid && runid <= 2397) {
       /// Starting to use GEM 2 moved to CRI 0 on 24/05/2022
-      muchconfig->SetParFileName("mMuchParNickel_25052022.par");
+      muchconfig->SetParFileName("mMuchParUpto26052022.par");
     }
 
     /// Enable duplicates rejection, Ignores the ADC for duplicates check
@@ -253,7 +265,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     if (2160 <= runid) {
       muchconfig->SetSystemTimeOffset(-1020);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       muchconfig->SetSystemTimeOffset(-980);  // [ns] value to be updated
     }
 
@@ -286,7 +298,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     if (2160 <= runid) {
       trd1Dconfig->SetSystemTimeOffset(1140);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       trd1Dconfig->SetSystemTimeOffset(1300);  // [ns] value to be updated
     }
   }
@@ -300,9 +312,9 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     // trdfasp2dconfig->SetDebugState();
     trdfasp2dconfig->SetDoWriteOutput();
     // Activate the line below to write Trd1D digis to a separate "TrdFaspDigi" branch. Can be used to separate between Fasp and Spadic digis
-    //trdfasp2dconfig->SetOutputBranchName("TrdFaspDigi");
+    // trdfasp2dconfig->SetOutputBranchName("TrdFaspDigi");
     uint8_t map[NFASPMOD];
-    uint16_t cri_map[NCRIMOD];
+    uint16_t crob_map[NCROBMOD];
     for (int i(0); i < NFASPMOD; i++)
       map[i] = i;
     if (runid <= 1588) {
@@ -332,14 +344,14 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
         crob_map[i] = crob_map22[i];
     }
     trdfasp2dconfig->SetFaspMapping(5, map);
-    trdfasp2dconfig->SetCriMapping(5, cri_map);
+    trdfasp2dconfig->SetCrobMapping(5, crob_map);
     std::string parfilesbasepathTrdfasp2d = Form("%s/parameters/trd", srcDir.Data());
     trdfasp2dconfig->SetParFilesBasePath(parfilesbasepathTrdfasp2d);
     trdfasp2dconfig->SetSystemTimeOffset(-1800);  // [ns] value to be updated
     if (2160 <= runid) {
       trdfasp2dconfig->SetSystemTimeOffset(-570);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       trdfasp2dconfig->SetSystemTimeOffset(-510);  // [ns] value to be updated
     }
     trdfasp2dconfig->SetMonitor(dynamic_pointer_cast<CbmTrdUnpackFaspMonitor>(GetTrdMonitor(outfilename, 1)));
@@ -392,7 +404,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
     if (2160 <= runid) {
       tofconfig->SetSystemTimeOffset(0);  // [ns] value to be updated
     }
-    if (2350 <= uRunId) {
+    if (2350 <= runid) {
       tofconfig->SetSystemTimeOffset(40);  // [ns] value to be updated
     }
     if (runid <= 1659) {
@@ -419,7 +431,7 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
       if (2160 <= runid) {
         bmonconfig->SetSystemTimeOffset(-80);  // [ns] value to be updated
       }
-      if (2350 <= uRunId) {
+      if (2350 <= runid) {
         bmonconfig->SetSystemTimeOffset(0);  // [ns] value to be updated
       }
       /// Enable Monitor plots
@@ -504,6 +516,23 @@ void run_unpack_online(std::vector<std::string> publisher = {"tcp://localhost:55
   std::cout << "After CpuTime = " << timer.CpuTime() << " s RealTime = " << timer.RealTime() << " s." << std::endl;
   // ------------------------------------------------------------------------
 
+  // --   Release all shared pointers to config before ROOT destroys things -
+  // => We need to destroy things by hand because run->Finish calls (trhought the FairRootManager) Source->Close which
+  //    does call the Source destructor, so due to share pointer things stay alive until out of macro scope...
+  run->SetSource(nullptr);
+  delete run;
+  delete source;
+
+  bmonconfig.reset();
+  stsconfig.reset();
+  muchconfig.reset();
+  trd1Dconfig.reset();
+  trdfasp2dconfig.reset();
+  tofconfig.reset();
+  richconfig.reset();
+  psdconfig.reset();
+  // ------------------------------------------------------------------------
+
 }  // End of main macro function
 
 
diff --git a/macro/run/run_unpack_online_bmon.C b/macro/run/run_unpack_online_bmon.C
index 00c6ef3916..07802aab9b 100644
--- a/macro/run/run_unpack_online_bmon.C
+++ b/macro/run/run_unpack_online_bmon.C
@@ -109,6 +109,12 @@ void run_unpack_online_bmon(std::vector<std::string> publisher = {"tcp://localho
     bmonconfig->SetParFilesBasePath(parfilesbasepathBmon);
     bmonconfig->SetParFileName("mBmonCriPar.par");
     bmonconfig->SetSystemTimeOffset(-1220);  // [ns] value to be updated
+    if (2160 <= runid) {
+      bmonconfig->SetSystemTimeOffset(-80);  // [ns] value to be updated
+    }
+    if (2350 <= runid) {
+      bmonconfig->SetSystemTimeOffset(0);  // [ns] value to be updated
+    }
     /// Enable Monitor plots
     bmonconfig->SetMonitor(GetTofMonitor(outfilename, true));
     if (2337 <= runid) {
@@ -187,6 +193,16 @@ void run_unpack_online_bmon(std::vector<std::string> publisher = {"tcp://localho
   std::cout << "After CpuTime = " << timer.CpuTime() << " s RealTime = " << timer.RealTime() << " s." << std::endl;
   // ------------------------------------------------------------------------
 
+  // --   Release all shared pointers to config before ROOT destroys things -
+  // => We need to destroy things by hand because run->Finish calls (trhought the FairRootManager) Source->Close which
+  //    does call the Source destructor, so due to share pointer things stay alive until out of macro scope...
+  run->SetSource(nullptr);
+  delete run;
+  delete source;
+
+  bmonconfig.reset();
+  // ------------------------------------------------------------------------
+
 }  // End of main macro function
 
 /**
diff --git a/macro/run/run_unpack_tsa.C b/macro/run/run_unpack_tsa.C
index c59c3c6813..6227f7cc18 100644
--- a/macro/run/run_unpack_tsa.C
+++ b/macro/run/run_unpack_tsa.C
@@ -229,14 +229,18 @@ void run_unpack_tsa(std::vector<std::string> infile = {"test.tsa"}, UInt_t runid
     int sensor, asic;
     std::ifstream asicTimeWalk_par(Form("%s/mStsAsicTimeWalk.par", parfilesbasepathSts.data()));
     while (asicTimeWalk_par >> std::hex >> sensor >> std::dec >> asic >> p0 >> p1 >> p2 >> p3) {
-      std::cout << Form("Setting time-walk parametersfor: module %x, ASIC %u\n", sensor, asic);
+      // std::cout << Form("Setting time-walk parameters for: module %x, ASIC %u\n", sensor, asic);
       parAsic->SetWalkCoef({p0, p1, p2, p3});
 
       if (walkMap.find(sensor) == walkMap.end()) { walkMap[sensor] = CbmStsParModule(*parMod); }
       walkMap[sensor].SetAsic(asic, *parAsic);
+      // std::cout << Form("Done with time-walk parameters for: module %x, ASIC %u\n", sensor, asic);
     }
 
     stsconfig->SetWalkMap(walkMap);
+    walkMap.clear();
+    delete parMod;
+    delete parAsic;
   }
   // -------------
 
@@ -522,6 +526,23 @@ void run_unpack_tsa(std::vector<std::string> infile = {"test.tsa"}, UInt_t runid
   std::cout << "After CpuTime = " << timer.CpuTime() << " s RealTime = " << timer.RealTime() << " s." << std::endl;
   // ------------------------------------------------------------------------
 
+  // --   Release all shared pointers to config before ROOT destroys things -
+  // => We need to destroy things by hand because run->Finish calls (trhought the FairRootManager) Source->Close which
+  //    does call the Source destructor, so due to share pointer things stay alive until out of macro scope...
+  run->SetSource(nullptr);
+  delete run;
+  delete source;
+
+  bmonconfig.reset();
+  stsconfig.reset();
+  muchconfig.reset();
+  trd1Dconfig.reset();
+  trdfasp2dconfig.reset();
+  tofconfig.reset();
+  richconfig.reset();
+  psdconfig.reset();
+  // ------------------------------------------------------------------------
+
 }  // End of main macro function
 
 
diff --git a/macro/run/run_unpack_tsa_bmon.C b/macro/run/run_unpack_tsa_bmon.C
index aac144c4e7..8cc767029f 100644
--- a/macro/run/run_unpack_tsa_bmon.C
+++ b/macro/run/run_unpack_tsa_bmon.C
@@ -122,6 +122,12 @@ void run_unpack_tsa_bmon(std::vector<std::string> infile = {"test.tsa"}, UInt_t
     bmonconfig->SetParFilesBasePath(parfilesbasepathBmon);
     bmonconfig->SetParFileName("mBmonCriPar.par");
     bmonconfig->SetSystemTimeOffset(-1220);  // [ns] value to be updated
+    if (2160 <= runid) {
+      bmonconfig->SetSystemTimeOffset(-80);  // [ns] value to be updated
+    }
+    if (2350 <= runid) {
+      bmonconfig->SetSystemTimeOffset(0);  // [ns] value to be updated
+    }
     /// Enable Monitor plots
     bmonconfig->SetMonitor(GetTofMonitor(outfilename, true));
     if (2337 <= runid) {
@@ -191,6 +197,16 @@ void run_unpack_tsa_bmon(std::vector<std::string> infile = {"test.tsa"}, UInt_t
   std::cout << "After CpuTime = " << timer.CpuTime() << " s RealTime = " << timer.RealTime() << " s." << std::endl;
   // ------------------------------------------------------------------------
 
+  // --   Release all shared pointers to config before ROOT destroys things -
+  // => We need to destroy things by hand because run->Finish calls (trhought the FairRootManager) Source->Close which
+  //    does call the Source destructor, so due to share pointer things stay alive until out of macro scope...
+  run->SetSource(nullptr);
+  delete run;
+  delete source;
+
+  bmonconfig.reset();
+  // ------------------------------------------------------------------------
+
 }  // End of main macro function
 
 
diff --git a/reco/steer/CbmRecoUnpack.h b/reco/steer/CbmRecoUnpack.h
index d0af724581..4d7c3dfb0e 100644
--- a/reco/steer/CbmRecoUnpack.h
+++ b/reco/steer/CbmRecoUnpack.h
@@ -61,7 +61,7 @@ public:
 
 
   /** @brief Destructor **/
-  ~CbmRecoUnpack() {};
+  ~CbmRecoUnpack() { LOG(debug) << "CbmRecoUnpack::~CbmRecoUnpack!"; };
 
   /** @brief Copy constructor - not implemented **/
   CbmRecoUnpack(const CbmRecoUnpack&) = delete;
diff --git a/reco/steer/CbmSourceTsArchive.h b/reco/steer/CbmSourceTsArchive.h
index 15e622e1d6..3ff171c078 100644
--- a/reco/steer/CbmSourceTsArchive.h
+++ b/reco/steer/CbmSourceTsArchive.h
@@ -42,7 +42,10 @@ public:
 
 
   /** @brief Destructor **/
-  virtual ~CbmSourceTsArchive() {};
+  virtual ~CbmSourceTsArchive()
+  {
+    if (fTsSource) delete fTsSource;
+  }
 
 
   /** @brief Copy constructor - not implemented **/
-- 
GitLab