GetEntry() crash in second using

[vseredov]

Dear rooters,

I discovered the crash of the TTree::GetEntry() method in following conditionis:

1. ROOT 5.28.00

2. there is root-file, created and wroted using TFile

3. file contains Tree, which constructed using TTree and using object of My_Class in following manner:
   …
   My_Class* object_of_MyClass = new My_Class();
   …
   TTree* My_Tree = new TTree(“my_tree_name”,“meme”);
   My_Tree->Branch(“my_branch_name”, &object_of_MyClass, 64000, 1);
   for(…)
   {
   …
   My_Tree->Fill();
   My_Tree->Write();
   …
   }
   … etc.

4. Myclass contains fields
   class MyClass : public TObject
   {
   …
   private:
   int a;
   TH2D* My_Histogram;
   …
   ClassDef(…);
   }
   Thus, My_Tree contains branch of type TH2D* My_Histogram;

5. there is the function(), which reads this file and this tree as following:
   int my_reader_class::function()
   {
   My_Class* object_of_MyClass = new My_Class(); // empty constructor
   …
   TFile* pointer_to_file = new TFile(File_Name, “READ”);
   …
   TTree* My_Tree = (TTree*)pointer_to_file->Get(“my_tree_name”);
   TBranch* bntrack1 = My_Tree->GetBranch(“my_branch_name”);
   bntrack1->SetAddress(&object_of_MyClass);
   My_Tree->GetEntry(33);
   …
   object_of_MyClass->Clear(); // TObject::Clear()
   delete object_of_MyClass; // or TObject::Delete()
   delete My_Tree; // or TTree::Delete("")
   pointer_to_file->DeleteAll(); // TDirectory::DeleteAll()
   pointer_to_file->Close(“R”);
   delete pointer_to_file;

     return 0;
   

}

6. there is file main.C, containing function main(), which running function() twice:
   #include …
   int main()
   {
   my_reader_class* MRC = new my_reader_class();

     MRC->function();                                                                            // one
   
     MRC->function();                                                                            // two
   
     delete MRC;
   
     return 0;
   

}

7. I use ROOT as the set of external libraries, what assumes compilation in following form:

i) rootcint -f my_reader_class_Dictionary.cxx -c my_reader_class.h

ii) g++ -shared -fPIC root-config --cflags -o libmy_reader_class.so my_reader_class.C my_reader_class_Dictionary.cxx

iii) g++ -g -o0 -pthread -m64 -I/(path) -L/(path) -Wl,-rpath,/(path) -I/(ROOT includes) -L/(ROOT libraries) -lCore -lCint -lRIO -lNet -lHist -lGraf -lGraf3d -lGpad -lTree -lRint -lPostscript -lMatrix -lPhysics -lMathCore -lThread -lGui -lTreePlayer -pthread -lm -ldl -rdynamic -lmy_reader_class main.C -o program_name

As the result, I can read entry number 33 first time, at the line which commented “//one”, even I can draw it to check that everything right. Everything is rigth. Entry number 33 contains exactly the histogram, which I have put to there before.

However, second time, at the line which commented “//two” I have the crash.
Crash at the start of GetEntry(33):

*** Break *** segmentation violation

===========================================================
There was a crash.
This is the entire stack trace of all threads:

#0 0x0000003b09098c05 in waitpid () from /lib64/libc.so.6
#1 0x0000003b0903c481 in do_system () from /lib64/libc.so.6
#2 0x00002ba55beddc6a in TUnixSystem::StackTrace() ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libCore.so
#3 0x00002ba55bedd63c in TUnixSystem::DispatchSignals(ESignals) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libCore.so
#4
#5 0x00002ba55d8bc1f8 in ROOT::delete_TH1D(void*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libHist.so
#6 0x00002ba55bea7d57 in TClass::Destructor(void*, bool) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libCore.so
#7 0x00002ba55cff2e3f in TBufferFile::ReadFastArray(void**, TClass const*, int, bool, TMemberStreamer*, TClass const*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#8 0x00002ba55d09837c in int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, int, int, int, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#9 0x00002ba55d028e6e in TStreamerInfoActions::GenericAction(TBuffer&, void*, TStreamerInfoActions::TConfiguration const*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#10 0x00002ba55cfed44d in TBufferFile::ReadSequence(TStreamerInfoActions::TActionSequence const&, void*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#11 0x00002ba55e7a4c19 in TBranchElement::ReadLeavesMember(TBuffer&) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#12 0x00002ba55e79b215 in TBranch::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#13 0x00002ba55e7a2253 in TBranchElement::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#14 0x00002ba55e7a2216 in TBranchElement::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#15 0x00002ba55e7e73a0 in TTree::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#16 0x00002ba562c36912 in builder_class::reconstruction_reader() ()
from /grid_mnt/vol__vol1__u/llr/galop/seredov/Magnetic_Line/TREE_BUILDER/tree_builder_version_g++/libbuilder_class.so
#17 0x000000000040198d in main () at main.C:57

