diff --git a/core/cont/inc/TClonesArray.h b/core/cont/inc/TClonesArray.h index 6a0396645ef5e..17e3da0cc5746 100644 --- a/core/cont/inc/TClonesArray.h +++ b/core/cont/inc/TClonesArray.h @@ -46,6 +46,7 @@ class TClonesArray : public TObjArray { TClonesArray& operator=(const TClonesArray& tc); void Compress() override; void Clear(Option_t *option="") override; + void ClearSlot(Int_t index, Option_t *opt = ""); void Delete(Option_t *option="") override; void Expand(Int_t newSize) override; virtual void ExpandCreate(Int_t n); diff --git a/core/cont/src/TClonesArray.cxx b/core/cont/src/TClonesArray.cxx index 7077b1f21fcb4..fe3227f362f58 100644 --- a/core/cont/src/TClonesArray.cxx +++ b/core/cont/src/TClonesArray.cxx @@ -131,6 +131,7 @@ When investigating misuse of TClonesArray, please make sure of the following: #include "TClass.h" #include "TObject.h" #include "TObjectTable.h" +#include "TProcessID.h" #include "snprintf.h" #include @@ -431,9 +432,13 @@ void TClonesArray::Clear(Option_t *option) TObject *obj = UncheckedAt(i); if (obj) { obj->Clear(cplus); - obj->ResetBit( kHasUUID ); - obj->ResetBit( kIsReferenced ); - obj->SetUniqueID( 0 ); + if (obj->TestBit(TObject::kIsReferenced)) + if (auto pid = TProcessID::GetProcessWithUID(obj)) { + pid->RecursiveRemove(obj); + obj->ResetBit(TObject::kIsReferenced); + } + obj->ResetBit(kHasUUID); + obj->SetUniqueID(0); } } } @@ -444,10 +449,24 @@ void TClonesArray::Clear(Option_t *option) TObjArray::Clear(); } +//////////////////////////////////////////////////////////////////////////////// +/// Clear a slot in the array with option opt, later nullify it and call Compress +/// \param index the index of the array to clear, it must be in range of length of fCont, range-check is not performed +/// \param opt the option passed to the Clear function +/// \note Consider calling Compress() after this operation +void TClonesArray::ClearSlot(Int_t index, Option_t *opt) +{ + if (fCont[index]) { + fCont[index]->Clear(opt); + fCont[index] = nullptr; + Changed(); + } +} + //////////////////////////////////////////////////////////////////////////////// /// Clear the clones array. Use this routine when your objects allocate /// memory (e.g. objects inheriting from TNamed or containing TStrings -/// allocate memory). If not you better use Clear() since if is faster. +/// allocate memory). If not you better use Clear() since it is faster. void TClonesArray::Delete(Option_t *) { diff --git a/tree/tree/test/CMakeLists.txt b/tree/tree/test/CMakeLists.txt index 5442f241ff3c4..19dc0b880c86f 100644 --- a/tree/tree/test/CMakeLists.txt +++ b/tree/tree/test/CMakeLists.txt @@ -16,6 +16,7 @@ if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) endif() target_include_directories(testTOffsetGeneration PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) ROOT_STANDARD_LIBRARY_PACKAGE(SillyStruct NO_INSTALL_HEADERS HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/SillyStruct.h SOURCES SillyStruct.cxx LINKDEF SillyStructLinkDef.h DEPENDENCIES RIO) +ROOT_STANDARD_LIBRARY_PACKAGE(TestRefObj NO_INSTALL_HEADERS HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/TestRefObj.h SOURCES TestRefObj.cxx LINKDEF TestRefObjLinkDef.h DEPENDENCIES RIO) ROOT_ADD_GTEST(testBulkApi BulkApi.cxx LIBRARIES RIO Tree TreePlayer) #FIXME: tests are having timeout on 32bit CERN VM (in docker container everything is fine), # to be reverted after investigation. @@ -26,6 +27,7 @@ if(NOT CMAKE_SIZEOF_VOID_P EQUAL 4) endif() ROOT_ADD_GTEST(testTBasket TBasket.cxx LIBRARIES RIO Tree) ROOT_ADD_GTEST(testTBranch TBranch.cxx LIBRARIES RIO Tree MathCore) +ROOT_ADD_GTEST(testClonesArray testClonesArray.cxx LIBRARIES RIO Tree TestRefObj) ROOT_ADD_GTEST(testTIOFeatures TIOFeatures.cxx LIBRARIES RIO Tree) ROOT_ADD_GTEST(testTTreeCluster TTreeClusterTest.cxx LIBRARIES RIO Tree MathCore) ROOT_ADD_GTEST(testTChainParsing TChainParsing.cxx LIBRARIES RIO Tree) diff --git a/tree/tree/test/ElementStruct.h b/tree/tree/test/ElementStruct.h index 77d6f33e2247e..a6f9fa3341f79 100644 --- a/tree/tree/test/ElementStruct.h +++ b/tree/tree/test/ElementStruct.h @@ -1,3 +1,5 @@ +#ifndef ELEMENTSTRUCT_H +#define ELEMENTSTRUCT_H /** * The ElementStruct has no purpose except to provide * inputs to the test cases. @@ -8,3 +10,5 @@ class ElementStruct { int i; double *d; //[i] }; + +#endif diff --git a/tree/tree/test/SillyStruct.cxx b/tree/tree/test/SillyStruct.cxx index 1534ebbb29ff5..64c8de61312db 100644 --- a/tree/tree/test/SillyStruct.cxx +++ b/tree/tree/test/SillyStruct.cxx @@ -1,3 +1,3 @@ #include "SillyStruct.h" -ClassImp(SillyStruct) +// ClassImp(SillyStruct) diff --git a/tree/tree/test/SillyStruct.h b/tree/tree/test/SillyStruct.h index a68b95ac82301..e09a435d93140 100644 --- a/tree/tree/test/SillyStruct.h +++ b/tree/tree/test/SillyStruct.h @@ -1,3 +1,6 @@ +#ifndef SILLYSTRUCT_H +#define SILLYSTRUCT_H + #include "Rtypes.h" #include "TObject.h" @@ -15,3 +18,5 @@ class SillyStruct : public TObject { ClassDefOverride(SillyStruct, 1) }; + +#endif diff --git a/tree/tree/test/TestRefObj.cxx b/tree/tree/test/TestRefObj.cxx new file mode 100644 index 0000000000000..2e1f89269db9b --- /dev/null +++ b/tree/tree/test/TestRefObj.cxx @@ -0,0 +1 @@ +#include "TestRefObj.h" diff --git a/tree/tree/test/TestRefObj.h b/tree/tree/test/TestRefObj.h new file mode 100644 index 0000000000000..be89f53228e79 --- /dev/null +++ b/tree/tree/test/TestRefObj.h @@ -0,0 +1,22 @@ +#ifndef TESTREFOBJ_H +#define TESTREFOBJ_H + +#include "TRef.h" +#include "TRefArray.h" + +// https://its.cern.ch/jira/browse/ROOT-7249 +class TestRefObj : public TObject { +protected: + TRefArray lChildren; + +public: + TestRefObj() : lChildren() {} + virtual ~TestRefObj() {} + virtual void Clear(Option_t *opt = "C") { lChildren.Clear(opt); } + virtual void SetChild(TestRefObj *aChild) { lChildren.Add(aChild); } + virtual const TestRefObj *GetChild(Int_t idx = 0) const { return static_cast(lChildren.At(idx)); } + virtual Bool_t HasChild() const { return (lChildren.GetEntriesFast() > 0); } + ClassDef(TestRefObj, 1); +}; + +#endif diff --git a/tree/tree/test/TestRefObjLinkDef.h b/tree/tree/test/TestRefObjLinkDef.h new file mode 100644 index 0000000000000..d373149ada3b9 --- /dev/null +++ b/tree/tree/test/TestRefObjLinkDef.h @@ -0,0 +1,9 @@ +#ifdef __CLING__ + +#pragma link off all globals; +#pragma link off all classes; +#pragma link off all functions; + +#pragma link C++ class TestRefObj+; + +#endif diff --git a/tree/tree/test/testClonesArray.cxx b/tree/tree/test/testClonesArray.cxx new file mode 100644 index 0000000000000..158cd144681b4 --- /dev/null +++ b/tree/tree/test/testClonesArray.cxx @@ -0,0 +1,120 @@ +#include "TObject.h" +#include "TestRefObj.h" +#include "TFile.h" +#include "TTree.h" +#include "TRandom.h" +#include "TClonesArray.h" +#include "TProcessID.h" +#include "TSystem.h" + +#include "gtest/gtest.h" + +// https://its.cern.ch/jira/browse/ROOT-7249 +TEST(TClonesArray, RefArrayClearChildren) +{ + Bool_t resetObjectCount = kTRUE; + Bool_t activateBranchRef = kFALSE; + Int_t splitLevel = 1; + Bool_t pruneSecondChildren = kTRUE; + auto filename = "test7249.root"; + auto treename = "tree"; + // streamwithrefs + { + TFile testFile(filename, "RECREATE"); + TTree dataTree(treename, treename); + + TClonesArray particles(TestRefObj::Class(), 100); + TClonesArray children(TestRefObj::Class(), 100); + + dataTree.Branch("particles", &particles, 32768, splitLevel); + dataTree.Branch("children", &children, 32768, splitLevel); + if (activateBranchRef) { + dataTree.BranchRef(); + } + + for (Int_t e = 0; e < 1000; e++) { + // For each "event". + UInt_t objCount = TProcessID::GetObjectCount(); + + TestRefObj *motherPart = static_cast(particles.ConstructedAt(0)); + TestRefObj *childPart = static_cast(children.ConstructedAt(0)); + motherPart->SetChild(childPart); + + // Prune all children if requested and event odd. + if (pruneSecondChildren && (e % 2 != 0)) { + children.Clear("C"); + } + + dataTree.Fill(); + + children.Clear("C"); + particles.Clear("C"); + + if (resetObjectCount) { + TProcessID::SetObjectCount(objCount); + } + } + dataTree.Write(); + testFile.Close(); + } + // readwithrefs + { + TFile testFile(filename, "READ"); + TTree *dataTree = testFile.Get(treename); + + TClonesArray *particles = nullptr; + TClonesArray *children = nullptr; + + dataTree->SetBranchAddress("particles", &particles); + dataTree->SetBranchAddress("children", &children); + + if (activateBranchRef) { + dataTree->BranchRef(); + } + const Long64_t entries = dataTree->GetEntriesFast(); + for (Long64_t e = 0; e < entries; e++) { + dataTree->GetEntry(e); + + // For each "event". + UInt_t objCount = TProcessID::GetObjectCount(); + + // UInt_t parts = particles->GetEntries(); + UInt_t childs = children->GetEntries(); + + auto parti = static_cast(particles->UncheckedAt(0)); + if (pruneSecondChildren && (e % 2 != 0)) { + ASSERT_EQ(childs, 0); + ASSERT_FALSE(parti->HasChild()); + } else { + ASSERT_NE(childs, 0); + ASSERT_TRUE(parti->HasChild()); + } + + children->Clear("C"); + particles->Clear("C"); + + if (resetObjectCount) { + TProcessID::SetObjectCount(objCount); + } + } + + delete dataTree; + testFile.Close(); + } + gSystem->Unlink(filename); +} + +// https://its.cern.ch/jira/browse/ROOT-7473 +TEST(TClonesArray, ClearSlot) +{ + TClonesArray particles(TestRefObj::Class(), 100); + TClonesArray children(TestRefObj::Class(), 100); + TestRefObj *motherPart = static_cast(particles.ConstructedAt(0)); + TestRefObj *childPart = static_cast(children.ConstructedAt(0)); + motherPart->SetChild(childPart); + particles.ClearSlot(0, "C"); + auto parti = static_cast(particles.UncheckedAt(0)); + ASSERT_EQ(parti, nullptr); + auto child = static_cast(children.UncheckedAt(0)); + ASSERT_NE(child, nullptr); +}