From eb2cee8870247feda02b41aff22830bda3646ac9 Mon Sep 17 00:00:00 2001 From: YounghyunJoo Date: Tue, 6 Dec 2016 10:23:10 +0900 Subject: [PATCH] Fix memory leak on ResourceCache Module - check memory leaks through Valgrind tool, and fix them Change-Id: I58fb3356059a55313008fd04519779c35715b421 Signed-off-by: YounghyunJoo Reviewed-on: https://gerrit.iotivity.org/gerrit/15177 Tested-by: jenkins-iotivity Reviewed-by: Uze Choi --- .../resourceCache/include/ResourceCacheManager.h | 2 + .../src/resourceCache/src/DataCache.cpp | 2 +- .../src/resourceCache/src/ResourceCacheManager.cpp | 9 +++- .../src/resourceCache/unittests/DataCacheTest.cpp | 54 ++++++++++++++-------- .../resourceCache/unittests/ResourceCacheTest.cpp | 36 +++++++++------ 5 files changed, 69 insertions(+), 34 deletions(-) diff --git a/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h b/service/resource-encapsulation/src/resourceCache/include/ResourceCacheManager.h index ae02cac..0fc84c4 100644 --- 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 --- 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 --- 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> ResourceCacheManager::s_cacheDataList(nullptr); + void ResourceCacheManager::stopResourceCacheManager() + { + delete s_instance; + s_instance = nullptr; + } + ResourceCacheManager::~ResourceCacheManager() { std::lock_guard 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 --- a/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp +++ b/service/resource-encapsulation/src/resourceCache/unittests/DataCacheTest.cpp @@ -19,8 +19,9 @@ //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= #include -#include -#include +#include +#include +#include #include "ResourceCacheManager.h" #include "DataCache.h" @@ -42,19 +43,33 @@ class DataCacheTest : public TestWithMock int) > ObserveCallback; public: std::shared_ptr 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())) + { + 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 --- a/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp +++ b/service/resource-encapsulation/src/resourceCache/unittests/ResourceCacheTest.cpp @@ -19,8 +19,7 @@ //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= #include -#include -#include +#include #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, 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; } -- 2.7.4