From a9bc5a7fd390b735e2aba62466d36c7c82928b47 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 12 Jun 2024 21:09:11 +0200 Subject: [PATCH] Fix potential race condition for writing GenericParameters --- include/podio/GenericParameters.h | 14 ++++++++++---- include/podio/utilities/RootHelpers.h | 3 ++- src/RNTupleWriter.cc | 3 +-- src/ROOTWriter.cc | 8 ++++---- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index f0f2b164a..4f41aad63 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -118,7 +118,7 @@ class GenericParameters { /// Get all the available values for a given type template > - std::vector> getValues() const; + std::tuple, std::vector>> getKeysAndValues() const; /// erase all elements void clear() { @@ -263,8 +263,9 @@ std::vector GenericParameters::getKeys() const { } template -std::vector> GenericParameters::getValues() const { +std::tuple, std::vector>> GenericParameters::getKeysAndValues() const { std::vector> values; + std::vector keys; auto& mtx = getMutex(); const auto& map = getMap(); { @@ -272,9 +273,14 @@ std::vector> GenericParameters::getValues() const { // values std::lock_guard lock{mtx}; values.reserve(map.size()); - std::transform(map.begin(), map.end(), std::back_inserter(values), [](const auto& pair) { return pair.second; }); + keys.reserve(map.size()); + + for (const auto& [k, v] : map) { + keys.emplace_back(k); + values.emplace_back(v); + } } - return values; + return {keys, values}; } template typename VecLike> diff --git a/include/podio/utilities/RootHelpers.h b/include/podio/utilities/RootHelpers.h index 884382b6e..360ef6674 100644 --- a/include/podio/utilities/RootHelpers.h +++ b/include/podio/utilities/RootHelpers.h @@ -57,7 +57,8 @@ namespace root_utils { ParamStorage(ParamStorage&&) = default; ParamStorage& operator=(ParamStorage&&) = default; - ParamStorage(const std::vector& ks, const std::vector>& vs) : keys(ks), values(vs) { + ParamStorage(std::tuple, std::vector>> keysValues) : + keys(std::move(std::get<0>(keysValues))), values(std::move(std::get<1>(keysValues))) { } /// Get a pointer to the stored keys for binding it to a TBranch diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 55d23167f..bbff2a392 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -47,8 +47,7 @@ void RNTupleWriter::fillParams(const GenericParameters& params, CategoryInfo& ca auto& paramStorage = getParamStorage(catInfo); auto& keys = paramStorage.keys; auto& values = paramStorage.values; - keys = params.getKeys(); - values = params.getValues(); + std::tie(keys, values) = params.getKeysAndValues(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) entry->BindRawPtr(root_utils::getGPKeyName(), &keys); entry->BindRawPtr(root_utils::getGPValueName(), &values); diff --git a/src/ROOTWriter.cc b/src/ROOTWriter.cc index df8e810fd..f0aab0acf 100644 --- a/src/ROOTWriter.cc +++ b/src/ROOTWriter.cc @@ -196,10 +196,10 @@ ROOTWriter::checkConsistency(const std::vector& collsToWrite, const } void ROOTWriter::fillParams(CategoryInfo& catInfo, const GenericParameters& params) { - catInfo.intParams = {params.getKeys(), params.getValues()}; - catInfo.floatParams = {params.getKeys(), params.getValues()}; - catInfo.doubleParams = {params.getKeys(), params.getValues()}; - catInfo.stringParams = {params.getKeys(), params.getValues()}; + catInfo.intParams = params.getKeysAndValues(); + catInfo.floatParams = params.getKeysAndValues(); + catInfo.doubleParams = params.getKeysAndValues(); + catInfo.stringParams = params.getKeysAndValues(); } } // namespace podio