Make the ROOTFrameData properly cleanup after itself

AIDASoft · Oct 11, 2023 · e0c71cf · e0c71cf
1 parent acc4742
commit e0c71cf
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 15 deletions.
diff --git a/include/podio/CollectionBuffers.h b/include/podio/CollectionBuffers.h
@@ -53,15 +53,19 @@ struct CollectionReadBuffers {
   using CreateFuncT = std::function<std::unique_ptr<podio::CollectionBase>(podio::CollectionReadBuffers, bool)>;
   using RecastFuncT = std::function<void(CollectionReadBuffers&)>;
 
+  using DeleteFuncT = std::function<void(CollectionReadBuffers&)>;
+
   CollectionReadBuffers(void* d, CollRefCollection* ref, VectorMembersInfo* vec, SchemaVersionT version,
-                        std::string_view typ, CreateFuncT&& createFunc, RecastFuncT&& recastFunc) :
+                        std::string_view typ, CreateFuncT&& createFunc, RecastFuncT&& recastFunc,
+                        DeleteFuncT&& deleteFunc) :
       data(d),
       references(ref),
       vectorMembers(vec),
       schemaVersion(version),
       type(typ),
       createCollection(std::move(createFunc)),
-      recast(std::move(recastFunc)) {
+      recast(std::move(recastFunc)),
+      deleteBuffers(std::move(deleteFunc)) {
   }
 
   CollectionReadBuffers() = default;
@@ -105,6 +109,11 @@ struct CollectionReadBuffers {
   // generation) to do the second cast and assign the result of that to the data
   // field again.
   RecastFuncT recast{};
+
+  // Workaround for https://github.com/AIDASoft/podio#500
+  // We need a function that explicitly deletes the buffers, but for this we
+  // need type information, so we attach a delete function at generation time
+  DeleteFuncT deleteBuffers{};
 };
 
 } // namespace podio

diff --git a/include/podio/ROOTFrameData.h b/include/podio/ROOTFrameData.h
@@ -19,7 +19,7 @@ class ROOTFrameData {
   using BufferMap = std::unordered_map<std::string, podio::CollectionReadBuffers>;
 
   ROOTFrameData() = delete;
-  ~ROOTFrameData() = default;
+  ~ROOTFrameData();
   ROOTFrameData(ROOTFrameData&&) = default;
   ROOTFrameData& operator=(ROOTFrameData&&) = default;
   ROOTFrameData(const ROOTFrameData&) = delete;
@@ -65,6 +65,14 @@ class ROOTFrameData {
   CollIDPtr m_idTable{nullptr};
   podio::GenericParameters m_parameters{};
 };
+
+// Interim workaround for https://github.com/AIDASoft/podio#500
+inline ROOTFrameData::~ROOTFrameData() {
+  for (auto& [_, buffer] : m_buffers) {
+    buffer.deleteBuffers(buffer);
+  }
+}
+
 } // namespace podio
 
 #endif // PODIO_ROOTFRAMEDATA_H
diff --git a/python/templates/macros/collections.jinja2 b/python/templates/macros/collections.jinja2
@@ -203,6 +203,21 @@ podio::CollectionReadBuffers createBuffersV{{ schemaVersion }}(bool isSubset) {
     }
   };
 
+  readBuffers.deleteBuffers = [](podio::CollectionReadBuffers& buffers) {
+    if (buffers.data) {
+      // If we have data then we are not a subset collection and we have to
+      // clean up all type erased buffers by casting them back to something that
+      // we can delete
+      delete static_cast<{{ class.full_type }}DataContainer*>(buffers.data);
+{% for member in VectorMembers %}
+      delete static_cast<std::vector<{{ member.full_type }}>*>((*buffers.vectorMembers)[0].second);
+{% endfor %}
+
+    }
+    delete buffers.references;
+    delete buffers.vectorMembers;
+  };
+
   return readBuffers;
 }
 

diff --git a/src/UserDataCollection.cc b/src/UserDataCollection.cc
@@ -20,18 +20,19 @@ namespace {
     // Register with schema version 1 to allow for potential changes
     CollectionBufferFactory::mutInstance().registerCreationFunc(
         userDataCollTypeName<T>(), UserDataCollection<T>::schemaVersion, [](bool) {
-          return podio::CollectionReadBuffers{new std::vector<T>(),
-                                              nullptr,
-                                              nullptr,
-                                              podio::UserDataCollection<T>::schemaVersion,
-                                              podio::userDataCollTypeName<T>(),
-                                              [](podio::CollectionReadBuffers buffers, bool) {
-                                                return std::make_unique<UserDataCollection<T>>(
-                                                    std::move(*buffers.dataAsVector<T>()));
-                                              },
-                                              [](podio::CollectionReadBuffers& buffers) {
-                                                buffers.data = podio::CollectionWriteBuffers::asVector<T>(buffers.data);
-                                              }};
+          return podio::CollectionReadBuffers{
+              new std::vector<T>(),
+              nullptr,
+              nullptr,
+              podio::UserDataCollection<T>::schemaVersion,
+              podio::userDataCollTypeName<T>(),
+              [](podio::CollectionReadBuffers buffers, bool) {
+                return std::make_unique<UserDataCollection<T>>(std::move(*buffers.dataAsVector<T>()));
+              },
+              [](podio::CollectionReadBuffers& buffers) {
+                buffers.data = podio::CollectionWriteBuffers::asVector<T>(buffers.data);
+              },
+              [](podio::CollectionReadBuffers& buffers) { delete static_cast<std::vector<T>*>(buffers.data); }};
         });
 
     // For now passing the same schema version for from and current versions