From 11b30f017950083f3860fc77bf165e6f0b331711 Mon Sep 17 00:00:00 2001
From: jmcarcell <jmcarcell@users.noreply.github.com>
Date: Fri, 11 Oct 2024 18:35:49 +0200
Subject: [PATCH] Fix a few more leaks

---
 k4FWCore/components/IOSvc.cpp  |  7 +++++--
 k4FWCore/components/IOSvc.h    |  3 +--
 k4FWCore/components/Writer.cpp | 16 +++++++++-------
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/k4FWCore/components/IOSvc.cpp b/k4FWCore/components/IOSvc.cpp
index b8f6d217..da1e725f 100644
--- a/k4FWCore/components/IOSvc.cpp
+++ b/k4FWCore/components/IOSvc.cpp
@@ -112,7 +112,7 @@ StatusCode IOSvc::finalize() { return Service::finalize(); }
 std::tuple<std::vector<podio::CollectionBase*>, std::vector<std::string>, podio::Frame> IOSvc::next() {
   podio::Frame frame;
   {
-    std::scoped_lock<std::mutex> lock(m_changeBufferLock);
+    std::lock_guard<std::mutex> lock(m_changeBufferLock);
     if (m_nextEntry < m_entries) {
       frame = podio::Frame(m_reader->readEvent(m_nextEntry));
     } else {
@@ -177,7 +177,10 @@ void IOSvc::handle(const Incident& incident) {
     code = m_dataSvc->retrieveObject("/Event/" + coll, collPtr);
     if (code.isSuccess()) {
       debug() << "Removing the collection: " << coll << " from the store" << endmsg;
-      code = m_dataSvc->unregisterObject(collPtr);
+      code          = m_dataSvc->unregisterObject(collPtr);
+      auto storePtr = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(collPtr);
+      storePtr->getData().release();
+      delete storePtr;
     } else {
       error() << "Expected collection " << coll << " in the store but it was not found" << endmsg;
     }
diff --git a/k4FWCore/components/IOSvc.h b/k4FWCore/components/IOSvc.h
index c34e42a2..d392352c 100644
--- a/k4FWCore/components/IOSvc.h
+++ b/k4FWCore/components/IOSvc.h
@@ -43,8 +43,7 @@ class IOSvc : public extends<Service, IIOSvc, IIncidentListener> {
   StatusCode initialize() override;
   StatusCode finalize() override;
 
-  std::tuple<std::vector<podio::CollectionBase*>, std::vector<std::string>, podio::Frame> next()
-      override;
+  std::tuple<std::vector<podio::CollectionBase*>, std::vector<std::string>, podio::Frame> next() override;
 
   std::shared_ptr<std::vector<std::string>> getCollectionNames() const override {
     return std::make_shared<std::vector<std::string>>(m_collectionNames);
diff --git a/k4FWCore/components/Writer.cpp b/k4FWCore/components/Writer.cpp
index 83b6d57e..1e276c96 100644
--- a/k4FWCore/components/Writer.cpp
+++ b/k4FWCore/components/Writer.cpp
@@ -23,7 +23,6 @@
 #include "GaudiKernel/IDataProviderSvc.h"
 #include "GaudiKernel/SmartDataPtr.h"
 #include "GaudiKernel/StatusCode.h"
-
 #include "podio/Frame.h"
 
 #include "IIOSvc.h"
@@ -189,7 +188,7 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
     StatusCode  code = m_dataSvc->retrieveObject("/Event" + k4FWCore::frameLocation, p);
     std::unique_ptr<AnyDataWrapper<podio::Frame>> ptr;
     // This is the case when we are reading from a file
-    // Since we unregistered the object, we need to delete it
+    // Putting it into a unique_ptr will make sure it's deleted
     if (code.isSuccess()) {
       auto sc = m_dataSvc->unregisterObject(p);
       if (!sc.isSuccess()) {
@@ -233,6 +232,10 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
         error() << "Failed to unregister collection " << coll << endmsg;
         return;
       }
+      // We still have to delete the AnyDataWrapper to avoid a leak
+      auto storePtr = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(storeCollection);
+      storePtr->getData().release();
+      delete storePtr;
     }
 
     for (auto& coll : m_collectionsToAdd) {
@@ -247,7 +250,10 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
         return;
       }
       const auto collection = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(storeCollection);
-      if (!collection) {
+      if (collection) {
+        ptr->getData().put(std::move(collection->getData()), coll);
+        delete collection;
+      } else {
         // Check the case when the data has been produced using the old DataHandle
         const auto old_collection = dynamic_cast<DataWrapperBase*>(storeCollection);
         if (!old_collection) {
@@ -258,10 +264,6 @@ class Writer final : public Gaudi::Functional::Consumer<void(const EventContext&
               const_cast<podio::CollectionBase*>(old_collection->collectionBase()));
           ptr->getData().put(std::move(uptr), coll);
         }
-
-      } else {
-        std::unique_ptr<podio::CollectionBase> uptr(std::move(collection->getData()));
-        ptr->getData().put(std::move(uptr), coll);
       }
     }