Memory leak: how to properly delete an array of TH1 objects?

I’ve encoutered a memory leak in my latest analysis ROOT script file.
Using “gObjectTable” I found that TH1 objects are never deleted during the analysis.
Since I am constructing all TH1 objects in a simple array, I’m guessing this arrays are not deleted.

The basic structure I’m using in a very large loop is the following:

LARGE LOOP{
  TFile* histogramFile = TFile::Open("histogramFileName");    //open input root file

  //declare and initialize histogram array
  TH1D* histogramList[2][70][10];
  for(int tIT=0; tIT <2; tIT+=1){
    for(int dIT=0; dIT <70; dIT+=1){
      for(int oIT=0; oIT <10; oIT+=1){
        histogramList[tIT][dIT][oIT] = (TH1D*)histogramFile->Get("histogramName");
      }
    }
  }
  // ... do something with the histograms ...
 histogramFile->Close();
 delete histogramList
} //END OF LARGE LOOP

This works quite fine until all the memory+swap is occupied.
How do I delete the complete array properly and free all the occupied memory?

Hi,

this is a C++ issue. The variable histogramList is a TH1D****: the statement delete histogramList will not free the memory associated to all the elements of the array.
You have to loop over the datastructure and delete the single histograms.

Danilo

Thanks for your reply.
I thought so too, however, I can’t find the correct syntax.
These two options are crashing:

  for(int tIT=0; 2 <nTimes; tIT+=1){
    for(int dIT=0; dIT <70; dIT+=1){
      for(int oIT=0; oIT <10; oIT+=1){
        delete histogramList[tIT][dIT][oIT]; //OPTION NO 1
        histogramList[tIT][dIT][oIT]->Delete(); //OPTION NO 2
      }
    }
  }

What would be the correct command?

From the source code snippet that you posted, it looks like all histograms are owned by the “histogramFile”. You MUST NOT delete these histograms as they are automatically deleted as soon as you close the “histogramFile”.
Note also that, you MUST NOT “delete histogramList;” as you did NOT call “new” for it.
So, instead of: histogramList[tIT][dIT][oIT] = (TH1D*)histogramFile->Get("histogramName"); use: histogramFile->GetObject("SomeHistogramName", histogramList[tIT][dIT][oIT]); if (!(histogramList[tIT][dIT][oIT])) { ... TH1D "SomeHistogramName" NOT found in "histogramFile" ... } and in the end, instead of: histogramFile->Close(); delete histogramList; use: delete histogramFile; // automatically deletes all objects owned by this file
BTW. If you have a pointer to a histogram which you own then you can simply “delete SomeHistoPointer;” when you no longer need it.

Hi,

the previous post underlines a very important aspect, i.e. the automatic memory management offered by ROOT.
Nevertheless, an observation is to be done:

 it looks like all histograms are owned by the "histogramFile". You MUST NOT delete these histograms as they are automatically deleted as soon as you close the "histogramFile".
Note also that, you MUST NOT "delete histogramList;" as you did NOT call "new" for it.

These sentences are not totally accurate since they do not take into account the fact that the destructor of TH1 takes into account the automatic de-registration. So it is fine to delete the pointers to histograms acquired from rootfiles explicitly.

Cheers,
Danilo

Danilo, your advice is extremely dangerous.
It will only work if the histogram (or any other object) is deleted by the user BEFORE the ROOT file that owns it is closed / deleted.

As soon as a ROOT file is closed / deleted, all pointers to all objects that it owned are “invalid” (and in general they will NOT be set to 0 / NULL by ROOT), so the user MUST NOT try to delete them afterwards.

Coyote, good observation.
It could happen that the file is closed before the deletion. Of course in this case, the TH1::AddDirectory(kFALSE); is to be used.

Cheers,
Danilo

That’s a completely different story.
In this case you globally change the owner of all NEWLY READ (RETRIEVED) histograms to “nobody” / “nothing”. They will no longer be owned by the ROOT file from which they originate (and they are NOT registered by “gROOT”, so you cannot “FindObject” them afterwards).
Note that you MUST call “TH1::AddDirectory(kFALSE);” BEFORE you read (retrieve) your histograms.

One can change the ownership of any single histogram, after it has been read (retrieved) from a ROOT file, by calling:
SomeHistoPointer->SetDirectory(0); // “nobody” / “nothing” will own it
or:
SomeHistoPointer->SetDirectory(gROOT); // “gROOT” will own it