[Root7] A path to reducing global memory management.

.github/workflows/root-ci.yml

@@ -167,7 +167,8 @@ jobs:
           HOME: /Users/sftnight
           INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') && !matrix.platform == 'mac15' && !matrix.platform == 'mac26'}}
           GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
-        run: ".github/workflows/root-ci-config/build_root.py 
+          ROOT_IMPLICIT_OWNERSHIP: silentOff
+        run: ".github/workflows/root-ci-config/build_root.py
                     --buildtype       RelWithDebInfo
                     --incremental     $INCREMENTAL
                     --base_ref        ${{ github.base_ref }}
@@ -526,6 +527,7 @@ jobs:
           INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') }}
           GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }}
           CMAKE_GENERATOR: ${{ matrix.cmake_generator }}
+          ROOT_IMPLICIT_OWNERSHIP: silentOff
         run: ".github/workflows/root-ci-config/build_root.py
                     --buildtype       RelWithDebInfo
                     --platform        ${{ matrix.image }}

bindings/pyroot/pythonizations/test/memory.py

@@ -1,7 +1,7 @@
 import ROOT
 import gc
 import os
 import unittest
 
 
 def _leak(obj):
@@ -16,7 +16,7 @@
     def test_tstyle_memory_management(self):
         """Regression test for https://github.com/root-project/root/issues/16918"""
 
         h1 = ROOT.TH1F("h1", "", 100, 0, 10)
 
         style = ROOT.TStyle("NewSTYLE", "")
         groot = ROOT.ROOT.GetROOT()