The lines below might hint at the cause of the crash.
If they do not help you then please submit a bug report at
root.cern.ch/bugs. Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.

#5 0x00002ba55d8bc1f8 in ROOT::delete_TH1D(void*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libHist.so
#6 0x00002ba55bea7d57 in TClass::Destructor(void*, bool) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libCore.so
#7 0x00002ba55cff2e3f in TBufferFile::ReadFastArray(void**, TClass const*, int, bool, TMemberStreamer*, TClass const*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#8 0x00002ba55d09837c in int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, int, int, int, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#9 0x00002ba55d028e6e in TStreamerInfoActions::GenericAction(TBuffer&, void*, TStreamerInfoActions::TConfiguration const*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#10 0x00002ba55cfed44d in TBufferFile::ReadSequence(TStreamerInfoActions::TActionSequence const&, void*) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libRIO.so
#11 0x00002ba55e7a4c19 in TBranchElement::ReadLeavesMember(TBuffer&) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#12 0x00002ba55e79b215 in TBranch::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#13 0x00002ba55e7a2253 in TBranchElement::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#14 0x00002ba55e7a2216 in TBranchElement::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#15 0x00002ba55e7e73a0 in TTree::GetEntry(long long, int) ()
from /opt/exp_soft/llr/lcg/app/releases/ROOT/5.28.00/x86_64-slc5-gcc43-opt/root/lib/libTree.so
#16 0x00002ba562c36912 in builder_class::reconstruction_reader() ()
from /grid_mnt/vol__vol1__u/llr/galop/seredov/Magnetic_Line/TREE_BUILDER/tree_builder_version_g++/libbuilder_class.so
#17 0x000000000040198d in main () at main.C:57

What I am doing wrong?

Thanks in advance.

[pcanal]

[quote]My_Class* object_of_MyClass = new My_Class(); // empty constructor[/quote]How empty is the constructor. I must at least initialize the value of My_Histogram (to zero).

Cheers,
Philippe.

[Pepe_Le_Pew]

Philippe, maybe you could have a look at the sequence of “Clear” / “[dD]elete*” directives in the end of the “my_reader_class::function” (point 5. in the first post above).

[pcanal]

Hi Pepe,

I think it is unrelated to the crash but indeed the routine should be written[code]My_Class* object_of_MyClass = 0; // TTree will call the default constructor a needed.
…
TFile* pointer_to_file = new TFile(File_Name, “READ”);
…
TTree* My_Tree;
pointer_to_file->GetObject(“my_tree_name”, MyTree); // typesafe version.
if (MyTree == 0) {
delete pointer_to_file;
return;
}
MyTree->SetBranchAddress(“my_branch_name”,&object_of_MyClass); // More generic interface.
…

My_Tree->GetEntry(33);
…
// Only needed if you were going to call GetEntry again on the same TTree object.
object_of_MyClass->Clear(); // TObject::Clear()

delete pointer_to_file; // Will Close the file and delete the TFile, the TTree and the user object created by the TTree.[/code]

And now that I have re-read your code, you are seemingly creating the TFile and the TTree for each call to GetEntry (probably an artifact of simplifying your code for this post but is very inefficient), then the most probable cause is definitively the missing initialization in the default constructor.

Cheers,
Philippe.

[Pepe_Le_Pew]

What about “delete object_of_MyClass;” (before/after “delete pointer_to_file;”)?
http://root.cern.ch/root/html/TBranchElement.html#TBranchElement:SetAddress
(The above page seems to suggest that the user is always responsible for deleting, unless “SetAddress(0)” was executed, even if the branch object was NOT explicitly allocated by the user.)

[pcanal]

