[io] Fix data loss in nested directories during batched TFileMerger

io/io/src/TFileMerger.cxx

@@ -828,7 +828,7 @@ Bool_t TFileMerger::MergeOne(TDirectory *target, TList *sourcelist, Int_t type,
       // and we are in incremental mode (because it will be reused
       // and has not been written to disk (for performance reason).
       // coverity[var_deref_model] the IsA()->InheritsFrom guarantees that the dynamic_cast will succeed.
-      if (ownobj && (!(type & kIncremental) || dirobj->GetFile() != target)) {
+      if (ownobj && (!(type & kIncremental) || dirobj->GetFile() != fOutputFile)) {
          dirobj->ResetBit(kMustCleanup);
          delete dirobj;
       }

io/io/test/TFileMergerTests.cxx

@@ -478,3 +478,97 @@ TEST(TFileMerger, TypeMismatchErrorTObjectWithNonTObject)
       fm.PartialMerge();
    }
 }
+
+// Regression test for https://github.com/root-project/root/issues/23666
+TEST(TFileMerger, NestedDirMaxOpenedFiles)
+{
+   struct CleanupRAII {
+      std::vector<std::string> fFiles;
+      ~CleanupRAII()
+      {
+         for (const auto &file : fFiles) {
+            gSystem->Unlink(file.c_str());
+         }
+      }
+   } cleanup;
+
+   const std::string file1 = "test_nested_merge_in1.root";
+   const std::string file2 = "test_nested_merge_in2.root";
+   const std::string file3 = "test_nested_merge_in3.root";
+   const std::string fileOut = "test_nested_merge_out.root";
+
+   cleanup.fFiles.push_back(file1);
+   cleanup.fFiles.push_back(file2);
+   cleanup.fFiles.push_back(file3);
+   cleanup.fFiles.push_back(fileOut);
+
+   const std::vector<std::pair<std::string, int>> inputs = {{file1, 5}, {file2, 7}, {file3, 11}};
+
+   // Create 3 input files with objects inside a nested directory (depth >= 2)
+   for (const auto &input : inputs) {
+      auto f = std::make_unique<TFile>(input.first.c_str(), "RECREATE");
+      ASSERT_FALSE(f->IsZombie());
+      auto topDir = f->mkdir("nested");
+      ASSERT_NE(topDir, nullptr);
+      auto subDir = topDir->mkdir("dir");
+      ASSERT_NE(subDir, nullptr);
+      subDir->cd();
+
+      auto tree = std::make_unique<TTree>("tree", "Test Tree");
+      int val = 0;
+      tree->Branch("val", &val, "val/I");
+      for (int i = 0; i < input.second; ++i) {
+         val = i;
+         tree->Fill();
+      }
+
+      TH1F hist("hist", "Test Hist", 10, 0, 10);
+      for (int i = 0; i < input.second; ++i) {
+         hist.Fill(i % 10);
+      }
+
+      f->Write();
+   }
+
+   // Merge the input files into the output file. The original issue was triggered by the use of the `-n` parameter in
+   // `hadd`, which opens at most `N` files at once, set to the same number of input files (3). The merger used to only
+   // merge data from the first two files leaving out data from the third.
+   {
+      TFileMerger merger{false};
+      EXPECT_TRUE(merger.OutputFile(fileOut.c_str()));
+      merger.SetMaxOpenedFiles(3);
+      merger.AddFile(file1.c_str(), /*cpProgress*/ false);
+      merger.AddFile(file2.c_str(), /*cpProgress*/ false);
+      merger.AddFile(file3.c_str(), /*cpProgress*/ false);
+      EXPECT_TRUE(merger.Merge());
+   }
+
+   // Verify correct values for merged objects in the output file
+   {
+      auto fOut = std::make_unique<TFile>(fileOut.c_str(), "READ");
+      ASSERT_FALSE(fOut->IsZombie());
+
+      auto topDir = fOut->Get<TDirectory>("nested");
+      ASSERT_NE(topDir, nullptr);
+      auto subDir = topDir->Get<TDirectory>("dir");
+      ASSERT_NE(subDir, nullptr);
+
+      auto tree = subDir->Get<TTree>("tree");
+      ASSERT_NE(tree, nullptr);
+      EXPECT_EQ(tree->GetEntries(), 5 + 7 + 11);
+      int val = 0;
+      tree->SetBranchAddress("val", &val);
+      int entryCursor{};
+      for (const auto &input : inputs) {
+         for (int i = 0; i < input.second; ++i) {
+            tree->GetEntry(entryCursor);
+            EXPECT_EQ(val, i);
+            entryCursor++;
+         }
+      }
+
+      auto hist = subDir->Get<TH1F>("hist");
+      ASSERT_NE(hist, nullptr);
+      EXPECT_EQ(hist->GetEntries(), 5 + 7 + 11);
+   }
+}

