Protect TBranch::SetAddress with a lock

This fixes #48.

include/PECFwk/core/ROOTLock.hpp

@@ -1,22 +1,19 @@
-/**
- * \file ROOTLock.hpp
- * 
- * The module defines a global locking mechanism to protect thread-unsafe ROOT routines.
- */
-
 #pragma once
 
 #include <mutex>
 
 
 /**
  * \class ROOTLock
- * \brief Provides a lock to protect thread-unsafe ROOT routines
+ * \brief Provides a lock to protect ROOT routines that are not thread-safe
  * 
- * The class is a simple wrapper around a static mutex. Since ROOT is not thread-safe, this lock
- * must be used to mark all the crical blocks of code that call ROOT routines. In particular,
- * creation of any ROOT objects must be guarded with the help of this class. The user should also
- * keep in mind that threads share the current ROOT directory object (gDirectory).
+ * This class is a simple wrapper around a static mutex. In general, ROOT is not thread-safe, and
+ * a number of operations must be protected by a lock. Their list includes (but is likely not
+ * limited to) the following:
+ *   * creation of objects inheriting from TObject, especially, TFile, TTree, and histograms;
+ *   * deletion of these objects;
+ *   * calling TTree::SetBranchAddress or TBranch::SetAddress.
+ * User should also remember that threads share the current ROOT directory object (gDirectory).
  */
 class ROOTLock
 {

modules/PECReader/src/PECGeneratorReader.cpp

@@ -1,6 +1,7 @@
 #include <PECFwk/PECReader/PECGeneratorReader.hpp>
 
 #include <PECFwk/core/Processor.hpp>
+#include <PECFwk/core/ROOTLock.hpp>
 #include <PECFwk/PECReader/PECInputData.hpp>
 
 #include <stdexcept>
@@ -47,11 +48,13 @@ void PECGeneratorReader::BeginRun(Dataset const &dataset)
     inputDataPlugin->LoadTree(treeName);
     TTree *tree = inputDataPlugin->ExposeTree(treeName);
 
+    ROOTLock::Lock();
     tree->SetBranchStatus("*", false);
     tree->SetBranchStatus("processId", true);
     tree->SetBranchStatus("nominalWeight", true);
 
     tree->SetBranchAddress("generator", &bfGeneratorPointer);
+    ROOTLock::Unlock();
 }
 
 

modules/PECReader/src/PECInputData.cpp

@@ -176,7 +176,9 @@ bool PECInputData::NextInputFile()
 
     // Set up the tree and update counters
     bfEventIDPointer = &bfEventID;
+    ROOTLock::Lock();
     eventIDTree->SetBranchAddress("eventId", &bfEventIDPointer);
+    ROOTLock::Unlock();
 
     nEvents = eventIDTree->GetEntries();
     nextEvent = 0;

modules/PECReader/src/PECJetMETReader.cpp

@@ -1,6 +1,7 @@
 #include <PECFwk/PECReader/PECJetMETReader.hpp>
 
 #include <PECFwk/core/Processor.hpp>
+#include <PECFwk/core/ROOTLock.hpp>
 #include <PECFwk/PECReader/PECInputData.hpp>
 
 #include <TVector2.h>
@@ -60,8 +61,10 @@ void PECJetMETReader::BeginRun(Dataset const &)
     // Set up the tree
     inputDataPlugin->LoadTree(treeName);
     TTree *tree = inputDataPlugin->ExposeTree(treeName);
+    ROOTLock::Lock();
     tree->SetBranchAddress("jets", &bfJetPointer);
     tree->SetBranchAddress("METs", &bfMETPointer);
+    ROOTLock::Unlock();
 }
 
 

modules/PECReader/src/PECLeptonReader.cpp

@@ -1,10 +1,10 @@
 #include <PECFwk/PECReader/PECLeptonReader.hpp>
 
 #include <PECFwk/core/Processor.hpp>
+#include <PECFwk/core/ROOTLock.hpp>
 #include <PECFwk/PECReader/PECInputData.hpp>
 
 #include <algorithm>
-/**/#include <iostream>
 
 
 PECLeptonReader::PECLeptonReader(std::string const name /*= "Leptons"*/):
@@ -37,15 +37,15 @@ void PECLeptonReader::BeginRun(Dataset const &)
       GetMaster().GetPluginBefore(inputDataPluginName, GetName()));
 
 
-    // Set up electron tree
+    // Set up the trees
     inputDataPlugin->LoadTree(electronTreeName);
+    inputDataPlugin->LoadTree(muonTreeName);
+
+    ROOTLock::Lock();
     inputDataPlugin->ExposeTree(electronTreeName)->
       SetBranchAddress("electrons", &bfElectronPointer);