Hi Pepe,

You are correct; Whenever doing:My_Class* object_of_MyClass = ...; .... MyTree->SetBranchAddress("my_branch_name",&object_of_MyClass); // More generic interface. .... delete object_of_MyClass;. It must be followed by a object_of_MyClass = 0;

Cheers,
Philippe.

[Pepe_Le_Pew]

Sorry, I was not precise enough in my question.

Consider the following cases:

1.a delete My_Tree; delete pointer_to_file;
1.b delete pointer_to_file; // no explicit “delete My_Tree;”, will automatically delete “My_Tree” though

-> will deleting of “My_Tree” delete “object_of_MyClass” automatically as well (in both these cases, assuming that “object_of_MyClass != 0”) or will it not?
-> also, if “object_of_MyClass” was automatically deleted, should I expect that it will also automatically set “object_of_MyClass = 0” or should I not?

2.a delete object_of_MyClass; delete My_Tree; delete pointer_to_file;
2.b delete object_of_MyClass; delete pointer_to_file; // no redundant explicit “delete My_Tree;” call

-> will one get a segmentation violation in these cases because, when deleting the “My_Tree’” (explicitly or implicitly), it will automatically try to delete “object_of_MyClass” for the second time (which has already been deleted explicitly, but “object_of_MyClass != 0” as it has not been “reset”)?

3.a delete object_of_MyClass; object_of_MyClass = 0; delete My_Tree; delete pointer_to_file;
3.b delete object_of_MyClass; object_of_MyClass = 0; delete pointer_to_file; // redundant call removed

-> no segmentation violation in these cases because when the “My_Tree” will be deleted it will notice that “object_of_MyClass = 0” and it will not try to delete it again

4.a delete My_Tree; delete pointer_to_file; delete object_of_MyClass;
4.b delete pointer_to_file; delete object_of_MyClass; // “My_Tree” automatically deleted

-> again, should I expect a segmentation violation in these cases because the explicit “delete object_of_MyClass;” call will try to delete an object which has already been deleted when deleting “My_Tree” (explicitly or implicitly)?

(Note, It seems to me that, in any of the above cases, I don’t need to care about setting “My_Tree = 0;” after I explicitly “delete My_Tree;” and before I “delete pointer_to_file;”.)

[pcanal]

[quote]1.a delete My_Tree; delete pointer_to_file;
1.b delete pointer_to_file; // no explicit “delete My_Tree;”, will automatically delete “My_Tree” though

-> will deleting “My_Tree” delete “object_of_MyClass” automatically as well (in both these cases, assuming that “object_of_MyClass != 0”) or will it not?[/quote]Both case will behave the same.
Since the address a pointer object_of_MyClass was passed to the branch, the object will not be deleted … and in this case is leaked.

[quote]2.a delete object_of_MyClass; delete My_Tree; delete pointer_to_file;
2.b delete object_of_MyClass; delete pointer_to_file; // no redundant explicit “delete My_Tree;” call

-> will one get a segmentation violation in these cases because, when deleting the “My_Tree’” (explicitly or implicitly), it will automatically try to delete “object_of_MyClass” for the second time (which has already been deleted explicitly)?[/quote]Both case will behave the same. And yes the behavior is ‘undefined’. Even-though the TTree will not be deleting the object (because the address was explicitly set), it might access the (deleted) object anyway.

[quote]3.a delete object_of_MyClass; object_of_MyClass = 0; delete My_Tree; delete pointer_to_file;
3.b delete object_of_MyClass; object_of_MyClass = 0; delete pointer_to_file; // redundant call removed

-> no segmentation violation in these cases because when the “My_Tree” will be deleted it will notice that “object_of_MyClass = 0” and it will not try to delete it again[/quote]Yes.

[quote]4.a delete My_Tree; delete pointer_to_file; delete object_of_MyClass;
4.b delete pointer_to_file; delete object_of_MyClass; // “My_Tree” automatically deleted

-> again, should I expect a segmentation violation in these cases because the explicit “delete object_of_MyClass;” call will try to delete an object which has already been deleted when deleting “My_Tree” (explicitly or implicitly)?[/quote]Both case behaves the same. There is no double delete (since the setting of the branch address passed the ownership to the calling code).

