diff --git a/core/cont/inc/TRefArray.h b/core/cont/inc/TRefArray.h index df0bb73c73e78..8a2e1b88762ce 100644 --- a/core/cont/inc/TRefArray.h +++ b/core/cont/inc/TRefArray.h @@ -173,8 +173,12 @@ inline TObject *TRefArray::operator[](Int_t at) const if (j >= 0 && j < fSize) { if (!fPID) return nullptr; if (!TProcessID::IsValid(fPID)) return nullptr; - TObject *obj = fPID->GetObjectWithID(fUIDs[j]); - if (!obj) obj = GetFromTable(j); + // Query the TRefTable first so that a TBranchRef (if any) can load the + // entry of the referenced branch; a stale object from a previously read + // entry may still be registered under this UID (ROOT-3594). + TObject *obj = GetFromTable(j); + if (!obj) + obj = fPID->GetObjectWithID(fUIDs[j]); return obj; } BoundsOk("At", at); @@ -188,8 +192,10 @@ inline TObject *TRefArray::At(Int_t at) const if (j >= 0 && j < fSize) { if (!fPID) return nullptr; if (!TProcessID::IsValid(fPID)) return nullptr; - TObject *obj = fPID->GetObjectWithID(fUIDs[j]); - if (!obj) obj = GetFromTable(j); + // See the comment in operator[] above (ROOT-3594). + TObject *obj = GetFromTable(j); + if (!obj) + obj = fPID->GetObjectWithID(fUIDs[j]); return obj; } BoundsOk("At", at); diff --git a/core/cont/src/TRefArray.cxx b/core/cont/src/TRefArray.cxx index 8c6c56e16d3dc..3d21c83c3edc7 100644 --- a/core/cont/src/TRefArray.cxx +++ b/core/cont/src/TRefArray.cxx @@ -405,7 +405,10 @@ TObject *TRefArray::After(const TObject *obj) const Int_t idx = IndexOf(obj) - fLowerBound; if (idx == -1 || idx == fSize-1) return nullptr; - return fPID->GetObjectWithID(fUIDs[idx+1]); + TObject *after = GetFromTable(idx + 1); + if (!after) + after = fPID->GetObjectWithID(fUIDs[idx + 1]); + return after; } //////////////////////////////////////////////////////////////////////////////// @@ -418,7 +421,10 @@ TObject *TRefArray::Before(const TObject *obj) const Int_t idx = IndexOf(obj) - fLowerBound; if (idx == -1 || idx == 0) return nullptr; - return fPID->GetObjectWithID(fUIDs[idx-1]); + TObject *before = GetFromTable(idx - 1); + if (!before) + before = fPID->GetObjectWithID(fUIDs[idx - 1]); + return before; } //////////////////////////////////////////////////////////////////////////////// @@ -568,7 +574,10 @@ void TRefArray::Streamer(TBuffer &R__b) TObject *TRefArray::First() const { - return fPID->GetObjectWithID(fUIDs[0]); + TObject *obj = GetFromTable(0); + if (!obj) + obj = fPID->GetObjectWithID(fUIDs[0]); + return obj; } //////////////////////////////////////////////////////////////////////////////// @@ -578,8 +587,11 @@ TObject *TRefArray::Last() const { if (fLast == -1) return nullptr; - else - return fPID->GetObjectWithID(fUIDs[GetAbsLast()]); + + TObject *obj = GetFromTable(GetAbsLast()); + if (!obj) + obj = fPID->GetObjectWithID(fUIDs[GetAbsLast()]); + return obj; } //////////////////////////////////////////////////////////////////////////////// diff --git a/roottest/root/treeformula/references/RefProxyTest.ref b/roottest/root/treeformula/references/RefProxyTest.ref index b63a690142994..cbaec8f20a40d 100644 --- a/roottest/root/treeformula/references/RefProxyTest.ref +++ b/roottest/root/treeformula/references/RefProxyTest.ref @@ -155,24 +155,24 @@ crap *********************************************************************************************************************************** * Row * Instance * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fTracks[f * fTracks[f * fTracks[f * fTracks[f * *********************************************************************************************************************************** -* 0 * 0 * 50331650 * 2 * 0.316 * 1.17 * 1.21 * 3 * -0.601 * 2.41 * 2.49 * -* 0 * 1 * 3 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 * -* 0 * 2 * 5 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 * -* 0 * 3 * 6 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 * -* 0 * 4 * 8 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 * -* 0 * 5 * 9 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 * -* 0 * 6 * 10 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 * -* 0 * 7 * 13 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 * -* 0 * 8 * 14 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 * -* 0 * 9 * 15 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 * -* 0 * 10 * 16 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 * -* 0 * 11 * 17 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 * -* 0 * 12 * 20 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 * -* 0 * 13 * 23 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 * -* 0 * 14 * 24 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 * -* 0 * 15 * 25 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 * -* 0 * 16 * 27 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 * -* 0 * 17 * 29 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 * +* 0 * 0 * 50331650 * 2 * 1.1 * 0.299 * 1.14 * 3 * -0.601 * 2.41 * 2.49 * +* 0 * 1 * 50331651 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 * +* 0 * 2 * 50331653 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 * +* 0 * 3 * 50331654 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 * +* 0 * 4 * 50331656 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 * +* 0 * 5 * 50331657 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 * +* 0 * 6 * 50331658 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 * +* 0 * 7 * 50331661 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 * +* 0 * 8 * 50331662 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 * +* 0 * 9 * 50331663 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 * +* 0 * 10 * 50331664 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 * +* 0 * 11 * 50331665 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 * +* 0 * 12 * 50331668 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 * +* 0 * 13 * 50331671 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 * +* 0 * 14 * 50331672 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 * +* 0 * 15 * 50331673 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 * +* 0 * 16 * 50331675 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 * +* 0 * 17 * 50331677 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 * *********************************************************************************************************************************** @@ -213,23 +213,23 @@ crap *********************************************************************************************************************************** * Row * Instance * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fTracks[f * fTracks[f * fTracks[f * fTracks[f * *********************************************************************************************************************************** -* 0 * 0 * 67108866 * 2 * 0.316 * 1.17 * 1.21 * 3 * -0.601 * 2.41 * 2.49 * -* 0 * 1 * 3 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 * -* 0 * 2 * 5 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 * -* 0 * 3 * 6 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 * -* 0 * 4 * 8 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 * -* 0 * 5 * 9 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 * -* 0 * 6 * 10 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 * -* 0 * 7 * 13 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 * -* 0 * 8 * 14 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 * -* 0 * 9 * 15 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 * -* 0 * 10 * 16 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 * -* 0 * 11 * 17 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 * -* 0 * 12 * 20 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 * -* 0 * 13 * 23 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 * -* 0 * 14 * 24 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 * -* 0 * 15 * 25 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 * -* 0 * 16 * 27 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 * -* 0 * 17 * 29 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 * +* 0 * 0 * 67108866 * 2 * 1.1 * 0.299 * 1.14 * 3 * -0.601 * 2.41 * 2.49 * +* 0 * 1 * 67108867 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 * +* 0 * 2 * 67108869 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 * +* 0 * 3 * 67108870 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 * +* 0 * 4 * 67108872 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 * +* 0 * 5 * 67108873 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 * +* 0 * 6 * 67108874 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 * +* 0 * 7 * 67108877 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 * +* 0 * 8 * 67108878 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 * +* 0 * 9 * 67108879 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 * +* 0 * 10 * 67108880 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 * +* 0 * 11 * 67108881 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 * +* 0 * 12 * 67108884 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 * +* 0 * 13 * 67108887 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 * +* 0 * 14 * 67108888 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 * +* 0 * 15 * 67108889 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 * +* 0 * 16 * 67108891 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 * +* 0 * 17 * 67108893 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 * *********************************************************************************************************************************** (bool) true diff --git a/tree/tree/test/TTreeRegressions.cxx b/tree/tree/test/TTreeRegressions.cxx index 5aba6fe4cddf8..d9e5e677d6388 100644 --- a/tree/tree/test/TTreeRegressions.cxx +++ b/tree/tree/test/TTreeRegressions.cxx @@ -1,11 +1,16 @@ -#include "TMemFile.h" -#include "TLeaf.h" -#include "TTree.h" +#include "TBranch.h" +#include "TClonesArray.h" +#include "TH1F.h" #include "TInterpreter.h" -#include "TSystem.h" +#include "TLeaf.h" #include "TLeafObject.h" -#include "TH1F.h" +#include "TMemFile.h" +#include "TNamed.h" +#include "TProcessID.h" #include "TROOT.h" +#include "TRefArray.h" +#include "TSystem.h" +#include "TTree.h" #include "gtest/gtest.h" @@ -322,3 +327,54 @@ TEST(TTreeRegressions, DrawAutoBinning) delete h; delete gROOT->FindObject("c1"); } + +// https://its.cern.ch/jira/browse/ROOT-3594 +// TRefArray::At must query the TRefTable (loading the referenced branch for +// the current entry) before looking up the TProcessID, like TRef::GetObject +// does; otherwise it returns a stale object from a previously read entry. +TEST(TTreeRegressions, TRefArrayReadsCurrentEntry) +{ + TMemFile f("tree_trefarraycurrententry.root", "recreate"); + { + TTree t("t", "t"); + t.BranchRef(); + auto *hits = new TClonesArray("TNamed"); + auto *refs = new TRefArray; + t.Branch("hits", &hits); + t.Branch("refs", &refs); + for (int ev = 0; ev < 2; ++ev) { + const auto objcount = TProcessID::GetObjectCount(); + hits->Clear(); + refs->Clear(); + for (int i = 0; i < 2; ++i) { + auto *h = new ((*hits)[i]) TNamed(TString::Format("ev%d_hit%d", ev, i).Data(), ""); + refs->Add(h); + } + t.Fill(); + TProcessID::SetObjectCount(objcount); + } + t.Write(); + delete hits; + delete refs; + } + + auto &t = *f.Get("t"); + TClonesArray *hits = nullptr; + TRefArray *refs = nullptr; + t.SetBranchAddress("hits", &hits); + t.SetBranchAddress("refs", &refs); + + // Fully load entry 0: its hits get registered in the TProcessID. + t.GetEntry(0); + ASSERT_NE(refs->At(0), nullptr); + EXPECT_STREQ(refs->At(0)->GetName(), "ev0_hit0"); + + // Emulate TTree::Draw: move to entry 1 but only read the refs branch. The + // TRefTable must load the hits branch for entry 1 when dereferencing. + t.LoadTree(1); + t.GetBranch("refs")->GetEntry(1); + ASSERT_NE(refs->At(0), nullptr); + EXPECT_STREQ(refs->At(0)->GetName(), "ev1_hit0"); + ASSERT_NE(refs->At(1), nullptr); + EXPECT_STREQ(refs->At(1)->GetName(), "ev1_hit1"); +}