fix: Use TKey::ReadObject<T> instead of ReadObj

TKey::ReadObj returns an owning pointer.
Using dynamic_cast and ignoring the "not castable" case can
leak memory.

Also one std::unique_ptr

See: https://root-forum.cern.ch/t/possible-leak-with-dynamic-cast-and-tkey-readobj/56799
See: root-project/root#13931

Author: ChristianTackeGSI

examples/simulation/Tutorial4/macros/run_tutorial4_createParameterFile.C

@@ -1,17 +1,21 @@
 /********************************************************************************
- *    Copyright (C) 2014 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH    *
+ * Copyright (C) 2014-2023 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH  *
  *                                                                              *
  *              This software is distributed under the terms of the             *
  *              GNU Lesser General Public Licence (LGPL) version 3,             *
  *                  copied verbatim in the file "LICENSE"                       *
  ********************************************************************************/
 
+#include <TCollection.h>
+#include <TGeoManager.h>
+#include <TKey.h>
+#include <TList.h>
+
 // forward declaration
 void writeVectorToFile(std::vector<Double_t>& vec, ofstream* myfile);
 
 void run_tutorial4_createParameterFile()
 {
-
     // Create shift in x, y, z when true
     Bool_t ShiftX = kTRUE;
     Bool_t ShiftY = kTRUE;
@@ -42,14 +46,14 @@ void run_tutorial4_createParameterFile()
 
     TList* l = f->GetListOfKeys();
 
-    TKey* key;
-    TIter next(l);
-
     TGeoManager* geoMan{nullptr};
 
-    while ((key = (TKey*)next())) {
-        if (key->ReadObj()->InheritsFrom("TGeoManager")) {
-            geoMan = static_cast<TGeoManager*>(key->ReadObj());
+    for (auto key : TRangeDynCast<TKey>(l)) {
+        if (!key) {
+            continue;
+        }
+        geoMan = key->ReadObject<TGeoManager>();
+        if (geoMan) {
             break;
         }
     }

fairroot/base/sim/FairModule.cxx

@@ -45,6 +45,7 @@
 #include <cstdlib>   // for getenv
 #include <cstring>   // for strcmp, strlen
 #include <map>
+#include <memory>
 
 void FairModule::ConstructGeometry()
 {
@@ -301,9 +302,8 @@ void FairModule::ConstructRootGeometry(TGeoMatrix* shiftM)
      */
     TGeoManager* OldGeo = gGeoManager;
     TGeoManager* NewGeo = 0;
-    TGeoVolume* volume = 0;
-    ;
-    TFile* f = TFile::Open(GetGeometryFileName().Data());
+
+    std::unique_ptr<TFile> f{TFile::Open(GetGeometryFileName().Data())};
     TList* l = f->GetListOfKeys();
     TKey* key;
     TIter next(l);
@@ -313,19 +313,19 @@ void FairModule::ConstructRootGeometry(TGeoMatrix* shiftM)
         /**loop inside the delivered root file and try to fine a TGeoManager object
          * the first TGeoManager found will be read
          */
-        if (strcmp(key->GetClassName(), "TGeoManager") != 0) {
+        NewGeo = key->ReadObject<TGeoManager>();
+        if (!NewGeo) {
             continue;
         }
         gGeoManager = 0;
-        NewGeo = static_cast<TGeoManager*>(key->ReadObj());
         break;
     }
     if (NewGeo != 0) {
         /** in case a TGeoManager was found get the top most volume and the node
          */
 
         NewGeo->cd();
-        volume = static_cast<TGeoVolume*>(NewGeo->GetNode(0)->GetDaughter(0)->GetVolume());
+        TGeoVolume* volume = NewGeo->GetNode(0)->GetDaughter(0)->GetVolume();
         v1 = volume->MakeCopyVolume(volume->GetShape());
         // n=NewGeo->GetTopNode();
         n = v1->GetNode(0);
@@ -338,8 +338,8 @@ void FairModule::ConstructRootGeometry(TGeoMatrix* shiftM)
          */
 
         key = static_cast<TKey*>(l->At(0));   // Get the first key in the list
-        volume = dynamic_cast<TGeoVolume*>(key->ReadObj());
-        if (volume != 0) {
+        auto volume = key->ReadObject<TGeoVolume>();
+        if (volume) {
             n = volume->GetNode(0);
         }
         if (n != 0) {
@@ -415,7 +415,6 @@ void FairModule::ConstructRootGeometry(TGeoMatrix* shiftM)
              */
             ExpandNode(n);
             delete NewGeo;
-            delete f;
         } else {
             LOG(fatal) << "Could not find the given mother volume " << fMotherVolumeName.Data()
                        << " where the geomanger should be added.";

fairroot/base/steer/FairRunAna.cxx

@@ -28,7 +28,7 @@
 #include "FairTrajFilter.h"       // for FairTrajFilter
 #include "signal.h"
 
-#include <TCollection.h>      // for TIter
+#include <TCollection.h>      // for TRangeDynCast
 #include <TDirectory.h>       // for TDirectory::TContext
 #include <TFile.h>            // for TFile
 #include <TGeoManager.h>      // for gGeoManager, TGeoManager
@@ -144,17 +144,7 @@ void FairRunAna::Init()
 
     // Load Geometry from user file
     if (fLoadGeo) {
-        if (fInputGeoFile != 0) {   // First check if the user has a separate Geo file!
-            TIter next(fInputGeoFile->GetListOfKeys());
-            TKey* key;
-            while ((key = dynamic_cast<TKey*>(next()))) {
-                if (strcmp(key->GetClassName(), "TGeoManager") != 0) {
-                    continue;
-                }
-                gGeoManager = dynamic_cast<TGeoManager*>(key->ReadObj());
-                break;
-            }
-        }
+        SearchForTGeoManagerInGeoFile();
     } else {
         /*** Get the container that normly has the geometry and all the basic stuff from simulation*/
         fRtdb->getContainer("FairGeoParSet");
@@ -180,17 +170,7 @@ void FairRunAna::Init()
     } else {   //  if(fInputFile )
         // NO input file but there is a geometry file
         if (fLoadGeo) {
-            if (fInputGeoFile != 0) {   // First check if the user has a separate Geo file!
-                TIter next(fInputGeoFile->GetListOfKeys());
-                TKey* key;
-                while ((key = dynamic_cast<TKey*>(next()))) {
-                    if (strcmp(key->GetClassName(), "TGeoManager") != 0) {
-                        continue;
-                    }
-                    gGeoManager = dynamic_cast<TGeoManager*>(key->ReadObj());
-                    break;
-                }
-            }
+            SearchForTGeoManagerInGeoFile();
         }
     }
 
@@ -643,3 +623,21 @@ void FairRunAna::Fill()
     }
 }
 //_____________________________________________________________________________
+
+void FairRunAna::SearchForTGeoManagerInGeoFile()
+{
+    if (!fInputGeoFile) {   // First check if the user has a separate Geo file!
+        return;
+    }
+    for (auto key : TRangeDynCast<TKey>(fInputGeoFile->GetListOfKeys())) {
+        if (!key) {
+            continue;
+        }
+        auto geo = key->ReadObject<TGeoManager>();
+        if (!geo) {
+            continue;
+        }
+        gGeoManager = geo;
+        break;
+    }
+}

fairroot/base/steer/FairRunAna.h

@@ -142,6 +142,11 @@ class FairRunAna : public FairRun
      */
     void CheckRunIdChanged();
 
+    /**
+     * \brief Internal helper: Search for TGeoManager in fInputGeoFile
+     */
+    void SearchForTGeoManagerInGeoFile();
+
     ClassDefOverride(FairRunAna, 6);
 };
 

fairroot/base/steer/FairRunAnaProof.cxx

@@ -94,17 +94,7 @@ void FairRunAnaProof::Init()
     // Load Geometry from user file
 
     if (fLoadGeo) {
-        if (fInputGeoFile != 0) {   // First check if the user has a separate Geo file!
-            TIter next(fInputGeoFile->GetListOfKeys());
-            TKey* key;
-            while ((key = static_cast<TKey*>(next()))) {
-                if (strcmp(key->GetClassName(), "TGeoManager") != 0) {
-                    continue;
-                }
-                gGeoManager = static_cast<TGeoManager*>(key->ReadObj());
-                break;
-            }
-        }
+        SearchForTGeoManagerInGeoFile();
     } else {
         //    FairGeoParSet* geopar=dynamic_cast<FairGeoParSet*>(fRtdb->getContainer("FairGeoParSet"));
         fRtdb->getContainer("FairGeoParSet");
@@ -142,17 +132,7 @@ void FairRunAnaProof::Init()
     } else {   //  if(fInputFile )
         // NO input file but there is a geometry file
         if (fLoadGeo) {
-            if (fInputGeoFile != 0) {   // First check if the user has a separate Geo file!
-                TIter next(fInputGeoFile->GetListOfKeys());
-                TKey* key;
-                while ((key = static_cast<TKey*>(next()))) {
-                    if (strcmp(key->GetClassName(), "TGeoManager") != 0) {
-                        continue;
-                    }
-                    gGeoManager = static_cast<TGeoManager*>(key->ReadObj());
-                    break;
-                }
-            }
+            SearchForTGeoManagerInGeoFile();
         }
     }
     // Init the Chain ptr