@@ -27,19 +27,19 @@
         """Regression test for https://github.com/root-project/root/issues/16942"""
         # The test is just that the memory regulation works correctly and the
         # application does not segfault
         f2 = ROOT.TF2("f2", "sin(x)*sin(y)/x/y")
 
     def test_tf3_memory_regulation(self):
         """Make sure TF3 is properly managed by the memory regulation logic"""
         # The test is just that the memory regulation works correctly and the
         # application does not segfault
         f3 = ROOT.TF3("f3","[0] * sin(x) + [1] * cos(y) + [2] * z",0,10,0,10,0,10)
 
     def test_tcolor_memory_regulation(self):
         """Make sure TColor is properly managed by the memory regulation logic"""
         # The test is just that the memory regulation works correctly and the
         # application does not segfault
         c = ROOT.TColor(42, 42, 42)
 
     def test_ttree_clone_in_file_context(self):
         """Test that CloneTree() doesn't give the ownership to Python when
@@ -49,8 +49,8 @@
 
         ttree = ROOT.TTree("tree", "tree")
 
         with ROOT.TFile(filename, "RECREATE") as infile:
             ttree_clone = ttree.CloneTree()
 
         os.remove(filename)
 
@@ -88,7 +88,7 @@
             "TH1I": ("h", "h", 10, 0, 10),
             "TH1L": ("h", "h", 10, 0, 10),
             "TH1F": ("h", "h", 10, 0, 10),
             "TH1D": ("h", "h", 10, 0, 10),
             "TProfile": ("h", "h", 10, 0, 10),
             "TH2C": ("h", "h", 10, 0, 10, 10, 0, 10),
             "TH2S": ("h", "h", 10, 0, 10, 10, 0, 10),
@@ -127,11 +127,14 @@
             "_check_object_setdirectory_in_memory_file_begin", "recreate")
 
         x = klass(*args)
-        # TEfficiency does not automatically register with the directory
-        if not classname == "TEfficiency":
-            self.assertIs(x.GetDirectory(), f1)
-            x.SetDirectory(ROOT.nullptr)
+        # Actively register the object in case implicit ownership is off:
+        if not x.GetDirectory():
+            x.SetDirectory(f1)
+
+        self.assertIs(x.GetDirectory(), f1)
+        x.SetDirectory(ROOT.nullptr)
         self.assertFalse(x.GetDirectory())
+
         # Make sure that at this point the ownership of the object is with Python
         ROOT.SetOwnership(x, True)
 

bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py

@@ -25,11 +25,13 @@ def setUpClass(cls):
         dir1.cd()
         h1 = ROOT.TH1F("h1", "h1", cls.nbins, cls.xmin, cls.xmax)
         ROOT.SetOwnership(h1, False)
+        h1.SetDirectory(dir1)
 
         dir2 = dir1.mkdir("dir2")
         dir2.cd()
         h2 = ROOT.TH1F("h2", "h2", cls.nbins, cls.xmin, cls.xmax)
         ROOT.SetOwnership(h2, False)
+        h2.SetDirectory(dir2)
 
     def checkHisto(self, h):
         xaxis = h.GetXaxis()

bindings/pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py

@@ -25,11 +25,13 @@ def setUpClass(cls):
         dir1.cd()
         h1 = ROOT.TH1F("h1", "h1", cls.nbins, cls.xmin, cls.xmax)
         ROOT.SetOwnership(h1, False)
+        h1.SetDirectory(dir1)
 
         dir2 = dir1.mkdir("dir2")
         dir2.cd()
         h2 = ROOT.TH1F("h2", "h2", cls.nbins, cls.xmin, cls.xmax)
         ROOT.SetOwnership(h2, False)
+        h2.SetDirectory(dir2)
 
     def checkHisto(self, h):
         xaxis = h.GetXaxis()

bindings/pyroot/pythonizations/test/tfile_context_manager.py

@@ -1,9 +1,9 @@
 import os
 import unittest
 
 import ROOT
 
 from ROOT import TFile
 
 
 class TFileContextManager(unittest.TestCase):
@@ -64,6 +64,7 @@
         histoname = "myhisto_3"
         with TFile(filename, "recreate") as outfile:
             hout = ROOT.TH1F(histoname, histoname, self.NBINS, self.XMIN, self.XMAX)
+            hout.SetDirectory(outfile)
             outfile.Write()
 
         self.check_file_data(outfile, filename, histoname)

core/base/inc/TROOT.h

@@ -97,6 +97,11 @@ namespace ROOT {
    void DisableImplicitMT();
    Bool_t IsImplicitMTEnabled();
    UInt_t GetThreadPoolSize();
+   namespace Experimental {
+      void EnableImplicitObjectOwnership();
+      void DisableImplicitObjectOwnerhsip();
+      bool IsImplicitObjectOwnershipEnabled();
+   }
 }
 
 class TROOT : public TDirectory {

core/base/src/TDirectory.cxx

@@ -178,18 +178,22 @@ TDirectory::TContext::~TContext()
 /// ~~~ {.cpp}
 ///     TDirectory::AddDirectory
 /// ~~~
+/// \note When ROOT::Experimental::IsImplicitObjectOwnershipEnabled() is off, these settings
+/// are without effect.
 
 void TDirectory::AddDirectory(Bool_t add)
 {
    fgAddDirectory = add;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Static function: see TDirectory::AddDirectory for more comments.
-
+/// Check whether objects such as histograms or TGraphs2D should be owned by the current directory.
+/// \copydetails AddDirectory(Bool_t)
+/// \note ROOT::Experimental::IsImplicitObjectOwnershipEnabled() might lead to this
+/// setting being always off, since it has higher precedence.
 Bool_t TDirectory::AddDirectoryStatus()
 {
-   return fgAddDirectory;
+   return ROOT::Experimental::IsImplicitObjectOwnershipEnabled() && fgAddDirectory;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -205,6 +209,7 @@ void TDirectory::Append(TObject *obj, Bool_t replace /* = kFALSE */)
    if (replace && obj->GetName() && obj->GetName()[0]) {
       TObject *old;
       while (nullptr != (old = GetList()->FindObject(obj->GetName()))) {
+         if (old == obj) return;
          Warning("Append","Replacing existing %s: %s (Potential memory leak).",
                  obj->IsA()->GetName(),obj->GetName());
          ROOT::DirAutoAdd_t func = old->IsA()->GetDirectoryAutoAdd();

core/base/src/TEnv.cxx

@@ -79,9 +79,7 @@ into the local file by default.
 #include "THashList.h"
 #include "TError.h"
 
-
-TEnv *gEnv;  // main environment created in TROOT
-
+TEnv *gEnv = nullptr; // main environment created in TROOT
 
 static struct BoolNameTable_t {
    const char *fName;

core/base/src/TROOT.cxx

@@ -71,6 +71,7 @@ of a main program creating an interactive version is shown below:
 #include <ROOT/RVersion.hxx>
 #include "RConfigure.h"
 #include "RConfigOptions.h"
+#include <atomic>
 #include <filesystem>
 #include <string>
 #include <map>
@@ -471,6 +472,49 @@ namespace Internal {
       return isImplicitMTEnabled;
    }
 
+   ////////////////////////////////////////////////////////////////////////////////
+   /// \brief Test if objects such as TTree and TH1-derived classes should be implicitly owned
+   /// by gDirectory.
+   /// A default can be set in a .rootrc using "Root.ImplicitOwnership: 1" or setting
+   /// the environment variable "ROOT_IMPLICIT_OWNERSHIP=0".
+   static std::atomic_bool &IsImplicitOwnershipEnabled()
+   {
+      static std::atomic_bool initCompleted = false;
+      static std::atomic_bool implicitOwnership = true;
+
+      if (!initCompleted.load(std::memory_order_acquire)) {
+         R__LOCKGUARD(gROOTMutex);
+         // test again, because another thread might have raced us here
+         if (!initCompleted) {
+            int desiredValue = -1;
+            bool silent = false;
+            if (auto env = gSystem->Getenv("ROOT_IMPLICIT_OWNERSHIP"); env) {
+               if (strcmp(env, "silentOff") == 0) {
+                  implicitOwnership = false;
+                  silent = true;
+               } else {
+                  try {
+                     desiredValue = std::stoi(env);
+                  } catch (std::invalid_argument& e) { }
+                  if (desiredValue < 0) Error("TROOT", "ROOT_IMPLICIT_OWNERSHIP should be >= 0");
+               }
+            } else if (gEnv) {
+               desiredValue = gEnv->GetValue("Root.ImplicitOwnership", -1);
+            }
+
+            if (desiredValue == 0) {
+               if (!silent) Info("TROOT", "Implicit object ownership switched off by ROOT_IMPLICIT_OWNERSHIP or rootrc");
+               implicitOwnership = false;
+            } else if (desiredValue > 0) {
+               implicitOwnership = true;
+            }
+
+            initCompleted = true;
+         }
+      }
+
+      return implicitOwnership;
+   }
 } // end of Internal sub namespace
 // back to ROOT namespace
 
@@ -616,6 +660,64 @@ namespace Internal {
       return 0;
 #endif
    }
+
+   namespace Experimental {
+   ////////////////////////////////////////////////////////////////////////////////
+   /// \brief Switch ROOT's object ownership model to ROOT 6 mode.
+   ///
+   /// In ROOT 6 mode, ROOT will implicitly assign ownership of histograms or TTrees
+   /// to the current \ref gDirectory, for example to the last TFile that was opened.
+   /// \code{.cpp}
+   /// TFile file(...);
+   /// TTree* tree = new TTree(...);
+   /// TH1D* histo = new TH1D(...);
+   /// file.Write(); // Both tree and histogram are in the file now
+   /// \endcode
+   ///
+   /// In ROOT 7 mode, these objects won't register themselves to the current gDirectory,
+   /// so they are fully owned by the user. To write these to files, the user needs to do
+   /// one of the following:
+   /// - Explicitly transfer ownership:
+   /// \code{.cpp}
+   /// TFile file(...);
+   /// TTree* tree = new TTree(...);
+   /// tree->SetDirectory(&file);
+   /// \endcode
+   /// - Keep ownership of the object, but write explicitly:
+   /// \code{.cpp}
+   /// TFile file(...);
+   /// std::unique_ptr<TH1D> histo{new TH1D(...)};
+   /// file.WriteObject(histo.get(), "HistogramName");
+   /// file.Close();
+   /// // histo is still valid
+   /// \endcode
+   ///
+   /// \note This setting has higher priority than TH1::AddDirectoryStatus() and TDirectory::AddDirectoryStatus().
+   /// These two will always evaluate to false if implicit ownership is off.
+   ///
+   /// \copydetails ROOT::Internal::IsImplicitOwnershipEnabled()
+   void EnableImplicitObjectOwnerhsip()
+   {
+      Internal::IsImplicitOwnershipEnabled() = true;
+   }
+
+   ////////////////////////////////////////////////////////////////////////////////
+   /// \brief Switch ROOT's object ownership model to ROOT 7 mode (no ownership).
+   /// \copydetails ROOT::Experimental::EnableImplicitObjectOwnership()
+   void DisableImplicitObjectOwnerhsip()
+   {
+      Internal::IsImplicitOwnershipEnabled() = false;
+   }
+
+   ////////////////////////////////////////////////////////////////////////////////
+   /// Test whether the current directory should take ownership of objects such as
+   /// TH1-derived classes, TTree, TEntryList etc.
+   /// \copydetails ROOT::Experimental::EnableImplicitObjectOwnership()
+   bool IsImplicitObjectOwnershipEnabled()
+   {
+      return Internal::IsImplicitOwnershipEnabled();
+   }
+}
 } // end of ROOT namespace
 
 TROOT *ROOT::Internal::gROOTLocal = ROOT::GetROOT();
@@ -2828,7 +2930,7 @@ void TROOT::SetBatch(Bool_t batch)
 ///    interactive mode.
 ///  - "server:port": turns the web display into server mode with specified port. Web widgets will not be displayed,
 ///    only text message with window URL will be printed on standard output
-///  
+///
 /// \note See more details related to webdisplay on RWebWindowsManager::ShowWindow
 
 void TROOT::SetWebDisplay(const char *webdisplay)

hist/hist/src/TEfficiency.cxx

@@ -1503,15 +1503,16 @@ Double_t TEfficiency::BetaMode(Double_t a,Double_t b)
 /// Notes:
 /// - calls: SetName(name), SetTitle(title)
 /// - set the statistic option to the default (kFCP)
-/// - appends this object to the current directory SetDirectory(gDirectory)
+/// - appends this object to the current directory SetDirectory(gDirectory) if
+///   TH1::AddDirectoryStatus() is active.
 
 void TEfficiency::Build(const char* name,const char* title)
 {
    SetName(name);
    SetTitle(title);
 
    SetStatisticOption(kDefStatOpt);
-   SetDirectory(gDirectory);
+   if (TH1::AddDirectoryStatus()) SetDirectory(gDirectory);
 
    SetBit(kPosteriorMode,false);
    SetBit(kShortestInterval,false);

hist/hist/src/TH1.cxx

@@ -736,11 +736,13 @@ TH1::TH1(const char *name,const char *title,Int_t nbins,const Double_t *xbins)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Static function: cannot be inlined on Windows/NT.
+/// Check whether TH1-derived classes are owned by the current gDirectory.
+/// \note ROOT::Experimental::IsImplicitObjectOwnershipEnabled() might lead to this
+/// setting being always off, since it has higher precedence.
 
 Bool_t TH1::AddDirectoryStatus()
 {
-   return fgAddDirectory;
+   return ROOT::Experimental::IsImplicitObjectOwnershipEnabled() && fgAddDirectory;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1249,16 +1251,19 @@ Bool_t TH1::Add(const TH1 *h1, const TH1 *h2, Double_t c1, Double_t c2)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Sets the flag controlling the automatic add of histograms in memory
+/// Sets the flag controlling the automatic add of histograms in memory.
 ///
 /// By default (fAddDirectory = kTRUE), histograms are automatically added
-/// to the list of objects in memory.
+/// to the current directory (gDirectory).
 /// Note that one histogram can be removed from its support directory
 /// by calling h->SetDirectory(nullptr) or h->SetDirectory(dir) to add it
 /// to the list of objects in the directory dir.
 ///
-/// NOTE that this is a static function. To call it, use;
-/// TH1::AddDirectory
+/// This is a static function. To call it, use `TH1::AddDirectory`
+///
+/// \note When ROOT::Experimental::IsImplicitOwnershipEnabled() is off, AddDirectory is
+/// without effect.
+///
 
 void TH1::AddDirectory(Bool_t add)
 {
@@ -2720,7 +2725,7 @@ void TH1::Copy(TObject &obj) const
    // will be added to gDirectory independently of the fDirectory stored.
    // and if the AddDirectoryStatus() is false it will not be added to
    // any directory (fDirectory = nullptr)
-   if (fgAddDirectory && gDirectory) {
+   if (AddDirectoryStatus() && gDirectory) {
       gDirectory->Append(&obj);
       ((TH1&)obj).fFunctions->UseRWLock();
       ((TH1&)obj).fDirectory = gDirectory;
@@ -2776,16 +2781,13 @@ TObject* TH1::Clone(const char* newname) const
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Perform the automatic addition of the histogram to the given directory
+/// Callback to perform the automatic addition of the histogram to the given directory.
 ///
-/// Note this function is called in place when the semantic requires
-/// this object to be added to a directory (I.e. when being read from
-/// a TKey or being Cloned)
+/// This callback is called when a TKey is read or an object is being Cloned.
 
 void TH1::DirectoryAutoAdd(TDirectory *dir)
 {
-   Bool_t addStatus = TH1::AddDirectoryStatus();
-   if (addStatus) {
+   if (fgAddDirectory) {
       SetDirectory(dir);
       if (dir) {
          ResetBit(kCanDelete);

hist/hist/src/TH2.cxx

@@ -854,6 +854,7 @@ void TH2::DoFitSlices(bool onX,
       } else {
          hlist[ipar] = new TH1D(name,title, nOutBins, &bins->fArray[firstOutBin-1]);
       }
+      hlist[ipar]->SetDirectory(gDirectory);
       hlist[ipar]->GetXaxis()->SetTitle(outerAxis.GetTitle());
       if (arr)
          (*arr)[ipar] = hlist[ipar];
@@ -866,6 +867,7 @@ void TH2::DoFitSlices(bool onX,
    } else {
       hchi2 = new TH1D(name,"chisquare", nOutBins, &bins->fArray[firstOutBin-1]);
    }
+   hchi2->SetDirectory(gDirectory);
    hchi2->GetXaxis()->SetTitle(outerAxis.GetTitle());
    if (arr)
       (*arr)[npar] = hchi2;
@@ -2126,7 +2128,8 @@ TProfile *TH2::ProfileY(const char *name, Int_t firstxbin, Int_t lastxbin, Optio
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Internal (protected) method for performing projection on the X or Y axis
-/// called by ProjectionX or ProjectionY
+/// called by ProjectionX or ProjectionY.
+/// The histograms created are added to gDirectory.
 
 TH1D *TH2::DoProjection(bool onX, const char *name, Int_t firstbin, Int_t lastbin, Option_t *option) const
 {
@@ -2239,6 +2242,7 @@ TH1D *TH2::DoProjection(bool onX, const char *name, Int_t firstbin, Int_t lastbi
          else
             h1 = new TH1D(pname,GetTitle(),lastOutBin-firstOutBin+1,&bins->fArray[firstOutBin-1]);
       }
+      h1->SetDirectory(gDirectory);
       if (opt.Contains("e") || GetSumw2N() ) h1->Sumw2();
    }
    if (pname != name)  delete [] pname;

hist/hist/src/THStack.cxx

@@ -370,6 +370,7 @@ void THStack::Add(TH1 *h1, Option_t *option)
    }
    if (!fHists) fHists = new TList();
    fHists->Add(h1,option);
+   h1->SetBit(kMustCleanup); // The histogram is likely in multiple lists now
    Modified(); //invalidate stack
 }