Adding class with array of structs as member

bpollack · September 17, 2013, 3:21pm

Hi ROOT,

I am implementing a class that inherits from TLorentzVector in my analyzer, but running into some memory management difficulties. The class includes an array of structs as one of its public members, and that array is new’d and initialized in the constructor like so:

Header snippet:

[code]class TCPhoton : public TCPhysObject {
public:
struct CrystalInfo{
int rawId;
int ieta;
int iphi;
int ix;
int iy;
double energy;
double time;
double timeErr;
int recoFlag;
};

TCPhoton();                        
virtual ~TCPhoton();

…

private:
CrystalInfo* _crysArray;

…

public:
void SetCrystal(int, CrystalInfo);
[/code]

.cc snippet:

[code]TCPhoton::TCPhoton() {
_crysArray = new CrystalInfo[100];

CrystalInfo crystal;

crystal.rawId = -99;
crystal.ieta= -99;
crystal.iphi= -99;
crystal.ix= -99;
crystal.iy= -99;
crystal.energy= -99;
crystal.time= -99;
crystal.timeErr= -99;
crystal.recoFlag= -99;

for(int i=0; i < 100; i++){
TCPhoton:: SetCrystal(i, crystal);
}

}

TCPhoton::~TCPhoton() { delete[] _crysArray; }

…

void TCPhoton::SetCrystal(int i , CrystalInfo crys) {
_crysArray[i].rawId = crys.rawId;
_crysArray[i].ieta =crys.ieta;
_crysArray[i].iphi =crys.iphi;
_crysArray[i].ix =crys.ix;
_crysArray[i].iy =crys.iy;
_crysArray[i].energy =crys.energy;
_crysArray[i].time =crys.time;
_crysArray[i].timeErr =crys.timeErr;
_crysArray[i].recoFlag =crys.recoFlag;
}

[/code]

I was running valgrind to find the source of a double-free error on root’s exit, and this ‘mismatch free / delete / delete[]’ error cropped up:

==13164== Mismatched free() / delete / delete []
==13164==    at 0x4807F82: operator delete(void*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/external/valgrind/3.6.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13164==    by 0xB2DCBFF: ROOT::delete_TCPhotoncLcLCrystalInfo(void*) (TCPhoton_cc_ACLiC_dict.cxx:269)
==13164==    by 0x4A49FD8: TClass::Destructor(void*, bool) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x7FA7566: TBufferFile::ReadFastArray(void**, TClass const*, int, bool, TMemberStreamer*, TClass const*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x8066DD7: int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, int, int, int, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x7FF3FD6: TStreamerInfoActions::GenericVectorPtrReadAction(TBuffer&, void*, void const*, TStreamerInfoActions::TConfiguration const*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x7FA6387: TBufferFile::ApplySequenceVecPtr(TStreamerInfoActions::TActionSequence const&, void*, void*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x4A25864: TClonesArray::Streamer(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0xBBA14F7: TLeafObject::ReadBasket(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB66F1C: TBranch::ReadLeavesImpl(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB6D6FE: TBranch::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBBB136B: TTree::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==  Address 0xdc95460 is 0 bytes inside a block of size 5,600 alloc'd
==13164==    at 0x48085DB: operator new[](unsigned long) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/external/valgrind/3.6.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13164==    by 0xB2DB633: TCPhoton::TCPhoton() (TCPhoton.cc:8)
==13164==    by 0xB2DC63E: ROOT::new_TCPhoton(void*) (TCPhoton_cc_ACLiC_dict.cxx:217)
==13164==    by 0x4A4989C: TClass::New(TClass::ENewType) const (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x4A25833: TClonesArray::Streamer(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0xBBA14F7: TLeafObject::ReadBasket(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB66F1C: TBranch::ReadLeavesImpl(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB6D6FE: TBranch::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBBB136B: TTree::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xE209985: simple::Process(long long) (in /uscms_data/d2/bpollack/CMSSW_5_3_8_patch1/src/HZG_Analyzer/HiggsZGAnalyzer/simple_C.so)
==13164==    by 0xE0C66D5: TTreePlayer::Process(TSelector*, char const*, long long, long long) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTreePlayer.so)
==13164==    by 0xE0CD90F: TTreePlayer::Process(char const*, char const*, long long, long long) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTreePlayer.so)
==13164== 
==13164== Mismatched free() / delete / delete []
==13164==    at 0x4807BFE: operator delete[](void*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/external/valgrind/3.6.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13164==    by 0xB2DB764: TCPhoton::~TCPhoton() (TCPhoton.cc:30)
==13164==    by 0xB2DB7A3: TCPhoton::~TCPhoton() (TCPhoton.cc:30)
==13164==    by 0xB2DC848: ROOT::delete_TCPhoton(void*) (TCPhoton_cc_ACLiC_dict.cxx:224)
==13164==    by 0x4A49FD8: TClass::Destructor(void*, bool) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x4A24E4D: TClonesArray::~TClonesArray() (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x4A24EF8: TClonesArray::~TClonesArray() (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x4A49FD8: TClass::Destructor(void*, bool) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0xBBA1616: TLeafObject::ReadBasket(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB66F1C: TBranch::ReadLeavesImpl(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB6D6FE: TBranch::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBBB136B: TTree::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==  Address 0x6aea6d0 is 0 bytes inside a block of size 56 alloc'd
==13164==    at 0x4808AC5: operator new(unsigned long) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/external/valgrind/3.6.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13164==    by 0xB2DCB7B: ROOT::new_TCPhotoncLcLCrystalInfo(void*) (TCPhoton_cc_ACLiC_dict.cxx:262)
==13164==    by 0x4A4989C: TClass::New(TClass::ENewType) const (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0x7FA9A9F: TBufferFile::ReadObjectAny(TClass const*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x7FA7542: TBufferFile::ReadFastArray(void**, TClass const*, int, bool, TMemberStreamer*, TClass const*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x8066DD7: int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, int, int, int, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x7FF3FD6: TStreamerInfoActions::GenericVectorPtrReadAction(TBuffer&, void*, void const*, TStreamerInfoActions::TConfiguration const*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x7FA6387: TBufferFile::ApplySequenceVecPtr(TStreamerInfoActions::TActionSequence const&, void*, void*) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libRIO.so)
==13164==    by 0x4A25864: TClonesArray::Streamer(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libCore.so)
==13164==    by 0xBBA14F7: TLeafObject::ReadBasket(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB66F1C: TBranch::ReadLeavesImpl(TBuffer&) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==    by 0xBB6D6FE: TBranch::GetEntry(long long, int) (in /uscmst1/prod/sw/cms/slc5_amd64_gcc462/lcg/root/5.32.00-cms22/lib/libTree.so)
==13164==

This seems to imply that the array struct that I have is not being deleted properly, but I don’t see any obvious issues. Do any of the root experts have any advice? I’ve included the full class defs and associated files if anyone wants to take a look.

Thanks,
Brian
TCClasses.tgz (2.58 KB)

honk · September 18, 2013, 10:25am

Hi,

just guessing since you didn’t post a macro to actually run your code.

You haven’t provided a copy constructor for TCPhoton so the compiler will generate a default one for you. That copy constructor won’t make a deep copy of pointer members, so a copy-constructed TCPhoton will share the same _crysArray. Usually, if a class contains pointer members and you e.g. define a deconstructor you also want to provide all of: custom copy constructor, assignment operator, and deconstructor (this is know as the “Rule of Three” in C++98). This is because the compiler-generated functions will copy only the values of member variables, i.e. they will copy pointer values, not values pointed to.

As an (I feel important) aside, if your class doesn’t have pointer members, the compiler-generated copy constructor, assignment operator, and deconstructor are most often fine. In your case there is no reason for _crysArray to be dynamically allocated, a member std::vector crysArray would be just fine. With that e.g. the default copy constructor will automatically perform a deep copy, and you wouldn’t need a custom destructor. TCPhoton::GetCrystalArray() can keep its interface if you return &_crysArray[0].

bpollack · September 19, 2013, 1:03pm

Hi Honk,

I took your suggestion and implemented a custom copy constructor and copy assignment operator. Unfortunately I was still getting a Mismatch free/delete/delete[] warning from valgrind, so I eventually took your second suggestion and simply swapped out my struct pointer member with a vector of structs. Now my class no longer throws any valgrind warnings.

Thanks a lot,
Brian