Skip to content

Commit cbebe24

Browse files
ferdymercurydpiparo
authored andcommitted
[io] Bypass TTree::ChangeFile when using TFileMerger
Fixes root-project#6640
1 parent d5d7921 commit cbebe24

File tree

4 files changed

+75
-5
lines changed

4 files changed

+75
-5
lines changed

io/io/inc/TFile.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,11 @@ class TFile : public TDirectoryFile {
273273
kWriteError = BIT(14),
274274
kBinaryFile = BIT(15),
275275
kRedirected = BIT(16),
276-
kReproducible = BIT(17)
276+
kReproducible = BIT(17),
277+
// Set this bit to block an unwanted automatic call to TTree::ChangeFile
278+
// e.g. during TFileMerger operations (bypassing any set MaxTreeSize)
279+
// This is a transient bit and has no relevance for I/O.
280+
kCannotChange = BIT(18)
277281
};
278282
enum ERelativeTo { kBeg = 0, kCur = 1, kEnd = 2 };
279283
enum { kStartBigFile = 2000000000 };

io/io/src/TFileMerger.cxx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,7 @@ Bool_t TFileMerger::PartialMerge(Int_t in_type)
978978
}
979979

980980
fOutputFile->SetBit(kMustCleanup);
981+
fOutputFile->SetBit(TFile::kCannotChange);
981982

982983
TDirectory::TContext ctxt;
983984

@@ -1039,6 +1040,7 @@ Bool_t TFileMerger::PartialMerge(Int_t in_type)
10391040
Clear();
10401041
} else {
10411042
fOutputFile->ResetBit(kMustCleanup);
1043+
fOutputFile->ResetBit(TFile::kCannotChange);
10421044
SafeDelete(fOutputFile);
10431045
}
10441046
return result;

io/io/test/TFileMergerTests.cxx

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,65 @@ TEST(TFileMerger, MergeBranches)
160160
EXPECT_EQ(a0tree.FindBranch("a")->GetEntries(), 2);
161161
file3->Write();
162162
}
163+
164+
165+
#include <memory>
166+
#include <TFile.h>
167+
#include <TFileMerger.h>
168+
#include <TTree.h>
169+
170+
// https://github.com/root-project/root/issues/6640
171+
TEST(TFileMerger, ChangeFile)
172+
{
173+
{
174+
TFile f{"file6640mergerinput.root", "RECREATE"};
175+
176+
TTree t{"T", "SetMaxTreeSize(1000)", 99, &f};
177+
int x;
178+
auto nentries = 20000;
179+
180+
t.Branch("x", &x, "x/I");
181+
182+
// Call function to forcedly trigger TTree::ChangeFile.
183+
// This will produce in total 3 files:
184+
// * file6640mergerinput.root
185+
// * file6640mergerinput_1.root
186+
// * file6640mergerinput_2.root
187+
TTree::SetMaxTreeSize(1000);
188+
189+
for (auto i = 0; i < nentries; i++)
190+
{
191+
x = i;
192+
t.Fill();
193+
}
194+
195+
// Write last file to disk
196+
auto cf = t.GetCurrentFile();
197+
cf->Write();
198+
cf->Close();
199+
}
200+
{
201+
TFileMerger filemerger{false, false};
202+
filemerger.OutputFile(std::unique_ptr<TFile>{TFile::Open("file6640mergeroutput.root", "RECREATE")});
203+
204+
TFile fin{"file6640mergerinput.root", "READ"};
205+
TFile fin1{"file6640mergerinput_1.root", "READ"};
206+
TFile fin2{"file6640mergerinput_2.root", "READ"};
207+
208+
filemerger.AddAdoptFile(&fin);
209+
filemerger.AddAdoptFile(&fin1);
210+
filemerger.AddAdoptFile(&fin2);
211+
212+
// Before the fix, TTree::ChangeFile was called during Merge
213+
// in the end deleting the TFileMerger's output file and leading to a crash.
214+
filemerger.Merge();
215+
}
216+
{
217+
TFile fout{"file6640mergeroutput.root", "READ"};
218+
EXPECT_EQ(fout.Get<TTree>("T")->GetEntries(), 20000);
219+
}
220+
gSystem->Unlink("file6640mergerinput.root");
221+
gSystem->Unlink("file6640mergerinput_1.root");
222+
gSystem->Unlink("file6640mergerinput_2.root");
223+
gSystem->Unlink("file6640mergeroutput.root");
224+
}

tree/tree/src/TTree.cxx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,6 +2742,10 @@ bool TTree::EnableCache()
27422742

27432743
TFile* TTree::ChangeFile(TFile* file)
27442744
{
2745+
// Changing file clashes with the design of TMemFile and derivates, see #6523.
2746+
// as well as with TFileMerger operations, see #6640
2747+
if ((dynamic_cast<TMemFile *>(file)) || file->TestBit(TFile::kCannotChange))
2748+
return file;
27452749
file->cd();
27462750
Write();
27472751
Reset();
@@ -2787,7 +2791,7 @@ TFile* TTree::ChangeFile(TFile* file)
27872791
break;
27882792
}
27892793
++nus;
2790-
Warning("ChangeFile", "file %s already exist, trying with %d underscores", fname, nus+1);
2794+
Warning("ChangeFile", "file %s already exists, trying with %d underscores", fname, nus+1);
27912795
}
27922796
Int_t compress = file->GetCompressionSettings();
27932797
TFile* newfile = TFile::Open(fname, "recreate", "chain files", compress);
@@ -4779,9 +4783,7 @@ Int_t TTree::Fill()
47794783
if (fDirectory)
47804784
if (TFile *file = fDirectory->GetFile())
47814785
if (static_cast<TDirectory *>(file) == fDirectory && (file->GetEND() > fgMaxTreeSize))
4782-
// Changing file clashes with the design of TMemFile and derivates, see #6523.
4783-
if (!(dynamic_cast<TMemFile *>(file)))
4784-
ChangeFile(file);
4786+
ChangeFile(file);
47854787

47864788
return nerror == 0 ? nbytes : -1;
47874789
}

0 commit comments

Comments
 (0)