From bb652036f0a432cdbe2657efe6fd04ee4fc710a3 Mon Sep 17 00:00:00 2001
From: Cedric CAFFY <cedric.caffy@hotmail.fr>
Date: Fri, 1 Feb 2019 03:23:13 -0500
Subject: [PATCH] Removed all the memory leak detected by valgrinding the unit
 tests Refactored OStoreDB::RetrieveMount::batchSucceedRetrieveForRepack()
 method

---
 objectstore/Agent.hpp            |  1 -
 objectstore/AgentWatchdog.cpp    |  2 +-
 objectstore/AlgorithmsTest.cpp   | 10 +++++++---
 objectstore/GarbageCollector.cpp |  8 ++++++++
 objectstore/GarbageCollector.hpp |  2 +-
 scheduler/SchedulerTest.cpp      |  4 ++--
 6 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/objectstore/Agent.hpp b/objectstore/Agent.hpp
index aaf5111294..daff010e46 100644
--- a/objectstore/Agent.hpp
+++ b/objectstore/Agent.hpp
@@ -45,7 +45,6 @@ class Agent: public ObjectOps<serializers::Agent, serializers::Agent_t> {
 public:
   CTA_GENERATE_EXCEPTION_CLASS(AgentStillOwnsObjects);
   Agent(GenericObject & go);
-  
   Agent(const std::string & name, Backend & os);
 
   void initialize();
diff --git a/objectstore/AgentWatchdog.cpp b/objectstore/AgentWatchdog.cpp
index 85d9b288b0..7a03ccb610 100644
--- a/objectstore/AgentWatchdog.cpp
+++ b/objectstore/AgentWatchdog.cpp
@@ -16,4 +16,4 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "AgentWatchdog.hpp"
\ No newline at end of file
+#include "AgentWatchdog.hpp"
diff --git a/objectstore/AlgorithmsTest.cpp b/objectstore/AlgorithmsTest.cpp
index 208c57e00f..c033acb24a 100644
--- a/objectstore/AlgorithmsTest.cpp
+++ b/objectstore/AlgorithmsTest.cpp
@@ -36,6 +36,7 @@ namespace unitTests {
 
 void fillRetrieveRequests(
   typename cta::objectstore::ContainerAlgorithms<cta::objectstore::RetrieveQueue,cta::objectstore::RetrieveQueueToTransfer>::InsertedElement::list &requests,
+  std::list<std::unique_ptr<cta::objectstore::RetrieveRequest> >& requestPtrs, //List to avoid memory leak on ArchiveQueueAlgorithms test
   cta::objectstore::BackendVFS &be,
   cta::objectstore::AgentReference &agentRef)
 {
@@ -71,8 +72,9 @@ void fillRetrieveRequests(
     rqc.mountPolicy.maxDrivesAllowed = 1;
     rqc.mountPolicy.retrieveMinRequestAge = 1;
     rqc.mountPolicy.retrievePriority = 1;
+    requestPtrs.emplace_back(new cta::objectstore::RetrieveRequest(rrAddr, be));
     requests.emplace_back(ContainerAlgorithms<RetrieveQueue,RetrieveQueueToTransfer>::InsertedElement{
-      new RetrieveRequest(rrAddr, be), 1, i, 667, mp, serializers::RetrieveJobStatus::RJS_ToTransfer
+      requestPtrs.back().get(), 1, i, 667, mp, serializers::RetrieveJobStatus::RJS_ToTransfer
     });
     auto &rr = *requests.back().retrieveRequest;
     rr.initialize();
@@ -185,8 +187,9 @@ TEST(ObjectStore, RetrieveQueueAlgorithms) {
   rel.release();
   agent.initialize();
   agent.insertAndRegisterSelf(lc);
+  std::list<std::unique_ptr<RetrieveRequest> > requestsPtrs;
   ContainerAlgorithms<RetrieveQueue,RetrieveQueueToTransfer>::InsertedElement::list requests;
-  fillRetrieveRequests(requests, be, agentRef);
+  fillRetrieveRequests(requests, requestsPtrs, be, agentRef); //memory leak here
 
   {
     // Second agent to test referenceAndSwitchOwnershipIfNecessary
@@ -205,7 +208,8 @@ TEST(ObjectStore, RetrieveQueueAlgorithms) {
     agent2.initialize();
     agent2.insertAndRegisterSelf(lc);
     ContainerAlgorithms<RetrieveQueue,RetrieveQueueToTransfer>::InsertedElement::list requests2;
-    fillRetrieveRequests(requests2, be2, agentRef2);
+    std::list<std::unique_ptr<RetrieveRequest> > requestsPtrs2;
+    fillRetrieveRequests(requests2, requestsPtrs2,be2, agentRef2);
 
     auto a1 = agentRef2.getAgentAddress();
     auto a2 = agentRef2.getAgentAddress();
diff --git a/objectstore/GarbageCollector.cpp b/objectstore/GarbageCollector.cpp
index 5a50a912c3..f664d45712 100644
--- a/objectstore/GarbageCollector.cpp
+++ b/objectstore/GarbageCollector.cpp
@@ -40,6 +40,14 @@ GarbageCollector::GarbageCollector(Backend & os, AgentReference & agentReference
   m_agentRegister.fetch();
 }
 
+GarbageCollector::~GarbageCollector(){
+  //Normally, the Garbage collector is never destroyed in production
+  //this destructor is here to avoid memory leaks on unit tests
+  for(auto &kv : m_watchedAgents){
+    delete kv.second;
+  }
+}
+
 void GarbageCollector::runOnePass(log::LogContext & lc) {
   trimGoneTargets(lc);
   acquireTargets(lc);
diff --git a/objectstore/GarbageCollector.hpp b/objectstore/GarbageCollector.hpp
index fd263baaa8..a7a55c6099 100644
--- a/objectstore/GarbageCollector.hpp
+++ b/objectstore/GarbageCollector.hpp
@@ -41,7 +41,7 @@ class RetrieveRequest;
 class GarbageCollector {
 public:
   GarbageCollector(Backend & os, AgentReference & agentReference, catalogue::Catalogue & catalogue);
-  
+  ~GarbageCollector();
   void runOnePass(log::LogContext & lc);
   
   void acquireTargets(log::LogContext & lc);
diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp
index 893dce33da..2364e93273 100644
--- a/scheduler/SchedulerTest.cpp
+++ b/scheduler/SchedulerTest.cpp
@@ -720,7 +720,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_failure) {
     scheduler.queueRetrieve("disk_instance", request, lc);
     scheduler.waitSchedulerDbSubthreadsComplete();
   }
-
+  
   // Try mounting the tape twice
   for(int mountPass = 0; mountPass < 2; ++mountPass)
   {
@@ -800,7 +800,7 @@ TEST_P(SchedulerTest, archive_and_retrieve_failure) {
       getSchedulerDB().replaceAgent(new objectstore::AgentReference("OStoreDBFactory2", dl));
     } // end of retries
   } // end of pass
-
+  
   {
     // We expect the retrieve queue to be empty
     auto rqsts = scheduler.getPendingRetrieveJobs(lc);
-- 
GitLab