-
-
-    // Set up muon tree
-    inputDataPlugin->LoadTree(muonTreeName);
     inputDataPlugin->ExposeTree(muonTreeName)->SetBranchAddress("muons", &bfMuonPointer);
+    ROOTLock::Unlock();
 }
 
 

modules/PECReader/src/PECPileUpReader.cpp

@@ -1,6 +1,7 @@
 #include <PECFwk/PECReader/PECPileUpReader.hpp>
 
 #include <PECFwk/core/Processor.hpp>
+#include <PECFwk/core/ROOTLock.hpp>
 #include <PECFwk/PECReader/PECInputData.hpp>
 
 
@@ -36,8 +37,10 @@ void PECPileUpReader::BeginRun(Dataset const &)
     //used are not read
     inputDataPlugin->LoadTree(treeName);
     TTree *tree = inputDataPlugin->ExposeTree(treeName);
+    ROOTLock::Lock();
     tree->SetBranchStatus("inTimeNumPU", false);
     tree->SetBranchAddress("puInfo", &bfPileUpInfoPointer);
+    ROOTLock::Unlock();
 }
 
 

modules/PECReader/src/PECTriggerFilter.cpp

@@ -1,10 +1,10 @@
 #include <PECFwk/PECReader/PECTriggerFilter.hpp>
 
 #include <PECFwk/core/Processor.hpp>
+#include <PECFwk/core/ROOTLock.hpp>
 #include <PECFwk/PECReader/PECInputData.hpp>
 
 #include <algorithm>
-/**/#include <iostream>
 
 
 using namespace std::literals::string_literals;
@@ -30,7 +30,6 @@ void PECTriggerFilter::BeginRun(Dataset const &)
     // Register reading the tree with trigger information and by default disable all branches
     inputDataPlugin->LoadTree(triggerTreeName);
     triggerTree = inputDataPlugin->ExposeTree(triggerTreeName);
-    triggerTree->SetBranchStatus("*", false);
 }
 
 
@@ -99,18 +98,24 @@ bool PECTriggerFilterData::ProcessEvent()
         // A valid trigger range has been found. Get the corresponding branch of the tree and
         //assign the buffer to it
         currentRange = *res;
+
+        ROOTLock::Lock();
         triggerTree->SetBranchStatus("*", false);
 
         TBranch *branch =
          triggerTree->GetBranch((currentRange->GetDataTriggerPattern() + "__accept").c_str());
 
         if (not branch)
+        {
+            ROOTLock::Unlock();
             throw std::runtime_error("PECTriggerFilterData::ProcessEvent: State of the trigger "s +
              "\"HLT_" + currentRange->GetDataTriggerPattern() + "_v*\" is not stored in the "
              "source tree.");
+        }
 
         branch->SetStatus(true);
         branch->SetAddress(&bfAccepted);
+        ROOTLock::Unlock();
     }
 
 
@@ -159,6 +164,8 @@ void PECTriggerFilterMC::BeginRun(Dataset const &dataset)
 
     // Set statuses and addresses of relevant branches of the trigger tree. Take into account that
     //the same trigger might be specified in several trigger ranges
+    ROOTLock::Lock();
+
     triggerTree->SetBranchStatus("*", false);
     unsigned curBufferIndex = 0;
 
@@ -182,8 +189,11 @@ void PECTriggerFilterMC::BeginRun(Dataset const &dataset)
             TBranch *branch = triggerTree->GetBranch((curTriggerName + "__accept").c_str());
 
             if (not branch)
+            {
+                ROOTLock::Unlock();
                 throw std::runtime_error("PECTriggerFilterMC::BeginRun: State of the trigger "s +
                  "\"HLT_" + curTriggerName + "_v*\" is not stored in the source tree.");
+            }
 
 
             // Set the status and address of the branch
@@ -204,6 +214,8 @@ void PECTriggerFilterMC::BeginRun(Dataset const &dataset)
             ranges.at(i).second = ranges.at(iPrev).second;
         }
     }
+
+    ROOTLock::Unlock();
 }
 
 

modules/extensions/src/BasicKinematicsPlugin.cpp

@@ -70,10 +70,6 @@ void BasicKinematicsPlugin::BeginRun(Dataset const &dataset)
     // Create the tree
     tree.reset(new TTree("Vars", "Basic kinematical variables"));
 
-    // End of critical block
-    ROOTLock::Unlock();
-
-
     // Assign branch addresses
     tree->Branch("Pt_Lep", &Pt_Lep);
     tree->Branch("Eta_Lep", &Eta_Lep);
@@ -86,6 +82,9 @@ void BasicKinematicsPlugin::BeginRun(Dataset const &dataset)
     tree->Branch("MET", &MET);
     tree->Branch("MtW", &MtW);
     tree->Branch("nPV", &nPV);
+
+    // End of critical block
+    ROOTLock::Unlock();
 }