Memory allocation in default constructor

Hi ROOT gurus,

This is really a followup to a 2001 post,
root.cern.ch/root/roottalk/roottalk01/1771.html
but I couldn’t figure out how to post a reply to something that old.

The issue is how to gracefully allocate memory for a TClonesArray data member.

If I imitate the Event example and say

TSimple::TSimple()
{
 if (!fgTracks) 
  fgTracks = new TClonesArray("TVectorD",10);
 fTracks = fgTracks;
}

then when I try to read a TSimple object from a ROOT file, I get error messages like this:

goCINT(18042) malloc: ***  Deallocation of a pointer not malloced: 0x41ec248; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug

I can avoid this if

  1. I don’t put anything in the TClonesArray. Not too useful.
  2. I comment out the “if” line in the default constructor. But unconditional allocation is exactly what I’m supposed not to do, right??

I could post a full example with < 50 loc if it would help.

Thanks,
Steve

Why do you need TClonesArray member allocated in a dynamic memory? You can simply have
TClonesArray as a member, not pointer to TClonesArray. This is good for several reasons:

  1. exception safety
  2. you do not need to think about copy constructor/copy assignment - compiler generated version will be OK :

class A
{
public:
A();
private:
TClonesArray a_;
};

Why do you need some static member?

I’ll look into the possibility of a non-pointer member. But, based on the Event example and the 2001 post I referred to, I figure that there is a way of having a TClonesArray* member that is subject neither to runtime read errors nor to memory leaks—so I’d like to understand what that way is.

Did you initialize the value of fgTracks? If you did not then fgTracks might have a random value …

Philippe.

[quote=“pcanal”]Did you initialize the value of fgTracks? If you did not then fgTracks might have a random value …

Philippe.[/quote]

Hi.

I think, his fgTrack is a static member, so, it cannot have a random value :slight_smile:

[quote]I think, his fgTrack is a static member, so, it cannot have a random value[/quote]It should not but who knows which compiler and os he is using.
Anyway you are correct that it should be set to zero but the compiler linker but the observe behavior (in the first order) indicates that the value of fgTracks is corrupted (or double deleted).

Since fTracks is allocated in the constructor, you also need to make sure that it is marked with the -> comment (to tell the I/O NOT to delete it):TClonesArray *fTracks; //-> .

Next step is to try with valgrind (valgrind.kde.org).

Also I am not sure where the error message comes from. Maybe the constructor for the TVectorD has not been run?

Philippe.

Hi guys,

Thanks for your responses.
Phillipe wrote:

Well, I have

TClonesArray* TSimple::fgTracks = 0;

in TSimple.cpp, which I should think would do it.

I am using Mac OS X 10.4, ROOT 4.04, g++ as my compiler, and MACOSX_DEPLOYMENT_TARGET=10.4 c++ as my linker.

I checked out using a private non-pointer TClonesArray as suggested by tpochep, but I don’t see how to make it work properly. If I use

TClonesArray a_;

as suggested, then how do I come along later and tell a_ what data type I want it to hold?

[quote]as suggested, then how do I come along later and tell a_ what data type I want it to hold?[/quote]You should ‘say it’ in your constructor.ie.
add something like

[quote]in TSimple.cpp, which I should think would do it.
[/quote]Yes it would. So the problem is somewhere else :frowning:

Philippe

[quote=“ssgubser”]Hi guys,

Thanks for your responses.
Phillipe wrote:

Well, I have

TClonesArray* TSimple::fgTracks = 0;

in TSimple.cpp, which I should think would do it.

I am using Mac OS X 10.4, ROOT 4.04, g++ as my compiler, and MACOSX_DEPLOYMENT_TARGET=10.4 c++ as my linker.
[/quote]

Hello.

IMHO The best thing you can do - please, give us the minimal code which you think “should work” but which “does not work” : YourClass.h and YourClass.cxx.

Hi guys,

OK, here’s the code: TSimple.h and TSimple.cpp plus small auxiliary files. The Makefile is almost platform independent, but you’ll have to edit it to specify your own compiler / linker explicitly. (Sorry about that—I’m a bit of a newbie as you can no doubt tell.)

To see the problem, just say

make
goCINT
root[0] .x test12.C
...
 *** Break *** bus error

To “fix” it, just comment out line 17 of TSimple.cpp and uncomment line 18 (thereby allocating memory for the TClonesArray* in the default constructor!). But won’t I then get a memory leak?

For comparison, I also include a TObjArray* member, which causes no problems. So there’s something special about a TClonesArray* member.

I am successfully avoiding the problem in my main code simply by avoiding TClonesArray* members altogether—so thanks for that tip! The moment I add a TClonesArray member to an otherwise working class, it breaks it unless I allocate it in the default constructor.
ForRoot.tar (10 KB)

Hi,

The code you sent fails because you are using the old I/O ROOT I/O (version 2). This version of the I/O assumes that the TClonesArray are allocated in the constructor and defined with //->. If you do that your test work.

However the first thing to do is to use the newer version of the ROOT I/O.
For this just use the following command:

to generate the dictionary. Or more flexible, write a linkdef.h file containing:

Also you ‘forgot’ to implement a destructor which in the case of the code you provided should look like:

TSimple::~TSimple() { delete fTOAptr; delete fTCAptr; }

Cheers,
Philippe.

Hi Philippe,

Indeed, adding the + after TSimple.h did the trick. Thanks also for reminding me to deallocate memory.