From 31957f9d81cb7475a682a3a1cdca59e124bc665b Mon Sep 17 00:00:00 2001
From: Marilena Bandieramonte <marilena.bandieramonte@cern.ch>
Date: Tue, 7 May 2019 13:53:26 +0000
Subject: [PATCH] Fix thread-unsafety in EscapedEnergyRegistry class
[ATLASSIM-4106]
EscapedEnergyRegistry class was implemented as a singleton, so it was not thread-safe.
This affected the HITs of LArCalibrationHitDeadMaterial when running in MT with the CaloCalibration on.
Added the #ifdef G4MULTITHREADED directive to handle
the multithreaded case. One instance of the class will be created
per each thread and stored in a tbb::concurrent_unordered_map that
is hashed with the threadID number.
---
.../CaloG4Sim/EscapedEnergyRegistry.h | 28 ++++++++++++-
.../CaloG4Sim/src/EscapedEnergyRegistry.cc | 41 ++++++++++++++++---
.../LArG4/LArG4H6SD/src/H62004DeadSDTool.cc | 4 +-
.../LArG4/LArG4H8SD/src/H8CalibSDTool.cc | 4 +-
.../LArG4/LArG4SD/src/DeadSDTool.cc | 4 +-
.../TileGeoG4Calib/src/TileGeoG4CalibSD.cc | 2 +-
6 files changed, 69 insertions(+), 14 deletions(-)
diff --git a/Calorimeter/CaloG4Sim/CaloG4Sim/EscapedEnergyRegistry.h b/Calorimeter/CaloG4Sim/CaloG4Sim/EscapedEnergyRegistry.h
index 1c02fa07b93..7c5168ef08d 100644
--- a/Calorimeter/CaloG4Sim/CaloG4Sim/EscapedEnergyRegistry.h
+++ b/Calorimeter/CaloG4Sim/CaloG4Sim/EscapedEnergyRegistry.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
*/
#ifndef CaloG4_EscapedEnergyRegistry_H
@@ -10,6 +10,10 @@
#include <map>
+#include <thread>
+#ifdef G4MULTITHREADED
+# include "tbb/concurrent_unordered_map.h"
+#endif
namespace CaloG4
{
@@ -30,6 +34,15 @@ namespace CaloG4
/// Since there's only one registry, this class uses the singleton
/// pattern.
///
+ /// UPDATE:
+ ///
+ ///12-Apr-2019 Marilena Bandieramonte
+ ///
+ /// This singleton class was not thread-safe.
+ /// Added the #ifdef G4MULTITHREADED directive to handle
+ /// the multithreaded case. One instance of the class will be created
+ /// per each thread and stored in a tbb::concurrent_unordered_map that
+ /// is hashed with the threadID number.
class EscapedEnergyRegistry
{
public:
@@ -57,7 +70,18 @@ namespace CaloG4
typedef m_processingMap_t::iterator m_processingMap_ptr_t;
typedef m_processingMap_t::const_iterator m_processingMap_const_ptr_t;
m_processingMap_t m_processingMap;
-
+#ifdef G4MULTITHREADED
+ // Thread-to-EscapeEnergyRegistry concurrent map type
+ using EERThreadMap_t = tbb::concurrent_unordered_map< std::thread::id, EscapedEnergyRegistry*, std::hash<std::thread::id> >;
+ // Concurrent map of EERs, one for each thread
+ static EERThreadMap_t m_EERThreadMap;
+ // @brief Search inside m_EERThreadMap the element with the current threadID
+ // and return it or return a null pointer if the element is not found
+ static EscapedEnergyRegistry* getEER();
+ // @brief Insert the current EER in m_EERThreadMap and
+ // associate it with the current threadID
+ static EscapedEnergyRegistry* setEER();
+#endif
};
} // namespace CaloG4
diff --git a/Calorimeter/CaloG4Sim/src/EscapedEnergyRegistry.cc b/Calorimeter/CaloG4Sim/src/EscapedEnergyRegistry.cc
index 11a9808cc89..40c265b9df6 100644
--- a/Calorimeter/CaloG4Sim/src/EscapedEnergyRegistry.cc
+++ b/Calorimeter/CaloG4Sim/src/EscapedEnergyRegistry.cc
@@ -1,10 +1,11 @@
/*
- Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
*/
// EscapedEnergyRegistry
// 15-Jul-2004 William Seligman
-
+// 12-Apr-2019 Marilena Bandieramonte
+//
#include "CaloG4Sim/EscapedEnergyRegistry.h"
#include "CaloG4Sim/VEscapedEnergyProcessing.h"
@@ -15,14 +16,24 @@
namespace CaloG4
{
+#ifdef G4MULTITHREADED
+EscapedEnergyRegistry::EERThreadMap_t EscapedEnergyRegistry::m_EERThreadMap;
+#endif
- // Standard implementation of a singleton pattern.
- EscapedEnergyRegistry* EscapedEnergyRegistry::GetInstance()
+EscapedEnergyRegistry* EscapedEnergyRegistry::GetInstance()
{
+#ifdef G4MULTITHREADED
+ auto eer = getEER();
+ if (!eer) //nullpointer if it is not found
+ return setEER();
+ else return eer;
+#else
+ //Standard implementation of a Singleton Pattern
static EscapedEnergyRegistry instance;
return &instance;
+#endif
}
-
+
EscapedEnergyRegistry::EscapedEnergyRegistry()
{}
@@ -37,6 +48,26 @@ namespace CaloG4
}
}
+#ifdef G4MULTITHREADED
+ EscapedEnergyRegistry* EscapedEnergyRegistry::getEER()
+ {
+ // Get current thread-ID
+ const auto tid = std::this_thread::get_id();
+ auto eerPair = m_EERThreadMap.find(tid);
+ if(eerPair == m_EERThreadMap.end())
+ return nullptr; //if not found return null pointer
+ else return eerPair->second;
+ }
+
+ EscapedEnergyRegistry* EscapedEnergyRegistry::setEER()
+ {
+ EscapedEnergyRegistry* instance = new EscapedEnergyRegistry;
+ const auto tid = std::this_thread::get_id();
+ auto inserted = m_EERThreadMap.insert( std::make_pair(tid, instance)).first;
+ return (EscapedEnergyRegistry*) inserted->second;
+ }
+#endif
+
void EscapedEnergyRegistry::AddAndAdoptProcessing( const G4String& name,
VEscapedEnergyProcessing* process )
{
diff --git a/LArCalorimeter/LArG4/LArG4H6SD/src/H62004DeadSDTool.cc b/LArCalorimeter/LArG4/LArG4H6SD/src/H62004DeadSDTool.cc
index 9f654e6e0be..cec5ec420c1 100644
--- a/LArCalorimeter/LArG4/LArG4H6SD/src/H62004DeadSDTool.cc
+++ b/LArCalorimeter/LArG4/LArG4H6SD/src/H62004DeadSDTool.cc
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
*/
#include "H62004DeadSDTool.h"
@@ -50,7 +50,7 @@ namespace LArG4
// Initialize the escaped energy processing for LAr volumes.
// This is from initialize processing in the former LArG4CalibSD.
// I still think we can do better than this, though.
- // FIXME: I don't think this is thread safe!!
+ // UPDATE: this is thread-safe now
ATH_MSG_DEBUG("Creating EscapedEnergyProcessing and adding to registry");
CaloG4::VEscapedEnergyProcessing* eep =
new EscapedEnergyProcessing( uninstSD.get() );
diff --git a/LArCalorimeter/LArG4/LArG4H8SD/src/H8CalibSDTool.cc b/LArCalorimeter/LArG4/LArG4H8SD/src/H8CalibSDTool.cc
index 19664e06660..3fae5f8e7b0 100644
--- a/LArCalorimeter/LArG4/LArG4H8SD/src/H8CalibSDTool.cc
+++ b/LArCalorimeter/LArG4/LArG4H8SD/src/H8CalibSDTool.cc
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
*/
#include "H8CalibSDTool.h"
@@ -96,7 +96,7 @@ StatusCode H8CalibSDTool::initializeCalculators()
std::vector<std::string> emptyStringVec;
auto uninstSD =
makeOneSD("Default::Dead::Uninstrumented::Calibration::Region", &*m_h8defaultcalc, emptyStringVec);
- // WARNING: This probably isn't thread safe!
+ // UPDATE: This is thread-safe now
CaloG4::VEscapedEnergyProcessing* eep =
new EscapedEnergyProcessing( uninstSD.get() );
auto registry = CaloG4::EscapedEnergyRegistry::GetInstance();
diff --git a/LArCalorimeter/LArG4/LArG4SD/src/DeadSDTool.cc b/LArCalorimeter/LArG4/LArG4SD/src/DeadSDTool.cc
index c1212f206bc..c63dae803f8 100644
--- a/LArCalorimeter/LArG4/LArG4SD/src/DeadSDTool.cc
+++ b/LArCalorimeter/LArG4/LArG4SD/src/DeadSDTool.cc
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
*/
#include "DeadSDTool.h"
@@ -120,7 +120,7 @@ namespace LArG4
// Initialize the escaped energy processing for LAr volumes.
// This is from initialize processing in the former LArG4CalibSD.
// I still think we can do better than this, though.
- // FIXME: I don't think this is thread safe!!
+ // UPDATE: this is thread-safe now
ATH_MSG_DEBUG("Creating EscapedEnergyProcessing and adding to registry");
CaloG4::VEscapedEnergyProcessing* eep =
new EscapedEnergyProcessing( uninstSD.get() );
diff --git a/TileCalorimeter/TileG4/TileGeoG4Calib/src/TileGeoG4CalibSD.cc b/TileCalorimeter/TileG4/TileGeoG4Calib/src/TileGeoG4CalibSD.cc
index 29fab721b69..9b41c559b09 100644
--- a/TileCalorimeter/TileG4/TileGeoG4Calib/src/TileGeoG4CalibSD.cc
+++ b/TileCalorimeter/TileG4/TileGeoG4Calib/src/TileGeoG4CalibSD.cc
@@ -126,7 +126,7 @@ TileGeoG4CalibSD::TileGeoG4CalibSD(const G4String& name, const std::vector<std::
m_tile_eep->SetEnergy5(0.);
m_tile_eep->SetEscapedEnergy(0.);
- // @TODO : Watch out for this!! Is it a singleton or something??
+ // @UPDATE: this is thread-safe now. EscapedEnergyRegistry is not a singleton in MT mode
CaloG4::EscapedEnergyRegistry* registry = CaloG4::EscapedEnergyRegistry::GetInstance();
registry->AddAndAdoptProcessing("Tile", m_tile_eep);
--
GitLab