From d02a2f20fa656e4d1683a4c7fb4bef869abd9360 Mon Sep 17 00:00:00 2001
From: Frank Winklmeier <frank.winklmeier@cern.ch>
Date: Tue, 28 May 2019 15:44:43 +0200
Subject: [PATCH] TrigServices: Enable static thread checker
Mark a few methods as non-thread safe. `TrigCOOLUpdateHelper` cannot use
the recommended way to access conditions via a `ReadCondHandle` as the
information needs to be read before the first event. However, the COOL
folder (/TRIGGER/HLT/COOLUPDATE) never changes during the run and thus
it is safe to use the old-style `DataHandle` access to the conditions
object.
Also apply some general cleanup to the code using C++17 features.
---
.../TrigServices/ATLAS_CHECK_THREAD_SAFETY | 1 +
.../TrigServices/src/HltEventLoopMgr.cxx | 2 +-
.../TrigServices/src/HltEventLoopMgr.h | 3 +-
.../TrigServices/src/TrigCOOLUpdateHelper.cxx | 51 +++++++++----------
.../TrigServices/src/TrigCOOLUpdateHelper.h | 22 ++++----
5 files changed, 39 insertions(+), 40 deletions(-)
create mode 100644 HLT/Trigger/TrigControl/TrigServices/ATLAS_CHECK_THREAD_SAFETY
diff --git a/HLT/Trigger/TrigControl/TrigServices/ATLAS_CHECK_THREAD_SAFETY b/HLT/Trigger/TrigControl/TrigServices/ATLAS_CHECK_THREAD_SAFETY
new file mode 100644
index 000000000000..4919d0b200a1
--- /dev/null
+++ b/HLT/Trigger/TrigControl/TrigServices/ATLAS_CHECK_THREAD_SAFETY
@@ -0,0 +1 @@
+HLT/Trigger/TrigControl/TrigServices
diff --git a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
index 9db5388955ea..78545cd5dba7 100644
--- a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
+++ b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
@@ -389,7 +389,7 @@ StatusCode HltEventLoopMgr::prepareForRun(const ptree& pt)
ATH_CHECK(ita->sysBeginRun()); // TEMPORARY: beginRun is deprecated
}
// Initialize COOL helper (needs to be done after IOVDbSvc has loaded all folders)
- m_coolHelper->readFolderInfo();
+ ATH_CHECK(m_coolHelper->readFolderInfo());
// close any open files (e.g. THistSvc)
ATH_CHECK(m_ioCompMgr->io_finalize());
diff --git a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.h b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.h
index e3553adc11ad..a5c0a0b7386b 100644
--- a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.h
+++ b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.h
@@ -15,6 +15,7 @@
#include "AthenaBaseComps/AthService.h"
#include "AthenaKernel/EventContextClid.h"
#include "AthenaKernel/Timeout.h"
+#include "CxxUtils/checker_macros.h"
#include "EventInfo/EventInfo.h"
#include "EventInfo/EventID.h" // number_type
#include "StoreGate/ReadHandleKey.h"
@@ -83,7 +84,7 @@ public:
/// @name State transitions of ITrigEventLoopMgr interface
///@{
- virtual StatusCode prepareForRun(const boost::property_tree::ptree& pt);
+ virtual StatusCode prepareForRun ATLAS_NOT_THREAD_SAFE (const boost::property_tree::ptree& pt);
virtual StatusCode hltUpdateAfterFork(const boost::property_tree::ptree& pt);
///@}
diff --git a/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.cxx b/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.cxx
index 5a82176af15c..60f483a12806 100644
--- a/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.cxx
+++ b/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.cxx
@@ -17,6 +17,7 @@
#include "AthenaKernel/IOVRange.h"
#include "AthenaPoolUtilities/CondAttrListCollection.h"
#include "CoralBase/AttributeListException.h"
+#include "StoreGate/DataHandle.h"
// TDAQ includes
#include "CTPfragment/CTPfragment.h"
@@ -33,8 +34,6 @@ TrigCOOLUpdateHelper::TrigCOOLUpdateHelper(const std::string &type,
m_robDataProviderSvc("ROBDataProviderSvc", name)
{}
-TrigCOOLUpdateHelper::~TrigCOOLUpdateHelper()
-{}
StatusCode TrigCOOLUpdateHelper::initialize()
{
@@ -42,11 +41,6 @@ StatusCode TrigCOOLUpdateHelper::initialize()
service("IOVSvc", m_iovSvc, /*createIf=*/false).ignore();
service("IOVDbSvc", m_iovDbSvc, /*createIf=*/false).ignore();
- // Register DataHandle with COOLUPDATE folder
- if (!m_coolFolderName.empty()) {
- ATH_CHECK( detStore()->regHandle(m_folderMapHandle, m_coolFolderName) );
- }
-
if (!m_monTool.empty()) ATH_CHECK(m_monTool.retrieve());
return StatusCode::SUCCESS;
@@ -77,9 +71,9 @@ StatusCode TrigCOOLUpdateHelper::stop()
// Read COOL folder info
// Loop over all registered keys and get folder name and CLID
//=========================================================================
-void TrigCOOLUpdateHelper::readFolderInfo()
+StatusCode TrigCOOLUpdateHelper::readFolderInfo()
{
- if (m_iovDbSvc==0) return;
+ if (m_iovDbSvc==nullptr) return StatusCode::SUCCESS;
m_folderInfo.clear();
// Loop over all keys registered with IOVDbSvc
@@ -99,18 +93,23 @@ void TrigCOOLUpdateHelper::readFolderInfo()
CLID clid = detStore()->clid(key);
if (clid!=CLID_NULL)
- m_folderInfo.insert(FolderInfoMap::value_type(foldername,FolderInfo(clid, key)));
+ m_folderInfo.insert({foldername, FolderInfo(clid, key)});
else
ATH_MSG_ERROR("Cannot find CLID for " << key);
}
if (!m_coolFolderName.empty()) {
+ // Read COOL folder map
+ const DataHandle<CondAttrListCollection> folderMapHandle;
+ ATH_CHECK( detStore()->regHandle(folderMapHandle, m_coolFolderName) );
+
ATH_MSG_INFO("COOL folder map in " << m_coolFolderName << ":");
- for (CondAttrListCollection::const_iterator c = m_folderMapHandle->begin();
- c != m_folderMapHandle->end(); ++c) {
- ATH_MSG_INFO(" (" << c->first << ") " << c->second["FolderName"].data<std::string>());
+ for (auto const& [idx, attr] : *folderMapHandle) {
+ m_folderNames[idx] = attr["FolderName"].data<std::string>();
+ ATH_MSG_INFO(" (" << idx << ") " << m_folderNames[idx]);
}
}
+ return StatusCode::SUCCESS;
}
StatusCode TrigCOOLUpdateHelper::resetFolders(const std::vector<std::string>& folders,
@@ -134,13 +133,13 @@ StatusCode TrigCOOLUpdateHelper::resetFolder(const std::string& folder,
{
// Force a reset of folders by setting an IOVRange in the past
- if (m_iovSvc==0 || m_iovDbSvc==0) return StatusCode::SUCCESS;
+ if (m_iovSvc==nullptr || m_iovDbSvc==nullptr) return StatusCode::SUCCESS;
// Set IOV in the past to trigger reload in IOVSvc
IOVRange iov_range(IOVTime(currentRun-2, 0),
IOVTime(currentRun-1, 0));
- FolderInfoMap::const_iterator f = m_folderInfo.find(folder);
+ const auto& f = m_folderInfo.find(folder);
if ( f==m_folderInfo.end() ) {
ATH_MSG_DEBUG("Folder " << folder << " not registered with IOVDbSvc");
return StatusCode::SUCCESS;
@@ -216,7 +215,7 @@ StatusCode TrigCOOLUpdateHelper::hltCoolUpdate(const EventContext& ctx)
StatusCode TrigCOOLUpdateHelper::hltCoolUpdate(const std::string& folderName, const EventContext& ctx)
{
- if ( m_iovSvc==0 || m_coolFolderName.empty() ) {
+ if ( m_iovSvc==nullptr || m_coolFolderName.empty() ) {
ATH_MSG_DEBUG("Passive mode. Not updating COOL folders");
return StatusCode::SUCCESS;
}
@@ -232,23 +231,21 @@ StatusCode TrigCOOLUpdateHelper::hltCoolUpdate(const std::string& folderName, co
return StatusCode::SUCCESS;
}
-StatusCode TrigCOOLUpdateHelper::getFolderName(CTPfragment::FolderIndex idx, std::string& folderName)
+StatusCode TrigCOOLUpdateHelper::getFolderName(CTPfragment::FolderIndex idx, std::string& folderName) const
{
if (m_coolFolderName.empty()) return StatusCode::SUCCESS;
-
- if (!m_folderMapHandle.isInitialized()) {
- ATH_MSG_ERROR("DataHandle for '" << m_coolFolderName << "' not initialized.");
- return StatusCode::FAILURE;
- }
- try {
- folderName = m_folderMapHandle->attributeList(idx)["FolderName"].data<std::string>();
- }
- catch (coral::AttributeListException &) {
+
+ const auto& itr = m_folderNames.find(idx);
+ if (itr==m_folderNames.end()) {
ATH_MSG_ERROR(m_coolFolderName << " does not contain a folder for index/channel " << idx
<< ". Existing folders are:");
- m_folderMapHandle->dump();
+ for (auto const& [idx, name] : m_folderNames) {
+ ATH_MSG_INFO(" (" << idx << ") " << name);
+ }
return StatusCode::FAILURE;
}
+
+ folderName = itr->second;
return StatusCode::SUCCESS;
}
diff --git a/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.h b/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.h
index 08b9280b29db..3cb066e27e8b 100644
--- a/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.h
+++ b/HLT/Trigger/TrigControl/TrigServices/src/TrigCOOLUpdateHelper.h
@@ -21,17 +21,15 @@
#include "AthenaBaseComps/AthAlgTool.h"
#include "AthenaMonitoring/Monitored.h"
+#include "CxxUtils/checker_macros.h"
#include "EventInfo/EventID.h"
-#include "StoreGate/DataHandle.h"
#include "ByteStreamCnvSvcBase/IROBDataProviderSvc.h"
// TDAQ includes
#include "CTPfragment/CTPExtraWordsFormat.h"
-class TH1F;
class IIOVSvc;
class IIOVDbSvc;
-class CondAttrListCollection;
/**
* Struct to hold CLID <-> folder name mapping
@@ -64,7 +62,6 @@ struct FolderUpdate {
class TrigCOOLUpdateHelper : public AthAlgTool {
public:
TrigCOOLUpdateHelper(const std::string& type, const std::string& name, const IInterface* parent);
- virtual ~TrigCOOLUpdateHelper();
virtual StatusCode initialize() override;
virtual StatusCode start() override;
@@ -88,7 +85,7 @@ public:
* @param idx Folder index
* @param folderName Returns folder name
*/
- StatusCode getFolderName(CTPfragment::FolderIndex idx, std::string& folderName);
+ StatusCode getFolderName(CTPfragment::FolderIndex idx, std::string& folderName) const;
/**
* @brief Schedule COOL folder updates according to extra payload in CTP fragment
@@ -98,7 +95,7 @@ public:
/**
* @brief Read information about existing COOL folders
*/
- void readFolderInfo();
+ StatusCode readFolderInfo ATLAS_NOT_THREAD_SAFE ();
private:
/**
@@ -119,15 +116,18 @@ private:
StatusCode resetFolders(const std::vector<std::string>& folders, EventID::number_type currentRun,
bool dropObject = false);
- typedef std::map<std::string, FolderInfo> FolderInfoMap;
- FolderInfoMap m_folderInfo; //!< COOL folder info
+ /// CLID/name mapping of COOL folders
+ std::map<std::string, FolderInfo> m_folderInfo;
- const DataHandle<CondAttrListCollection> m_folderMapHandle;
+ /// Map to store scheduled/done COOL folder updates
std::map<CTPfragment::FolderIndex, FolderUpdate> m_folderUpdates;
+ /// Map to store the folder update index -> name mapping
+ std::map<CTPfragment::FolderIndex, std::string> m_folderNames;
+
// Services and Tools
- IIOVSvc* m_iovSvc{0};
- IIOVDbSvc* m_iovDbSvc{0};
+ IIOVSvc* m_iovSvc{nullptr};
+ IIOVDbSvc* m_iovDbSvc{nullptr};
ServiceHandle<IROBDataProviderSvc> m_robDataProviderSvc;
// Properties
--
GitLab