Fix memory leak on ResourceCache Module

- check memory leaks through Valgrind tool, and fix them

Change-Id: I58fb3356059a55313008fd04519779c35715b421
Signed-off-by: YounghyunJoo <yh_.joo@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/15177
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>

diff --git a/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h b/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h
index ae02cac..0fc84c4 100644 (file)
--- a/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h
+++ b/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h
@@ -68,6 +68,8 @@ namespace OIC
                 // throw InvalidParameterException;
                 bool isCachedData(CacheID id) const;
 
+                static void stopResourceCacheManager();
+
             private:
                 static ResourceCacheManager *s_instance;
                 static std::mutex s_mutex;

diff --git a/service/resource-encapsulation/src/resourceCache/src/DataCache.cpp b/service/resource-encapsulation/src/resourceCache/src/DataCache.cpp
index e774d68..f9ed998 100644 (file)
--- a/service/resource-encapsulation/src/resourceCache/src/DataCache.cpp
+++ b/service/resource-encapsulation/src/resourceCache/src/DataCache.cpp
@@ -99,7 +99,7 @@ namespace OIC
             if (subscriberList != nullptr)
             {
                 subscriberList->clear();
-                subscriberList.release();
+                subscriberList.reset();
             }
 
             if (sResource->isObservable())

diff --git a/service/resource-encapsulation/src/resourceCache/src/ResourceCacheManager.cpp b/service/resource-encapsulation/src/resourceCache/src/ResourceCacheManager.cpp
index 7b0728c..107e0ce 100644 (file)
--- a/service/resource-encapsulation/src/resourceCache/src/ResourceCacheManager.cpp
+++ b/service/resource-encapsulation/src/resourceCache/src/ResourceCacheManager.cpp
@@ -26,17 +26,24 @@ namespace OIC
 {
     namespace Service
     {
-        ResourceCacheManager *ResourceCacheManager::s_instance = NULL;
+        ResourceCacheManager *ResourceCacheManager::s_instance = nullptr;
         std::mutex ResourceCacheManager::s_mutexForCreation;
         std::mutex ResourceCacheManager::s_mutex;
         std::unique_ptr<std::list<DataCachePtr>> ResourceCacheManager::s_cacheDataList(nullptr);
 
+        void ResourceCacheManager::stopResourceCacheManager()
+        {
+            delete s_instance;
+            s_instance = nullptr;
+        }
+
         ResourceCacheManager::~ResourceCacheManager()
         {
             std::lock_guard<std::mutex> lock(s_mutex);
             if (s_cacheDataList != nullptr)
             {
                 s_cacheDataList->clear();
+                s_cacheDataList.reset();
             }
         }
 

diff --git a/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp b/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp
index b7a8d56..617596c 100644 (file)
--- a/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp
+++ b/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp
@@ -19,8 +19,9 @@
 //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
 #include <iostream>
-#include <gtest/gtest.h>
-#include <HippoMocks/hippomocks.h>
+#include <mutex>
+#include <condition_variable>
+#include <chrono>
 
 #include "ResourceCacheManager.h"
 #include "DataCache.h"
@@ -42,19 +43,33 @@ class DataCacheTest : public TestWithMock
              int) > ObserveCallback;
     public:
         std::shared_ptr<DataCache> cacheHandler;
-        PrimitiveResource::Ptr pResource;
         CacheCB cb;
         CacheID id;
+        static PrimitiveResource::Ptr pResource;
+        static MockRepository mocks;
+        static bool isLast;
 
     protected:
+        DataCacheTest()
+        {
+            id = 0;
+            static std::once_flag flag;
+            std::call_once(flag, [this]()
+            {
+                isLast = false;
+                pResource = PrimitiveResource::Ptr(mocks.Mock< PrimitiveResource >());
+            });
+        }
+        virtual ~DataCacheTest() noexcept(noexcept(std::declval<Test>().~Test()))
+        {
+            if (isLast)
+            {
+                mocks.reset();
+            }
+        }
         virtual void SetUp()
         {
             TestWithMock::SetUp();
-            pResource = PrimitiveResource::Ptr(mocks.Mock< PrimitiveResource >(),
-                                               [](PrimitiveResource *)
-                                               {
-
-                                               });
 
             mocks.OnCall(pResource.get(), PrimitiveResource::isObservable).Return(false);
             cacheHandler.reset(new DataCache());
@@ -67,14 +82,16 @@ class DataCacheTest : public TestWithMock
         virtual void TearDown()
         {
             cacheHandler.reset();
-            pResource.reset();
             TestWithMock::TearDown();
         }
 };
 
+PrimitiveResource::Ptr DataCacheTest::pResource = nullptr;
+MockRepository DataCacheTest::mocks;
+bool DataCacheTest::isLast;
+
 TEST_F(DataCacheTest, initializeDataCache_normalCase)
 {
-
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -117,35 +134,35 @@ TEST_F(DataCacheTest, initializeDataCache_normalCaseObservable)
 TEST_F(DataCacheTest, initializeDataCache_normalCaseNonObservable)
 {
 
+    std::condition_variable responseCon;
+    std::mutex mutexForCondition;
+
     mocks.OnCall(pResource.get(), PrimitiveResource::requestGet).Do(
-        [](GetCallback callback)
+        [& responseCon](GetCallback callback)
     {
         OIC::Service::HeaderOptions hos;
 
         OIC::Service::RCSResourceAttributes attr;
         OIC::Service::ResponseStatement rep(attr);
         callback(hos, rep, OC_STACK_OK);
-        return;
-    }
-    );
+        responseCon.notify_all();
+    });
     mocks.OnCall(pResource.get(), PrimitiveResource::isObservable).Return(false);
     mocks.OnCall(pResource.get(), PrimitiveResource::cancelObserve);
 
     cacheHandler->initializeDataCache(pResource);
 
-    sleep(3);
+    std::unique_lock< std::mutex > lock{ mutexForCondition };
+    responseCon.wait_for(lock, std::chrono::milliseconds(1000));
 }
 
 TEST_F(DataCacheTest, initializeDataCache_normalCaseTimeOut)
 {
-
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestObserve);
     mocks.OnCall(pResource.get(), PrimitiveResource::cancelObserve);
