From 84d03f5a9472b1f0de125d1fb9738c1e774ef7db Mon Sep 17 00:00:00 2001 From: "jyong2.kim" Date: Thu, 23 Jul 2015 17:01:17 +0900 Subject: [PATCH] Fix invalid memory access on resource broker module. remove the duplicated memory access on list. invalidate callback in deleted instance. delete useless member variable on ResourcePresence class. Change-Id: If6b3831fa2355140b23efdafc213615280e336ac Signed-off-by: jyong2.kim Reviewed-on: https://gerrit.iotivity.org/gerrit/1844 Tested-by: jenkins-iotivity Reviewed-by: kwon doil Reviewed-by: Uze Choi --- .../NotificationManager/src/HostingObject.cpp | 2 +- .../src/resourceBroker/include/BrokerTypes.h | 1 + .../src/resourceBroker/include/ResourcePresence.h | 8 +-- .../src/resourceBroker/src/ResourcePresence.cpp | 84 ++++++++++++---------- 4 files changed, 51 insertions(+), 44 deletions(-) diff --git a/service/notification-manager/NotificationManager/src/HostingObject.cpp b/service/notification-manager/NotificationManager/src/HostingObject.cpp index a8c030c..22d4b5c 100644 --- a/service/notification-manager/NotificationManager/src/HostingObject.cpp +++ b/service/notification-manager/NotificationManager/src/HostingObject.cpp @@ -110,7 +110,7 @@ void HostingObject::stateChangedCB(ResourceState state, RemoteObjectPtr rObject) { try { -// rObject->stopMonitoring(); + rObject->stopMonitoring(); }catch(InvalidParameterException &e) { OIC_HOSTING_LOG(DEBUG, diff --git a/service/resource-manipulation/src/resourceBroker/include/BrokerTypes.h b/service/resource-manipulation/src/resourceBroker/include/BrokerTypes.h index f281811..8ef780f 100755 --- a/service/resource-manipulation/src/resourceBroker/include/BrokerTypes.h +++ b/service/resource-manipulation/src/resourceBroker/include/BrokerTypes.h @@ -82,6 +82,7 @@ namespace OIC typedef std::function BrokerCB; struct BrokerRequesterInfo { + BrokerRequesterInfo(BrokerID _id, BrokerCB _cb) : brokerId(_id), brokerCB(_cb){} BrokerID brokerId; BrokerCB brokerCB; }; diff --git a/service/resource-manipulation/src/resourceBroker/include/ResourcePresence.h b/service/resource-manipulation/src/resourceBroker/include/ResourcePresence.h index 62de9b5..acb54b6 100755 --- a/service/resource-manipulation/src/resourceBroker/include/ResourcePresence.h +++ b/service/resource-manipulation/src/resourceBroker/include/ResourcePresence.h @@ -35,7 +35,7 @@ namespace OIC { namespace Service { - class ResourcePresence + class ResourcePresence : public std::enable_shared_from_this { public: ResourcePresence(); @@ -64,10 +64,8 @@ namespace OIC BROKER_MODE mode; bool isWithinTime; - boost::atomic_bool isTimeoutCB; boost::atomic_long receivedTime; std::mutex cbMutex; - std::condition_variable cbCondition; unsigned int timeoutHandle; RequestGetCB pGetCB; @@ -75,10 +73,12 @@ namespace OIC TimerCB pPollingCB; void registerDevicePresence(); + public: void getCB(const HeaderOptions &hos, const ResponseStatement& rep, int eCode); + void timeOutCB(unsigned int msg); + private: void verifiedGetResponse(int eCode); - void timeOutCB(unsigned int msg); void pollingCB(unsigned int msg = 0); void executeAllBrokerCB(BROKER_STATE changedState); diff --git a/service/resource-manipulation/src/resourceBroker/src/ResourcePresence.cpp b/service/resource-manipulation/src/resourceBroker/src/ResourcePresence.cpp index f9eb8de..9cd1053 100755 --- a/service/resource-manipulation/src/resourceBroker/src/ResourcePresence.cpp +++ b/service/resource-manipulation/src/resourceBroker/src/ResourcePresence.cpp @@ -33,30 +33,48 @@ #include "DeviceAssociation.h" #include "DevicePresence.h" +namespace +{ +using namespace OIC::Service; + + void getCallback(const HeaderOptions &hos, const ResponseStatement& rep, + int eCode, std::weak_ptr this_ptr) + { + std::shared_ptr Ptr = this_ptr.lock(); + if(Ptr) + { + Ptr->getCB(hos, rep, eCode); + } + } + void timeOutCallback(unsigned int msg, std::weak_ptr this_ptr) + { + std::shared_ptr Ptr = this_ptr.lock(); + if(Ptr) + { + Ptr->timeOutCB(msg); + } + } +} + namespace OIC { namespace Service { ResourcePresence::ResourcePresence() + : requesterList(nullptr), primitiveResource(nullptr), + state(BROKER_STATE::REQUESTED), mode(BROKER_MODE::NON_PRESENCE_MODE), + isWithinTime(true), receivedTime(0L), timeoutHandle(0) { - primitiveResource = nullptr; - isTimeoutCB = false; - receivedTime = 0L; - state = BROKER_STATE::REQUESTED; - isWithinTime = true; - mode = BROKER_MODE::NON_PRESENCE_MODE; - timeoutHandle = 0; - - requesterList = nullptr; - - pGetCB = std::bind(&ResourcePresence::getCB, this, - std::placeholders::_1, std::placeholders::_2, std::placeholders::_3); - pTimeoutCB = std::bind(&ResourcePresence::timeOutCB, this,std::placeholders::_1); - pPollingCB = std::bind(&ResourcePresence::pollingCB, this,std::placeholders::_1); } void ResourcePresence::initializeResourcePresence(PrimitiveResourcePtr pResource) { + pGetCB = std::bind(getCallback, std::placeholders::_1, std::placeholders::_2, + std::placeholders::_3, std::weak_ptr(shared_from_this())); + pTimeoutCB = std::bind(timeOutCallback, std::placeholders::_1, + std::weak_ptr(shared_from_this())); + pPollingCB = std::bind(&ResourcePresence::pollingCB, this, std::placeholders::_1); + primitiveResource = pResource; requesterList = std::unique_ptr> @@ -94,12 +112,8 @@ namespace OIC void ResourcePresence::addBrokerRequester(BrokerID _id, BrokerCB _cb) { - BrokerRequesterInfoPtr newRequester; - newRequester.reset(new BrokerRequesterInfo()); - - newRequester->brokerId = _id; - newRequester->brokerCB = _cb; - requesterList->push_back(newRequester); + requesterList->push_back( + std::make_shared(BrokerRequesterInfo(_id, _cb))); } void ResourcePresence::removeAllBrokerRequester() @@ -168,7 +182,8 @@ namespace OIC setResourcestate(changedState); if(requesterList->empty() != true) { - for(BrokerRequesterInfoPtr & item : * requesterList) + std::list list = * requesterList; + for(BrokerRequesterInfoPtr item : list) { item->brokerCB(state); } @@ -184,7 +199,6 @@ namespace OIC void ResourcePresence::timeOutCB(unsigned int msg) { std::unique_lock lock(cbMutex); - isTimeoutCB = true; time_t currentTime; time(¤tTime); @@ -193,10 +207,7 @@ namespace OIC if((receivedTime.load(boost::memory_order_consume) == 0) || ((receivedTime + BROKER_SAFE_SECOND) > currentTime )) { - this->isWithinTime = false; - isTimeoutCB = false; - cbCondition.notify_all(); - + this->isWithinTime = true; return; } this->isWithinTime = false; @@ -205,28 +216,24 @@ namespace OIC executeAllBrokerCB(BROKER_STATE::LOST_SIGNAL); pollingCB(); - - isTimeoutCB = false; - cbCondition.notify_all(); } void ResourcePresence::pollingCB(unsigned int msg) { - OC_LOG_V(DEBUG,BROKER_TAG,"IN PollingCB\n"); - this->requestResourceState(); - timeoutHandle = expiryTimer.postTimer(BROKER_SAFE_MILLISECOND,pTimeoutCB); + if(this->requesterList->size() != 0) + { + OC_LOG_V(DEBUG,BROKER_TAG,"IN PollingCB\n"); + this->requestResourceState(); + timeoutHandle = expiryTimer.postTimer(BROKER_SAFE_MILLISECOND,pTimeoutCB); + } } void ResourcePresence::getCB(const HeaderOptions &hos, const ResponseStatement& rep, int eCode) { OC_LOG_V(DEBUG, BROKER_TAG, "response getCB\n"); - while(isTimeoutCB) - { - OC_LOG_V(DEBUG, BROKER_TAG, "waiting for terminate TimeoutCB\n"); - std::unique_lock lock(cbMutex); - cbCondition.wait(lock); - } + OC_LOG_V(DEBUG, BROKER_TAG, "waiting for terminate TimeoutCB\n"); + std::unique_lock lock(cbMutex); time_t currentTime; time(¤tTime); @@ -242,7 +249,6 @@ namespace OIC if(mode == BROKER_MODE::NON_PRESENCE_MODE) { - // TODO set timer & request get expiryTimer.postTimer(BROKER_SAFE_MILLISECOND,pPollingCB); } -- 2.7.4