From 841fc89fc53d18df2ec0ee3cd77e4fe7e658179d Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Thu, 23 Oct 2025 12:37:51 +0200 Subject: [PATCH] [df] Fix logic to check when implicit MT is enabled RTTreeDS needs to act differently w.r.t. TTreeReader creation whether implicit MT is enabled or not. Fix the logic to use the more appropriate ROOT::IsImplicitMTEnabled rather than just checking for the number of active slots. --- tree/dataframe/src/RTTreeDS.cxx | 17 +++++----- tree/dataframe/test/datasource_tree.cxx | 44 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/tree/dataframe/src/RTTreeDS.cxx b/tree/dataframe/src/RTTreeDS.cxx index cf2a4c5c4c8ab..6d5836b594286 100644 --- a/tree/dataframe/src/RTTreeDS.cxx +++ b/tree/dataframe/src/RTTreeDS.cxx @@ -399,15 +399,14 @@ void ROOT::Internal::RDF::RTTreeDS::Finalize() void ROOT::Internal::RDF::RTTreeDS::Initialize() { - if (fNSlots == 1) { - assert(!fTreeReader); - fTreeReader = std::make_unique(fTree.get(), fTree->GetEntryList(), /*warnAboutLongerFriends*/ true); - if (fGlobalEntryRange.has_value() && fGlobalEntryRange->first <= std::numeric_limits::max() && - fGlobalEntryRange->second <= std::numeric_limits::max() && fTreeReader && - fTreeReader->SetEntriesRange(fGlobalEntryRange->first, fGlobalEntryRange->second) != - TTreeReader::kEntryValid) { - throw std::logic_error("Something went wrong in initializing the TTreeReader."); - } + if (ROOT::IsImplicitMTEnabled()) + return; + assert(!fTreeReader); + fTreeReader = std::make_unique(fTree.get(), fTree->GetEntryList(), /*warnAboutLongerFriends*/ true); + if (fGlobalEntryRange.has_value() && fGlobalEntryRange->first <= std::numeric_limits::max() && + fGlobalEntryRange->second <= std::numeric_limits::max() && fTreeReader && + fTreeReader->SetEntriesRange(fGlobalEntryRange->first, fGlobalEntryRange->second) != TTreeReader::kEntryValid) { + throw std::logic_error("Something went wrong in initializing the TTreeReader."); } } diff --git a/tree/dataframe/test/datasource_tree.cxx b/tree/dataframe/test/datasource_tree.cxx index 6acabbcf60935..3c8235d315f5e 100644 --- a/tree/dataframe/test/datasource_tree.cxx +++ b/tree/dataframe/test/datasource_tree.cxx @@ -56,3 +56,47 @@ TEST(RTTreeDS, BranchWithNestedSameName) expect_vec_eq(branchNames, expectedBranchNames); } + +#ifdef R__USE_IMT +struct Dataset20164RAIII { + const char *fTreeName{"tree_20164"}; + const char *fFileName{"tree_20164.root"}; + + Dataset20164RAIII() + { + std::unique_ptr ofile{TFile::Open(fFileName, "recreate")}; + + std::unique_ptr myTree = std::make_unique(fTreeName, fTreeName); + + int v1{42}; + + myTree->Branch("v1", &v1); + myTree->Fill(); + ofile->Write(); + + // It's important to use 1 as the logic in RTTreeDS was faulty only for the case fNSlots==1 + ROOT::EnableImplicitMT(1); + } + + ~Dataset20164RAIII() + { + ROOT::DisableImplicitMT(); + std::remove(fFileName); + } +}; + +TEST(RTTreeDS, Regression20164) +{ + // Test that TTree thread tasks coherently use the TTreeReader created for the specific task and do not access + // a TTreeReader from the RTTreeDS. + Dataset20164RAIII dataset{}; + + auto chain = std::make_unique(dataset.fTreeName); + chain->Add(dataset.fFileName); + + auto val = ROOT::RDataFrame(*chain).Sum("v1").GetValue(); + + ASSERT_EQ(val, 42); +} + +#endif