[quote](Note, It seems to me that, in any of the above cases, I don’t need to care about setting “My_Tree = 0;” after I explicitly “delete My_Tree;” and before I “delete pointer_to_file;”.)[/quote]That entirely depend on the lifetime and use (in your case) of My_Tree.

See root.cern.ch/root/html/TTree.html#TTree:GetEntry for more details.

Cheers,
Philippe.

[Pepe_Le_Pew]

[quote] 1.a delete My_Tree; delete pointer_to_file;
1.b delete pointer_to_file; // no explicit “delete My_Tree;”, will automatically delete “My_Tree” though

-> will deleting "My_Tree" delete "object_of_MyClass" automatically as well (in both these cases, assuming that "object_of_MyClass != 0") or will it not?

Both case will behave the same. The TTree will delete the user object only if it allocated it (so if object_of_MyClass was zero when GetEntry was called).
[/quote]

[quote] 4.a delete My_Tree; delete pointer_to_file; delete object_of_MyClass;
4.b delete pointer_to_file; delete object_of_MyClass; // “My_Tree” automatically deleted

-> again, should I expect a segmentation violation in these cases because the explicit "delete object_of_MyClass;" call will try to delete an object which has already been deleted when deleting "My_Tree" (explicitly or implicitly)?

Both case behaves the same. There is a double delete if the pointer was null when GetEntry was called.
[/quote]

Well, the whole bunch of my questions was triggered by:
http://root.cern.ch/root/html/TBranchElement.html#TBranchElement:SetAddress
in which, regardless whether there is “Event* event = new Event();” or “Event* event = 0;”, there is always an explicit call “delete event; event = 0;”.
Moreover … it says … [quote]If addr is not zero, but the pointer addr points at is
zero, then we allocate a branch object and set the passed
pointer to point at the allocated object. The caller
owns the allocated object and is responsible for deleting
it when it is no longer needed.[/quote] That seems to suggest that I am always responsible for deleting “object_of_MyClass”, regardless whether it was zero or non-zero when GetEntry was called.
(P.S. The “TTree::GetEntry” description gives no hints about the ownership of the objects it uses -> i.e. nothing is said about what to do with that objects once they are no longer needed.)

[pcanal]

Hi,

[quote]That seems to suggest that I am always responsible for deleting “object_of_MyClass”, regardless whether it was zero or non-zero when GetEntry was called.[/quote]You are completely right, I keep remembering the old semantic .

If you pass the address of a pointer to the branch, you own the object (whether you allocate it or not) and must delete it explicitly. I have updated the previous answer to correct it.

Cheers,
Philippe.

[Pepe_Le_Pew]

Great.
So, there are three safe ways to deal with this problem.

1. One deletes “object_of_MyClass” AFTER the tree is deleted … one of …
   1.a delete My_Tree; delete object_of_MyClass; delete pointer_to_file;
   1.b delete My_Tree; delete pointer_to_file; delete object_of_MyClass;
   1.c delete pointer_to_file; delete object_of_MyClass;

2. One deletes “object_of_MyClass” BEFORE the tree is deleted (note: setting “object_of_MyClass = 0;”, BEFORE the tree is going to be deleted, either explicitly or implicitly, is MANDATORY) … one of …
   2.a delete object_of_MyClass; object_of_MyClass = 0; delete My_Tree; delete pointer_to_file;
   2.b delete object_of_MyClass; object_of_MyClass = 0; delete pointer_to_file;

3. One first calls “bntrack1->SetAddress(0);” or “My_Tree->ResetBranchAddress(bntrack1);” or “My_Tree->ResetBranchAddresses();” and then the “object_of_MyClass” is no longer “associated” with the tree in any way, so one can delete it in any moment (either before or after “delete My_Tree;” and/or “delete pointer_to_file;”, and there is no need to set “object_of_MyClass = 0;”):
   http://root.cern.ch/root/html/TTree.html#TTree:ResetBranchAddress
   http://root.cern.ch/root/html/TTree.html#TTree:ResetBranchAddresses
   (Well, the descriptions of both of the above methods seem to be a bit short and there’s just that slightly worrying “Note: If any of our branches own any objects, they are deleted.”)

Note that, in any case, the “delete pointer_to_file;” call will automatically delete “My_Tree”, if it was not deleted in advance (so the “delete My_Tree;” call is usually redundant).

