From 1d877f7578dd8438542af84e556c11892a2691cd Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 10 Oct 2023 17:17:42 +0200 Subject: [PATCH] Make the ROOTFrameData properly cleanup after itself --- include/podio/CollectionBuffers.h | 13 +++++++++-- include/podio/ROOTFrameData.h | 10 ++++++++- python/templates/macros/collections.jinja2 | 15 +++++++++++++ src/UserDataCollection.cc | 25 +++++++++++----------- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/include/podio/CollectionBuffers.h b/include/podio/CollectionBuffers.h index b51161c2d..834c465c9 100644 --- a/include/podio/CollectionBuffers.h +++ b/include/podio/CollectionBuffers.h @@ -53,15 +53,19 @@ struct CollectionReadBuffers { using CreateFuncT = std::function(podio::CollectionReadBuffers, bool)>; using RecastFuncT = std::function; + using DeleteFuncT = std::function; + 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 index 157e3fbea..3cc63d04a 100644 --- a/include/podio/ROOTFrameData.h +++ b/include/podio/ROOTFrameData.h @@ -19,7 +19,7 @@ class ROOTFrameData { using BufferMap = std::unordered_map; 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 index efc7c24cb..cda1cf948 100644 --- 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*>((*buffers.vectorMembers)[{{ loop.index0 }}].second); +{% endfor %} + + } + delete buffers.references; + delete buffers.vectorMembers; + }; + return readBuffers; } diff --git a/src/UserDataCollection.cc b/src/UserDataCollection.cc index c965f5b9c..673e7969c 100644 --- 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(), UserDataCollection::schemaVersion, [](bool) { - return podio::CollectionReadBuffers{new std::vector(), - nullptr, - nullptr, - podio::UserDataCollection::schemaVersion, - podio::userDataCollTypeName(), - [](podio::CollectionReadBuffers buffers, bool) { - return std::make_unique>( - std::move(*buffers.dataAsVector())); - }, - [](podio::CollectionReadBuffers& buffers) { - buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); - }}; + return podio::CollectionReadBuffers{ + new std::vector(), + nullptr, + nullptr, + podio::UserDataCollection::schemaVersion, + podio::userDataCollTypeName(), + [](podio::CollectionReadBuffers buffers, bool) { + return std::make_unique>(std::move(*buffers.dataAsVector())); + }, + [](podio::CollectionReadBuffers& buffers) { + buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + }, + [](podio::CollectionReadBuffers& buffers) { delete static_cast*>(buffers.data); }}; }); // For now passing the same schema version for from and current versions