Skip to content

Commit 67284a6

Browse files
committed
[tree] Get pointer to the correct file in TTreeReader::Restart
In the method we need to check if the current TTree has a TTreeCache associated with the current file. The previous logic was getting the pointer to the current file but calling LoadTree afterwards, which deleted the current file and switched it for a new instance. In turn, this would cause bad reads as highlighted by a run of the associated unit test with the address sanitizer: ``` ==18501==ERROR: AddressSanitizer: heap-use-after-free on address 0x61800004be60 at pc 0x000108735e2c bp 0x00016b501950 sp 0x00016b501948 READ of size 8 at 0x61800004be60 thread T0 #0 0x000108735e28 in TFile::GetCacheRead(TObject const*) const TFile.cxx:1262 #1 0x00010a6e0758 in TTree::GetReadCache(TFile*) const TTree.cxx:6403 #2 0x00010a6e79c0 in TTree::GetReadCache(TFile*, bool) TTree.cxx:6416 #3 0x00010acd2f58 in TTreeReader::Restart() TTreeReader.cxx:574 #4 0x00010acd2634 in TTreeReader::SetEntriesRange(long long, long long) TTreeReader.cxx:550 #5 0x0001048fd23c in main test_ttreereader_uninitialized_chain.cpp:28 #6 0x000190a96b94 in start+0x17b8 (dyld:arm64e+0xfffffffffff3ab94) 0x61800004be60 is located 480 bytes inside of 824-byte region [0x61800004bc80,0x61800004bfb8) freed by thread T0 here: #0 0x00010c757b0c in _ZdlPv+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4bb0c) #1 0x000107556f6c in TStorage::ObjectDealloc(void*) TStorage.cxx:325 #2 0x0001074a8a68 in TObject::operator delete(void*) TObject.cxx:1189 #3 0x000108732650 in TFile::~TFile() TFile.cxx:569 #4 0x00010a5c6e04 in TChain::LoadTree(long long) TChain.cxx:1449 #5 0x00010acd2dfc in TTreeReader::Restart() TTreeReader.cxx:571 #6 0x00010acd2634 in TTreeReader::SetEntriesRange(long long, long long) TTreeReader.cxx:550 #7 0x0001048fd23c in main test_ttreereader_uninitialized_chain.cpp:28 #8 0x000190a96b94 in start+0x17b8 (dyld:arm64e+0xfffffffffff3ab94) previously allocated by thread T0 here: #0 0x00010c7576e4 in _Znwm+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4b6e4) #1 0x000107556eec in TStorage::ObjectAlloc(unsigned long) TStorage.cxx:293 #2 0x0001056ee26c in TObject::operator new(unsigned long) TObject.h:187 #3 0x00010875b3c8 in TFile::Open(char const*, char const*, char const*, int, int) TFile.cxx:3956 #4 0x00010a5c71a0 in TChain::LoadTree(long long) TChain.cxx:1481 #5 0x00010a5c0f04 in TChain::GetEntries() const TChain.cxx:910 #6 0x0001048fd224 in main test_ttreereader_uninitialized_chain.cpp:28 #7 0x000190a96b94 in start+0x17b8 (dyld:arm64e+0xfffffffffff3ab94) ``` Change the logic to instead load the tree beforehand, so the pointer to the file will be the correct one.
1 parent d2cbd74 commit 67284a6

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

tree/treeplayer/src/TTreeReader.cxx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,10 +566,12 @@ void TTreeReader::Restart()
566566
fDirector->SetReadEntry(-1);
567567
fProxiesSet = false; // we might get more value readers, meaning new proxies.
568568
fEntry = -1;
569+
// Ensure we have a TTree at this point so that if it's connected to a file
570+
// we will get the pointer to the correct file in the following call
571+
if (!fTree->GetTree())
572+
fTree->LoadTree(0);
573+
assert(fTree->GetTree());
569574
if (const auto curFile = fTree->GetCurrentFile()) {
570-
if (!fTree->GetTree()) {
571-
fTree->LoadTree(0);
572-
}
573575
if (fTree->GetTree()) {
574576
if (auto tc = fTree->GetTree()->GetReadCache(curFile, true)) {
575577
tc->DropBranch("*", true);

0 commit comments

Comments
 (0)