BTW. Maybe someone could add some notes about the ownership of objects in the “TTree” class description , in “==> Case B” at least, and in the “TTree::GetEntry” description.

[pcanal]

Hi Pepe,

Yes, you are correct.

Just a small note in C++:if (object_of_MyClass) delete object_of_MyClass;the if part is redundant and you can safely execute:delete object_of_MyClass;whether the pointer is null or not.

Cheers,
Philippe.

[Pepe_Le_Pew]

I am allowed to try to delete a zero pointer
Why was I always trying to protect against it
Well, I edited my post and removed the redundant “if”. <img src="/uploads/default/original/2X/8/84c2fe9464a4066c00e1bd5978e913e7869cbb07.gif" width=“22” height=“16” alt=":-"" title=“Whistle”/>

[pcanal]

[quote]Well, I’d rather leave “if (object_of_MyClass)”, just as a precaution (e.g. imagine a branch with an “object_of_MyClass” that was initialized with zero at the beginning but then the “GetEntry” was never called for that particular branch).[/quote]Yes, and it should work fine also in this case (really p = 0; delete p; is fine).

Cheers,
Philippe.

[jade2]

[quote=“Pepe Le Pew”]Great.
So, there are three safe ways to deal with this problem.

1. One deletes “object_of_MyClass” AFTER the tree is deleted … one of …
   1.a delete My_Tree; delete object_of_MyClass; delete pointer_to_file;
   1.b delete My_Tree; delete pointer_to_file; delete object_of_MyClass;
   1.c delete pointer_to_file; delete object_of_MyClass;

2. One deletes “object_of_MyClass” BEFORE the tree is deleted (note: setting “object_of_MyClass = 0;”, BEFORE the tree is going to be deleted, either explicitly or implicitly, is MANDATORY) … one of …
   2.a delete object_of_MyClass; object_of_MyClass = 0; delete My_Tree; delete pointer_to_file;
   2.b delete object_of_MyClass; object_of_MyClass = 0; delete pointer_to_file;

3. One first calls “bntrack1->SetAddress(0);” or “My_Tree->ResetBranchAddress(bntrack1);” or “My_Tree->ResetBranchAddresses();” and then the “object_of_MyClass” is no longer “associated” with the tree in any way, so one can delete it in any moment (either before or after “delete My_Tree;” and/or “delete pointer_to_file;”, and there is no need to set “object_of_MyClass = 0;”):
   http://root.cern.ch/root/html/TTree.html#TTree:ResetBranchAddress
   http://root.cern.ch/root/html/TTree.html#TTree:ResetBranchAddresses
   (Well, the descriptions of both of the above methods seem to be a bit short and there’s just that slightly worrying “Note: If any of our branches own any objects, they are deleted.”)
   [/quote]

Hi Phillipe,
Now this order of deletion is not only of the retrieving of data from an existing root file, but as well as when one is collecting data and writing the events to the root file, right? Is it possible that if one doesn’t follow this order of deletion in the end that the contents of what was collected could be corrupted (as you noted that the behavior is undefined (e.g. doesn’t necessarily crash))? I can retrieve the contents of my data fine but I’m noticing some garbage data in my root file and can’t figure out the root cause…

I didn’t quite follow this order suggested and deleted my event object before removing the TTree and TFile (in my case the event object was on the heap while the TTree & TFile was on the stack; so in the destructor I had cleaned up my event object, then the TTree & TFile probably followed after that since that was on the stack). Easier control of the sequence of deletion would be better if I put all three on the heap and explicitly delete it in the destructor the order suggested…

[pcanal]

[quote]Is it possible that if one doesn’t follow this order of deletion in the end that the contents of what was collected could be corrupted (as you noted that the behavior is undefined (e.g. doesn’t necessarily crash))? I can retrieve the contents of my data fine but I’m noticing some garbage data in my root file and can’t figure out the root cause…[/quote]It might, but it should not (in the simplest case of mis-ordering). What and where is the ‘garbage data’ you are concerned about?

[quote]I didn’t quite follow this order suggested and deleted my event object before removing the TTree and TFile (in my case the event object was on the heap while the TTree & TFile was on the stack;[/quote]In that case you can just set the pointer to zero and if the pointer goes out of scope before the TFile and TTree can be deleted, make sure to call tree->ResetBranchAddresses() to make the TTree forget about the pointer.

Cheers,
Philippe.