Fix potential race condition for writing GenericParameters

AIDASoft · Jun 12, 2024 · a9bc5a7 · a9bc5a7
1 parent b4e581f
commit a9bc5a7
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 11 deletions.
diff --git 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 <typename T, typename = EnableIfValidGenericDataType<T>>
-  std::vector<std::vector<T>> getValues() const;
+  std::tuple<std::vector<std::string>, std::vector<std::vector<T>>> getKeysAndValues() const;
 
   /// erase all elements
   void clear() {
@@ -263,18 +263,24 @@ std::vector<std::string> GenericParameters::getKeys() const {
 }
 
 template <typename T, typename>
-std::vector<std::vector<T>> GenericParameters::getValues() const {
+std::tuple<std::vector<std::string>, std::vector<std::vector<T>>> GenericParameters::getKeysAndValues() const {
   std::vector<std::vector<T>> values;
+  std::vector<std::string> keys;
   auto& mtx = getMutex<T>();
   const auto& map = getMap<T>();
   {
     // Lock to avoid concurrent changes to the map while we get the stored
     // 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 T, template <typename...> typename VecLike>

diff --git 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<std::string>& ks, const std::vector<std::vector<T>>& vs) : keys(ks), values(vs) {
+    ParamStorage(std::tuple<std::vector<std::string>, std::vector<std::vector<T>>> 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
@@ -47,8 +47,7 @@ void RNTupleWriter::fillParams(const GenericParameters& params, CategoryInfo& ca
   auto& paramStorage = getParamStorage<T>(catInfo);
   auto& keys = paramStorage.keys;
   auto& values = paramStorage.values;
-  keys = params.getKeys<T>();
-  values = params.getValues<T>();
+  std::tie(keys, values) = params.getKeysAndValues<T>();
 #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0)
   entry->BindRawPtr(root_utils::getGPKeyName<T>(), &keys);
   entry->BindRawPtr(root_utils::getGPValueName<T>(), &values);

diff --git a/src/ROOTWriter.cc b/src/ROOTWriter.cc
@@ -196,10 +196,10 @@ ROOTWriter::checkConsistency(const std::vector<std::string>& collsToWrite, const
 }
 
 void ROOTWriter::fillParams(CategoryInfo& catInfo, const GenericParameters& params) {
-  catInfo.intParams = {params.getKeys<int>(), params.getValues<int>()};
-  catInfo.floatParams = {params.getKeys<float>(), params.getValues<float>()};
-  catInfo.doubleParams = {params.getKeys<double>(), params.getValues<double>()};
-  catInfo.stringParams = {params.getKeys<std::string>(), params.getValues<std::string>()};
+  catInfo.intParams = params.getKeysAndValues<int>();
+  catInfo.floatParams = params.getKeysAndValues<float>();
+  catInfo.doubleParams = params.getKeysAndValues<double>();
+  catInfo.stringParams = params.getKeysAndValues<std::string>();
 }
 
 } // namespace podio