From d10aff353ec55d487d3600e3b9f89b54cc96b661 Mon Sep 17 00:00:00 2001 From: scott snyder <sss@karma> Date: Tue, 26 Jan 2021 16:13:59 -0500 Subject: [PATCH] MuonMM_CnvTools: const fixes Preparing to make methods of IMuonRdoToPrepDataTool const. Adjust MmRdoToPrepDataTool* so that its interfaces may be made const. Also enable thread-safety checking. The old pre-MT MmRdoToPrepDataTool is declared as not thread-safe, as it has some mutable state, while the mutable state is removed from MmRdoToPrepDataToolMT. MmRdoToPrepDataToolCore is now abstract and is removed from the entries list. --- .../MuonCnv/MuonMM_CnvTools/CMakeLists.txt | 2 +- .../MuonMM_CnvTools/ATLAS_CHECK_THREAD_SAFETY | 1 + .../src/MmRdoToPrepDataTool.cxx | 10 ++--- .../MuonMM_CnvTools/src/MmRdoToPrepDataTool.h | 11 +++-- .../src/MmRdoToPrepDataToolCore.cxx | 43 +++++++------------ .../src/MmRdoToPrepDataToolCore.h | 31 ++++++------- .../src/MmRdoToPrepDataToolMT.cxx | 9 ++-- .../src/MmRdoToPrepDataToolMT.h | 4 +- .../components/MuonMM_CnvTools_entries.cxx | 2 - 9 files changed, 47 insertions(+), 66 deletions(-) create mode 100644 MuonSpectrometer/MuonCnv/MuonMM_CnvTools/MuonMM_CnvTools/ATLAS_CHECK_THREAD_SAFETY diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/CMakeLists.txt b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/CMakeLists.txt index dab2628d59bc..b511cd0bab6d 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/CMakeLists.txt +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/CMakeLists.txt @@ -19,5 +19,5 @@ atlas_add_component( MuonMM_CnvTools src/*.cxx src/components/*.cxx INCLUDE_DIRS ${TDAQ-COMMON_INCLUDE_DIRS} - LINK_LIBRARIES ${TDAQ-COMMON_LIBRARIES} ByteStreamData ByteStreamData_test GaudiKernel AthenaBaseComps SGTools StoreGateLib SGtests AtlasDetDescr Identifier ByteStreamCnvSvcBaseLib MuonReadoutGeometry MuonDigitContainer MuonIdHelpersLib MuonRDO MuonPrepRawData MMClusterizationLib NSWCalibToolsLib MuonCnvToolInterfacesLib MuonMM_CnvToolsLib ) + LINK_LIBRARIES ${TDAQ-COMMON_LIBRARIES} ByteStreamData ByteStreamData_test GaudiKernel AthenaBaseComps SGTools StoreGateLib SGtests AtlasDetDescr Identifier ByteStreamCnvSvcBaseLib MuonReadoutGeometry MuonDigitContainer MuonIdHelpersLib MuonRDO MuonPrepRawData MMClusterizationLib NSWCalibToolsLib MuonCnvToolInterfacesLib MuonMM_CnvToolsLib CxxUtils ) diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/MuonMM_CnvTools/ATLAS_CHECK_THREAD_SAFETY b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/MuonMM_CnvTools/ATLAS_CHECK_THREAD_SAFETY new file mode 100644 index 000000000000..3c63df19ac2f --- /dev/null +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/MuonMM_CnvTools/ATLAS_CHECK_THREAD_SAFETY @@ -0,0 +1 @@ +MuonSpectrometer/MuonCnv/MuonMM_CnvTools/MuonMM_CnvTools diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.cxx b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.cxx index de1002778d40..20a7ef09a95e 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.cxx +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.cxx @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #include "MmRdoToPrepDataTool.h" @@ -22,20 +22,18 @@ StatusCode Muon::MmRdoToPrepDataTool::initialize() return StatusCode::SUCCESS; } -Muon::MmRdoToPrepDataToolCore::SetupMM_PrepDataContainerStatus Muon::MmRdoToPrepDataTool::setupMM_PrepDataContainer() +Muon::MMPrepDataContainer* Muon::MmRdoToPrepDataTool::setupMM_PrepDataContainer() const { if(!evtStore()->contains<Muon::MMPrepDataContainer>(m_mmPrepDataContainerKey.key())){ - m_fullEventDone=false; SG::WriteHandle< Muon::MMPrepDataContainer > handle(m_mmPrepDataContainerKey); StatusCode status = handle.record(std::make_unique<Muon::MMPrepDataContainer>(m_idHelperSvc->mmIdHelper().module_hash_max())); if (status.isFailure() || !handle.isValid() ) { ATH_MSG_FATAL("Could not record container of MicroMega PrepData Container at " << m_mmPrepDataContainerKey.key()); - return FAILED; + return nullptr; } m_mmPrepDataContainer = handle.ptr(); - return ADDED; } - return ALREADYCONTAINED; + return m_mmPrepDataContainer; } diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.h b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.h index bd7431af3beb..e77a6883fa7a 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.h +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataTool.h @@ -1,17 +1,17 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #ifndef MUONMmRdoToPrepDataTool_H #define MUONMmRdoToPrepDataTool_H #include "MmRdoToPrepDataToolCore.h" - #include "MuonPrepRawData/MuonPrepDataContainer.h" +#include "CxxUtils/checker_macros.h" namespace Muon { - class MmRdoToPrepDataTool : virtual public MmRdoToPrepDataToolCore + class ATLAS_NOT_THREAD_SAFE MmRdoToPrepDataTool : virtual public MmRdoToPrepDataToolCore { public: MmRdoToPrepDataTool(const std::string&,const std::string&,const IInterface*); @@ -23,7 +23,10 @@ namespace Muon virtual StatusCode initialize() override; protected: - virtual SetupMM_PrepDataContainerStatus setupMM_PrepDataContainer() override; + virtual Muon::MMPrepDataContainer* setupMM_PrepDataContainer() const override; + + private: + mutable Muon::MMPrepDataContainer* m_mmPrepDataContainer = nullptr; }; } // end of namespace diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.cxx b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.cxx index 517f527637cc..f799bd0f8fa2 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.cxx +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.cxx @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #include "MmRdoToPrepDataToolCore.h" @@ -22,13 +22,11 @@ Muon::MmRdoToPrepDataToolCore::MmRdoToPrepDataToolCore(const std::string& t, const std::string& n, const IInterface* p ) : - AthAlgTool(t,n,p), - m_fullEventDone(false), - m_mmPrepDataContainer(nullptr) + AthAlgTool(t,n,p) { declareInterface<Muon::IMuonRdoToPrepDataTool>(this); - // template for property decalration + // template for property declaration declareProperty("OutputCollection", m_mmPrepDataContainerKey = std::string("MM_Measurements"), "Muon::MMPrepDataContainer to record"); declareProperty("InputCollection", m_rdoContainerKey = std::string("MMRDO"), @@ -50,8 +48,9 @@ StatusCode Muon::MmRdoToPrepDataToolCore::initialize() return StatusCode::SUCCESS; } -StatusCode Muon::MmRdoToPrepDataToolCore::processCollection(const MM_RawDataCollection *rdoColl, - std::vector<IdentifierHash>& idWithDataVect) +StatusCode Muon::MmRdoToPrepDataToolCore::processCollection(Muon::MMPrepDataContainer* mmPrepDataContainer, + const MM_RawDataCollection *rdoColl, + std::vector<IdentifierHash>& idWithDataVect) const { ATH_MSG_DEBUG(" ***************** Start of process MM Collection"); @@ -64,7 +63,7 @@ StatusCode Muon::MmRdoToPrepDataToolCore::processCollection(const MM_RawDataColl MMPrepDataCollection* prdColl = nullptr; // check if the collection already exists, otherwise add it - if ( m_mmPrepDataContainer->indexFindPtr(hash) != nullptr) { + if ( mmPrepDataContainer->indexFindPtr(hash) != nullptr) { ATH_MSG_DEBUG("In processCollection: collection already contained in the MM PrepData container"); return StatusCode::FAILURE; @@ -85,7 +84,7 @@ StatusCode Muon::MmRdoToPrepDataToolCore::processCollection(const MM_RawDataColl prdColl->setIdentifier(moduleId); } - if (StatusCode::SUCCESS != m_mmPrepDataContainer->addCollection(prdColl, hash)) { + if (StatusCode::SUCCESS != mmPrepDataContainer->addCollection(prdColl, hash)) { ATH_MSG_DEBUG("In processCollection - Couldn't record in the Container MM Collection with hashID = " << (int)hash ); return StatusCode::FAILURE; @@ -214,14 +213,8 @@ StatusCode Muon::MmRdoToPrepDataToolCore::processCollection(const MM_RawDataColl } -Muon::MmRdoToPrepDataToolCore::SetupMM_PrepDataContainerStatus Muon::MmRdoToPrepDataToolCore::setupMM_PrepDataContainer() +const MM_RawDataContainer* Muon::MmRdoToPrepDataToolCore::getRdoContainer() const { - return FAILED; -} - - -const MM_RawDataContainer* Muon::MmRdoToPrepDataToolCore::getRdoContainer() { - auto rdoContainerHandle = SG::makeHandle(m_rdoContainerKey); if(rdoContainerHandle.isValid()) { ATH_MSG_DEBUG("MM_getRdoContainer success"); @@ -233,7 +226,8 @@ const MM_RawDataContainer* Muon::MmRdoToPrepDataToolCore::getRdoContainer() { } -void Muon::MmRdoToPrepDataToolCore::processRDOContainer( std::vector<IdentifierHash>& idWithDataVect ) +void Muon::MmRdoToPrepDataToolCore::processRDOContainer( Muon::MMPrepDataContainer* mmPrepDataContainer, + std::vector<IdentifierHash>& idWithDataVect ) const { ATH_MSG_DEBUG("In processRDOContainer"); @@ -248,7 +242,7 @@ void Muon::MmRdoToPrepDataToolCore::processRDOContainer( std::vector<IdentifierH auto rdoColl = *it; if (rdoColl->empty()) continue; ATH_MSG_DEBUG("New RDO collection with " << rdoColl->size() << "MM Hits"); - if(processCollection(rdoColl, idWithDataVect).isFailure()) { + if(processCollection(mmPrepDataContainer, rdoColl, idWithDataVect).isFailure()) { ATH_MSG_DEBUG("processCsm returns a bad StatusCode - keep going for new data collections in this event"); } } @@ -268,21 +262,14 @@ StatusCode Muon::MmRdoToPrepDataToolCore::decode( std::vector<IdentifierHash>& i //is idVect a right thing to use here? to be reviewed maybe ATH_MSG_DEBUG("Size of the RDO container to be decoded: " << idVect.size() ); - SetupMM_PrepDataContainerStatus containerRecordStatus = setupMM_PrepDataContainer(); + Muon::MMPrepDataContainer* mmPrepDataContainer = setupMM_PrepDataContainer(); - if ( containerRecordStatus == FAILED ) { + if ( !mmPrepDataContainer ) { return StatusCode::FAILURE; } - processRDOContainer(idWithDataVect); + processRDOContainer(mmPrepDataContainer, idWithDataVect); - // check if the full event has already been decoded - if ( m_fullEventDone ) { - ATH_MSG_DEBUG ("Full event dcoded, nothing to do"); - return StatusCode::SUCCESS; - } - - return StatusCode::SUCCESS; } diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.h b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.h index f99bc526c2d6..4c0a0a183b20 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.h +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolCore.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #ifndef MUONMmRdoToPrepDataToolCore_H @@ -35,40 +35,35 @@ namespace Muon virtual ~MmRdoToPrepDataToolCore()=default; /** standard Athena-Algorithm method */ - virtual StatusCode initialize(); + virtual StatusCode initialize() override; /** Decode method - declared in Muon::IMuonRdoToPrepDataTool*/ - StatusCode decode( std::vector<IdentifierHash>& idVect, std::vector<IdentifierHash>& selectedIdVect ); + virtual StatusCode decode( std::vector<IdentifierHash>& idVect, std::vector<IdentifierHash>& selectedIdVect ) override; //new decode methods for Rob based readout StatusCode decode( const std::vector<uint32_t>& robIds, const std::vector<IdentifierHash>& chamberHashInRobs ); - StatusCode decode( const std::vector<uint32_t>& robIds ); + virtual StatusCode decode( const std::vector<uint32_t>& robIds ) override; - StatusCode processCollection(const MM_RawDataCollection *rdoColl, - std::vector<IdentifierHash>& idWithDataVect); + StatusCode processCollection(Muon::MMPrepDataContainer* mmPrepDataContainer, + const MM_RawDataCollection *rdoColl, + std::vector<IdentifierHash>& idWithDataVect) const; - void printInputRdo(); - void printPrepData(); + virtual void printInputRdo() override; + virtual void printPrepData() override; protected: - enum SetupMM_PrepDataContainerStatus { - FAILED = 0, ADDED, ALREADYCONTAINED - }; + virtual Muon::MMPrepDataContainer* setupMM_PrepDataContainer() const = 0; - virtual SetupMM_PrepDataContainerStatus setupMM_PrepDataContainer(); + const MM_RawDataContainer* getRdoContainer() const; - const MM_RawDataContainer* getRdoContainer(); - - void processRDOContainer( std::vector<IdentifierHash>& idWithDataVect ); + void processRDOContainer( Muon::MMPrepDataContainer* mmPrepDataContainer, + std::vector<IdentifierHash>& idWithDataVect ) const; SG::ReadCondHandleKey<MuonGM::MuonDetectorManager> m_muDetMgrKey {this, "DetectorManagerKey", "MuonDetectorManager", "Key of input MuonDetectorManager condition data"}; ServiceHandle<Muon::IMuonIdHelperSvc> m_idHelperSvc {this, "MuonIdHelperSvc", "Muon::MuonIdHelperSvc/MuonIdHelperSvc"}; - bool m_fullEventDone; - /// MdtPrepRawData containers - Muon::MMPrepDataContainer* m_mmPrepDataContainer; SG::WriteHandleKey<Muon::MMPrepDataContainer> m_mmPrepDataContainerKey; SG::ReadHandleKey<MM_RawDataContainer> m_rdoContainerKey; diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.cxx b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.cxx index 6dacc020873b..b2250fae3fb2 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.cxx +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.cxx @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #include "MmRdoToPrepDataToolMT.h" @@ -21,7 +21,7 @@ StatusCode Muon::MmRdoToPrepDataToolMT::initialize() return StatusCode::SUCCESS; } -Muon::MmRdoToPrepDataToolCore::SetupMM_PrepDataContainerStatus Muon::MmRdoToPrepDataToolMT::setupMM_PrepDataContainer() +Muon::MMPrepDataContainer* Muon::MmRdoToPrepDataToolMT::setupMM_PrepDataContainer() const { // MT version of this method always adds container. Caching will be added later. SG::WriteHandle< Muon::MMPrepDataContainer > handle(m_mmPrepDataContainerKey); @@ -29,8 +29,7 @@ Muon::MmRdoToPrepDataToolCore::SetupMM_PrepDataContainerStatus Muon::MmRdoToPrep if (status.isFailure() || !handle.isValid() ) { ATH_MSG_FATAL("Could not record container of MicroMega PrepData Container at " << m_mmPrepDataContainerKey.key()); - return FAILED; + return nullptr; } - m_mmPrepDataContainer = handle.ptr(); - return ADDED; + return handle.ptr(); } diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.h b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.h index d1190430c9d5..22a09cb1409a 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.h +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/MmRdoToPrepDataToolMT.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration */ #ifndef MUONMmRdoToPrepDataToolMT_H @@ -23,7 +23,7 @@ namespace Muon virtual StatusCode initialize() override; protected: - virtual SetupMM_PrepDataContainerStatus setupMM_PrepDataContainer() override; + virtual Muon::MMPrepDataContainer* setupMM_PrepDataContainer() const override; }; } // end of namespace diff --git a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/components/MuonMM_CnvTools_entries.cxx b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/components/MuonMM_CnvTools_entries.cxx index a5fb8f517add..0422090c5b99 100644 --- a/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/components/MuonMM_CnvTools_entries.cxx +++ b/MuonSpectrometer/MuonCnv/MuonMM_CnvTools/src/components/MuonMM_CnvTools_entries.cxx @@ -1,10 +1,8 @@ #include "../MmRdoToPrepDataTool.h" -#include "../MmRdoToPrepDataToolCore.h" #include "../MmRdoToPrepDataToolMT.h" #include "../MM_RDO_Decoder.h" DECLARE_COMPONENT(Muon::MmRdoToPrepDataTool) -DECLARE_COMPONENT(Muon::MmRdoToPrepDataToolCore) DECLARE_COMPONENT(Muon::MmRdoToPrepDataToolMT) DECLARE_COMPONENT( Muon::MM_RDO_Decoder ) -- GitLab