roottest/root/io/hadd/CMakeLists.txt

@@ -284,3 +284,26 @@ ROOTTEST_ADD_TEST(test_hadd_merge_rntuple_634,
                   COMMAND ${ROOT_hadd_CMD} -f test_hadd_merge_rntuple_634_merged.root test_hadd_merge_rntuple_634_1.root test_hadd_merge_rntuple_634_2.root
                   POSTCMD ${ROOT_root_CMD} -q check_merge_rntuple_634.C
                   PASSRC 0)
+
+
+# Regression test for https://github.com/root-project/root/issues/23666
+configure_file(test_hadd_nested_objects_23666.root . COPYONLY)
+configure_file(hadd_check_nested_objects_argn_23666.C . COPYONLY)
+ROOTTEST_ADD_TEST(test_hadd_nested_objects_argn_23666,
+                  PRECMD ${CMAKE_COMMAND} -E rm -f test_hadd_nested_objects_argn_23666_merged.root
+                  COMMAND ${ROOT_hadd_CMD} -f -n 3 test_hadd_nested_objects_argn_23666_merged.root test_hadd_nested_objects_23666.root test_hadd_nested_objects_23666.root test_hadd_nested_objects_23666.root
+                  POSTCMD ${ROOT_root_CMD} -q hadd_check_nested_objects_argn_23666.C
+                  PASSRC 0)
+
+configure_file(hadd_check_nested_objects_128files_23666.C . COPYONLY)
+configure_file(test_hadd_nested_objects_128files_23666.txt . COPYONLY)
+ROOTTEST_ADD_TEST(test_hadd_nested_objects_128files_23666,
+                  PRECMD ${CMAKE_COMMAND} -E rm -f test_hadd_nested_objects_128files_23666_merged.root
+                  COMMAND ${ROOT_hadd_CMD} -f test_hadd_nested_objects_128files_23666_merged.root @test_hadd_nested_objects_128files_23666.txt
+                  POSTCMD ${ROOT_root_CMD} -q hadd_check_nested_objects_128files_23666.C
+                  PASSRC 0)
+ROOTTEST_ADD_TEST(test_hadd_nested_objects_128files_n16_23666,
+                  PRECMD ${CMAKE_COMMAND} -E rm -f test_hadd_nested_objects_128files_23666_merged.root
+                  COMMAND ${ROOT_hadd_CMD} -f -n 16 test_hadd_nested_objects_128files_23666_merged.root @test_hadd_nested_objects_128files_23666.txt
+                  POSTCMD ${ROOT_root_CMD} -q hadd_check_nested_objects_128files_23666.C
+                  PASSRC 0)

roottest/root/io/hadd/hadd_check_nested_objects_128files_23666.C

@@ -0,0 +1,12 @@
+int hadd_check_nested_objects_argn_23666()
+{
+   auto file = std::unique_ptr<TFile>(TFile::Open("test_hadd_nested_objects_128files_23666_merged.root"));
+   auto tree = std::unique_ptr<TTree>(file->Get<TTree>("A/B/tree"));
+
+   if (auto nentries = tree->GetEntries(); nentries != 128) {
+      std::cerr << "Number of entries is " << nentries << " (expected 128) \n";
+      return 1;
+   }
+
+   return 0;
+}

roottest/root/io/hadd/hadd_check_nested_objects_argn_23666.C

@@ -0,0 +1,12 @@
+int hadd_check_nested_objects_argn_23666()
+{
+   auto file = std::unique_ptr<TFile>(TFile::Open("test_hadd_nested_objects_argn_23666_merged.root"));
+   auto tree = std::unique_ptr<TTree>(file->Get<TTree>("A/B/tree"));
+
+   if (auto nentries = tree->GetEntries(); nentries != 3) {
+      std::cerr << "Number of entries is " << nentries << " (expected 3) \n";
+      return 1;
+   }
+
+   return 0;
+}

roottest/root/io/hadd/test_hadd_nested_objects_128files_23666.txt

@@ -0,0 +1,128 @@
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root
+test_hadd_nested_objects_23666.root