Memory leak in TClonesArray! Bug?

Hello ROOT-Team,

I found a memory leak in TClonesArray (a least I think I did). When using TClonesArray::Clone() I loose lots and lots of memory. This happens quite often in my application by using copy-constructors and assignment-operators.Maybe I just did something wrong. Please have a look at my code snippets below, especially at Bar::setTClonesArray()

To print a clean picture I reduced the whole programming-stuff on two classes. “Foo” - the one I store in a TClonesArray - and “Bar”, the one that holds a TClonesArray with n elements of class “Foo”.

The problem occured with ROOT 3.04/07 with gcc3.3 (32 Bit Fedora Core 2 Linux) and ROOT 4.02/00 with gcc3.4 (64 Bit Debian Linux).

Class “Foo”, the one I store in a TClonesArray. It’s derived from TObject, a dictionary is available and it includes the ClassDef()/ClassImp()-Macros. Otherwise nothing spechial here.

// Header
#include "TObject.h"
#include "TString.h"

class Foo : public TObject {

	private:
		TString mFoo;	

	public:
		Foo();
		Foo( const Foo& pCopyMe );
		virtual ~Foo();

		Foo& operator=( const Foo& pAssignMe );

		inline const TString& getFooMember() const { return mFoo; }		
		inline void setFooMember( const TString& pFoo ) { mFoo = pFoo; }		

	ClassDef( Foo, 1 );
}

Implementation of “Foo”. Very boring, except the ClassImp-Macro may be…

include "Foo.h"

ClassImp( Foo )

Foo::Foo() : TObject() {}
Foo::Foo(const Foo& pCopyMe) : TObject(pCopyMe) {};
Foo::~Foo() {}

Foo& Foo::operator( const Foo& pAssignMe ) {
	if (this != &pAssignMe) {
		mFoo = pAssignMe.getFooMember();
	}
	return *this;
}

Class “Bar”, the one with the problematic TClonesArray as member.

#include "TClonesArray"

class Bar  {

	private:
		TClonesArray* mArray;	

	public:
		Bar();
		Bar( const Bar& pCopyMe );
		~Bar();

		Bar& operator=( const Bar& pAssignMe );

		inline const TClonesArray* getTClonesArray() { return mArray; }
		void setTClonesArray( const TClonesArray* pTClonesArray );
}

Implementation of Bar. Please look at Bar::setTClonesArray!

#include "Foo.h"

Bar::Bar() : mArray( new TClonesArray("Foo", 100) ) {}

Bar::Bar( const Bar& pCopyMe ) : mArray(0) {
	setTClonesArray( pCopyMe.getTClonesArray() );
}

Bar::~Bar() {

	if ( mArray ) {
		// Delete elements of TClonesArray.
		// After that delete TClonesArray itself.
		mArray->Delete();
		delete mArray;
	}
}


void Bar::setTClonesArray( const TClonesArray* pTClonesArray ) {

	if ( mArray ) {
		mArray->Delete();
		delete mArray;
		mArray = 0;
	}   
		

	if ( pTClonesArray ) {
		// --- This I did earlier, but leaks as hell ---
		// mArray = dynamic_cast<TClonesArray*>( pTClonesArray->Clone() ); 
		
		// Now I copy all elements one by one. Looks nasty but works and does not leak...
		mArray = new TClonesArray("Foo", pTClonesArray->GetEntries());
		for ( Int_t idx = 0; idx < pTClonesArray->GetEntries(); idx++ ) {		
			new((*mArray)[idx] ) Foo(*(dynamic_cast<Foo*>(pTClonesArray->At(idx))));	
		}		
	}
}


Bar& Bar::operator=( const Bar& pAssignMe ) {
	
	if ( this != &pAssignMe ) {
		setTClonesArray( pAssignMe.getTClonesArray() );
	}
	return *this
}

Thanks

H.-Gerd

Hi H.-Gerd,
after spending some time converting your code into valid C++ I cannot reproduce this with root 4.03/01. I attached your fixed code, as well as my test case.

Note, though, that copying containers is in general not a good idea. If you do you should at least not copy the contained elements as well - it’s really inefficient. Instead, if possible, pass pointers or references to the collections around.
Axel.
rosarius.tar.gz (1.29 KB)