-
     cacheHandler->initializeDataCache(pResource);
-
     sleep(3);
 }
 
@@ -268,4 +285,5 @@ TEST_F(DataCacheTest, requestGet_normalCasetest)
     cacheHandler->initializeDataCache(pResource);
 
     cacheHandler->requestGet();
+    isLast = true;
 }

diff --git a/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp b/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp
index b3a2bfd..67d443d 100644 (file)
--- a/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp
+++ b/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp
@@ -19,8 +19,7 @@
 //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
 #include <iostream>
-#include <gtest/gtest.h>
-#include <HippoMocks/hippomocks.h>
+#include <mutex>
 
 #include "ResourceCacheManager.h"
 #include "UnitTestHelper.h"
@@ -31,20 +30,25 @@ class ResourceCacheManagerTest : public TestWithMock
 {
     public:
         ResourceCacheManager *cacheInstance;
-        PrimitiveResource::Ptr pResource;
         CacheCB cb;
         CacheID id;
+        static PrimitiveResource::Ptr pResource;
+        static MockRepository mocks;
+        static bool isLast;
 
     protected:
         virtual void SetUp()
         {
             TestWithMock::SetUp();
+            id = 0;
             cacheInstance = ResourceCacheManager::getInstance();
-            pResource = PrimitiveResource::Ptr(mocks.Mock< PrimitiveResource >(),
-                                               [](PrimitiveResource *)
-                                               {
 
-                                               });
+            static std::once_flag flag;
+            std::call_once(flag, [this]()
+            {
+                isLast = false;
+                pResource = PrimitiveResource::Ptr(mocks.Mock< PrimitiveResource >());
+            });
             mocks.OnCall(pResource.get(), PrimitiveResource::isObservable).Return(false);
             cb = ([](std::shared_ptr<PrimitiveResource >, const RCSResourceAttributes &)->OCStackResult
                     {
@@ -54,11 +58,18 @@ class ResourceCacheManagerTest : public TestWithMock
 
         virtual void TearDown()
         {
-            pResource.reset();
-            TestWithMock::TearDown();
+            //TestWithMock::TearDown();
+            if (isLast)
+            {
+                mocks.reset();
+            }
         }
 };
 
+PrimitiveResource::Ptr ResourceCacheManagerTest::pResource;
+MockRepository ResourceCacheManagerTest::mocks;
+bool ResourceCacheManagerTest::isLast;
+
 TEST_F(ResourceCacheManagerTest, requestResourceCache_resourceIsNULL)
 {
 
@@ -85,7 +96,6 @@ TEST_F(ResourceCacheManagerTest, requestResourceCache_cacheCBIsNULL)
 
 TEST_F(ResourceCacheManagerTest, requestResourceCache_reportTimeIsNULL)
 {
-
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -104,7 +114,6 @@ TEST_F(ResourceCacheManagerTest, requestResourceCache_reportTimeIsNULL)
 
 TEST_F(ResourceCacheManagerTest, requestResourceCache_normalCase)
 {
-
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -130,7 +139,6 @@ TEST_F(ResourceCacheManagerTest, cancelResourceCache_cacheIDIsZero)
 
 TEST_F(ResourceCacheManagerTest, cancelResourceCache_normalCase)
 {
-
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.ExpectCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -162,7 +170,6 @@ TEST_F(ResourceCacheManagerTest, updateResourceCacheCacheID_cacheIsNULL)
 
 TEST_F(ResourceCacheManagerTest, updateResourceCacheCacheID_normalCase)
 {
-
     mocks.OnCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.OnCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.OnCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -210,7 +217,6 @@ TEST_F(ResourceCacheManagerTest, getResourceCacheStateCacheID_handlerIsNULL)
 
 TEST_F(ResourceCacheManagerTest, getResourceCacheStateCacheID_normalCase)
 {
-
     mocks.OnCall(pResource.get(), PrimitiveResource::requestGet);
     mocks.OnCall(pResource.get(), PrimitiveResource::isObservable).Return(true);
     mocks.OnCall(pResource.get(), PrimitiveResource::requestObserve);
@@ -229,4 +235,6 @@ TEST_F(ResourceCacheManagerTest, getResourceCacheStateCacheID_normalCase)
     cacheInstance->cancelResourceCache(id);
 
     ASSERT_EQ(state, CACHE_STATE::READY_YET);
+    ResourceCacheManager::stopResourceCacheManager();
+    isLast = true;
 }