Fix for IOT-927 resource container attribute update/notify race condition
authorMarkus Jung <markus.jung@samsung.com>
Tue, 2 Feb 2016 12:34:13 +0000 (21:34 +0900)
committerUze Choi <uzchoi@samsung.com>
Tue, 23 Feb 2016 04:42:59 +0000 (04:42 +0000)
- Asynchronous notification of observing clients
- Mutex for access on RCSResourceAttributes
- returning RCSResourceAttributes by value
- reference to const RCSResourceAttributes for handleSetAttributeRequest API

Note: The change breaks backward compatibility with previous APIs

Change-Id: I2f84353aa993772e7a1a908c766c77786fe7c3ea
Signed-off-by: Markus Jung <markus.jung@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/4913
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Madan Lanka <lanka.madan@samsung.com>
Reviewed-by: George Nash <george.nash@intel.com>
Reviewed-by: JungHo Kim <jhyo.kim@samsung.com>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
16 files changed:
service/resource-container/bundle-api/include/BundleResource.h
service/resource-container/bundle-api/include/ProtocolBridgeResource.h
service/resource-container/bundle-api/include/SoftSensorResource.h
service/resource-container/examples/BMISensorBundle/include/BMISensorResource.h
service/resource-container/examples/BMISensorBundle/src/BMISensorResource.cpp
service/resource-container/examples/ContainerSampleClient.cpp
service/resource-container/examples/DiscomfortIndexSensorBundle/include/DiscomfortIndexSensorResource.h
service/resource-container/examples/DiscomfortIndexSensorBundle/src/DiscomfortIndexSensorResource.cpp
service/resource-container/examples/HueSampleBundle/include/HueLight.h
service/resource-container/examples/HueSampleBundle/src/HueLight.cpp
service/resource-container/src/BundleResource.cpp
service/resource-container/src/JavaBundleResource.cpp
service/resource-container/src/JavaBundleResource.h
service/resource-container/src/ResourceContainerImpl.cpp
service/resource-container/unittests/ResourceContainerTest.cpp
service/resource-container/unittests/TestBundle/include/TestBundleActivator.h

index 8b3dbe5..e43e0a1 100644 (file)
@@ -26,6 +26,8 @@
 #include <map>
 #include <vector>
 #include <memory>
+#include <mutex>
+
 
 #include "NotificationReceiver.h"
 #include "RCSResourceAttributes.h"
@@ -79,14 +81,14 @@ namespace OIC
                 *
                 * @return void
                 */
-                void registerObserver(NotificationReceiver *pNotiReceiver);
+                void registerObserver(NotificationReceiverpNotiReceiver);
 
                 /**
                 * Return all attributes of the resource
                 *
                 * @return Attributes of the resource
                 */
-                RCSResourceAttributes &getAttributes();
+                const RCSResourceAttributes getAttributes();
 
                 /**
                 * Set attributes of the resource
@@ -95,7 +97,7 @@ namespace OIC
                 *
                 * @return void
                 */
-                void setAttributes(RCSResourceAttributes &attrs);
+                void setAttributes(const RCSResourceAttributes &attrs);
 
                 /**
                 * Return the value of an attribute
@@ -127,19 +129,34 @@ namespace OIC
                 *
                 * @param value Value of attribute to set
                 *
+                * @param notify Flag to indicate if OIC clients should be notified about an update
+                *
+                * @return void
+                */
+                void setAttribute(const std::string &key, RCSResourceAttributes::Value &value,
+                                  bool notify);
+
+                /**
+                * Sets the value of an attribute
+                *
+                * @param key Name of attribute to set
+                *
+                * @param value Value of attribute to set
+                *
                 * @return void
                 */
                 void setAttribute(const std::string &key, RCSResourceAttributes::Value &&value);
 
                 /**
-                * Sends a notification to all observers.
+                * Sets the value of an attribute
                 *
-                * Calling this is not needed when setAttribute() was called
-                * with notify == true.
+                * @param key Name of attribute to set
+                *
+                * @param value Value of attribute to set
                 *
                 * @return void
                 */
-                void sendNotification();
+                void setAttribute(const std::string &key, RCSResourceAttributes::Value &value);
 
                 /**
                 * This function should be implemented by the according bundle resource
@@ -151,7 +168,7 @@ namespace OIC
                 *
                 * @return All attributes
                 */
-                virtual RCSResourceAttributes &handleGetAttributesRequest() = 0;
+                virtual RCSResourceAttributes handleGetAttributesRequest() = 0;
 
                 /**
                 * This function should be implemented by the according bundle resource
@@ -169,8 +186,10 @@ namespace OIC
                 *
                 * @return void
                 */
-                virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs) = 0;
+                virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs) = 0;
+            private:
 
+                void sendNotification(NotificationReceiver *notficiationRecevier, std::string uri);
 
             public:
                 std::string m_bundleId;
@@ -179,8 +198,9 @@ namespace OIC
                     std::vector< std::map< std::string, std::string > > > m_mapResourceProperty;
 
             private:
-                NotificationReceiver *m_pNotiReceiver;
+                NotificationReceiverm_pNotiReceiver;
                 RCSResourceAttributes m_resourceAttributes;
+                std::mutex m_resourceAttributes_mutex;
         };
     }
 }
index cdfc998..8ec6b87 100644 (file)
@@ -67,7 +67,7 @@ namespace OIC
                 *
                 * @return Value of all attributes
                 */
-                virtual RCSResourceAttributes &handleGetAttributesRequest() = 0;
+                virtual RCSResourceAttributes handleGetAttributesRequest() = 0;
 
                 /**
                 * This function should be implemented by the according bundle resource
@@ -85,7 +85,7 @@ namespace OIC
                 *
                 * @return void
                 */
-                virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs) = 0;
+                virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs) = 0;
         };
     }
 }
index d26654d..c48e72a 100644 (file)
@@ -64,7 +64,7 @@ namespace OIC
                 *
                 * @return Value of all attributes
                 */
-                virtual RCSResourceAttributes &handleGetAttributesRequest() = 0;
+                virtual RCSResourceAttributes handleGetAttributesRequest() = 0;
 
                 /**
                 * This function should be implemented by the according bundle resource
@@ -82,7 +82,7 @@ namespace OIC
                 *
                 * @return void
                 */
-                virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs) = 0;
+                virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs) = 0;
 
                 /**
                 * SoftSensor logic. Has to be provided by the soft sensor developer.
index e264962..dcdca42 100644 (file)
@@ -35,9 +35,9 @@ class BMISensorResource : public SoftSensorResource
         BMISensorResource& operator=( const BMISensorResource& rhs )=delete;
         ~BMISensorResource();
 
-        virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs);
+        virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs);
 
-        virtual RCSResourceAttributes &handleGetAttributesRequest();
+        virtual RCSResourceAttributes handleGetAttributesRequest();
 
         virtual void executeLogic();
 
index 9bfe09f..899def0 100644 (file)
@@ -32,12 +32,12 @@ BMISensorResource::~BMISensorResource()
 }
 
 void BMISensorResource::handleSetAttributesRequest(
-    RCSResourceAttributes &value)
+    const RCSResourceAttributes &value)
 {
     BundleResource::setAttributes(value);
 }
 
-RCSResourceAttributes &BMISensorResource::handleGetAttributesRequest()
+RCSResourceAttributes BMISensorResource::handleGetAttributesRequest()
 {
     return BundleResource::getAttributes();
 }
@@ -63,4 +63,4 @@ void BMISensorResource::onUpdatedInputResource(const std::string attributeName,
         m_mapInputData.insert(std::make_pair("height", values.back().toString()));
 
     executeLogic();
-}
\ No newline at end of file
+}
index e7ce3f3..ef26a7e 100644 (file)
@@ -289,7 +289,7 @@ void putLightRepresentation(std::shared_ptr<OCResource> resource)
 
         // Invoke resource's put API with rep, query map and the callback parameter
 
-        resource->put(rep, QueryParamsMap(), &onPut);
+        resource->post(rep, QueryParamsMap(), &onPut);
     }
 }
 
@@ -314,7 +314,7 @@ void onGet(const HeaderOptions &headerOptions, const OCRepresentation &rep, cons
             std::cout << "\tcolor: " << mylight.m_color << std::endl;
             std::cout << "\tdim: " << mylight.m_dim << std::endl;
 
-            putLightRepresentation(curResource);
+            postLightRepresentation(curResource);
         }
         else
         {
index 583094e..17edfcb 100644 (file)
@@ -35,9 +35,9 @@ class DiscomfortIndexSensorResource : public SoftSensorResource
         DiscomfortIndexSensorResource(const DiscomfortIndexSensorResource &other)=delete;
         DiscomfortIndexSensorResource& operator=( const DiscomfortIndexSensorResource& rhs )=delete;
 
-        virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs);
+        virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs);
 
-        virtual RCSResourceAttributes &handleGetAttributesRequest();
+        virtual RCSResourceAttributes handleGetAttributesRequest();
 
         virtual void executeLogic();
 
index d51febb..e55f9fe 100644 (file)
@@ -35,12 +35,12 @@ DiscomfortIndexSensorResource::~DiscomfortIndexSensorResource()
 }
 
 void DiscomfortIndexSensorResource::handleSetAttributesRequest(
-    RCSResourceAttributes &value)
+    const RCSResourceAttributes &value)
 {
     BundleResource::setAttributes(value);
 }
 
-RCSResourceAttributes &DiscomfortIndexSensorResource::handleGetAttributesRequest()
+RCSResourceAttributes DiscomfortIndexSensorResource::handleGetAttributesRequest()
 {
     return BundleResource::getAttributes();
 }
index 40c4aa0..1aa0835 100644 (file)
@@ -40,9 +40,9 @@ namespace OIC
 
                 virtual void initAttributes();
 
-                virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs);
+                virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs);
 
-                virtual RCSResourceAttributes &handleGetAttributesRequest();
+                virtual RCSResourceAttributes handleGetAttributesRequest();
 
 
             private:
index 6490685..90ae1a1 100644 (file)
@@ -49,41 +49,41 @@ void HueLight::initAttributes()
     BundleResource::setAttribute("color", 0);
 }
 
-RCSResourceAttributes &HueLight::handleGetAttributesRequest()
+RCSResourceAttributes HueLight::handleGetAttributesRequest()
 {
     cout << "HueLight::handleGetAttributesRequest" << endl;
     // TODO read from HueLight and update attribute data
     return BundleResource::getAttributes();
 }
 
-void HueLight::handleSetAttributesRequest(RCSResourceAttributes &value)
+void HueLight::handleSetAttributesRequest(const RCSResourceAttributes &value)
 {
     cout << "HueLight::handleSetAttributesRequest" << std::endl;
 
     // TODO construct single write
 
-    for (RCSResourceAttributes::iterator it = value.begin(); it != value.end(); it++)
+    for (RCSResourceAttributes::const_iterator it = value.begin(); it != value.end(); it++)
     {
         std::string attributeName = it->key();
         RCSResourceAttributes::Value attrValue = it->value();
 
         if (attributeName == "on-off")
         {
-            m_connector->transmit(this->m_address + "/state", "{\"on\":" + attrValue.toString() + "}");
+            //m_connector->transmit(this->m_address + "/state", "{\"on\":" + attrValue.toString() + "}");
         }
 
         if (attributeName == "dim")
         {
             // needs conversion * 2.5
-            m_connector->transmit(this->m_address + "/state", "{\"bri\":" + attrValue.toString() + "}");
+            //m_connector->transmit(this->m_address + "/state", "{\"bri\":" + attrValue.toString() + "}");
         }
 
         if (attributeName == "color")
         {
             // needs conversion *650
-            m_connector->transmit(this->m_address + "/state", "{\"hue\":" + attrValue.toString() + "}");
+            //m_connector->transmit(this->m_address + "/state", "{\"hue\":" + attrValue.toString() + "}");
         }
     }
 
     BundleResource::setAttributes(value);
-}
\ No newline at end of file
+}
index 4bd10a7..f808904 100644 (file)
@@ -22,6 +22,9 @@
 
 #include <list>
 #include <string.h>
+#include <iostream>
+#include <boost/thread.hpp>
+#include "NotificationReceiver.h"
 
 #include "InternalTypes.h"
 
@@ -29,9 +32,9 @@ namespace OIC
 {
     namespace Service
     {
-        BundleResource::BundleResource()
+        BundleResource::BundleResource() : m_pNotiReceiver(nullptr)
         {
-            m_pNotiReceiver = nullptr;
+
         }
 
         BundleResource::~BundleResource()
@@ -47,28 +50,42 @@ namespace OIC
         std::list< std::string > BundleResource::getAttributeNames()
         {
             std::list< std::string > ret;
-            for (RCSResourceAttributes::iterator it = m_resourceAttributes.begin();
-                 it != m_resourceAttributes.end(); ++it)
-            {
-                ret.push_back(it->key());
+
+            for (auto &it : m_resourceAttributes){
+                ret.push_back(it.key());
             }
+
             return ret;
         }
 
-        RCSResourceAttributes &BundleResource::getAttributes()
+        const RCSResourceAttributes BundleResource::getAttributes()
         {
-            return m_resourceAttributes;
+            std::lock_guard<std::mutex> lock(m_resourceAttributes_mutex);
+            return RCSResourceAttributes(m_resourceAttributes);
         }
 
-        void BundleResource::setAttributes(RCSResourceAttributes &attrs)
+        void BundleResource::setAttributes(const RCSResourceAttributes &attrs)
         {
-            for (RCSResourceAttributes::iterator it = attrs.begin(); it != attrs.end(); ++it)
-            {
+            std::lock_guard<std::mutex> lock(m_resourceAttributes_mutex);
+
+            for (auto &it : m_resourceAttributes){
                 OIC_LOG_V(INFO, CONTAINER_TAG, "set attribute \(%s)'",
-                         std::string(it->key() + "\', with " + it->value().toString()).c_str());
+                           std::string(it.key() + "\', with " + it.value().toString()).c_str());
 
-                m_resourceAttributes[it->key()] = it->value();
+                m_resourceAttributes[it.key()] = it.value();
             }
+
+            // asynchronous notification
+            auto notifyFunc = [](NotificationReceiver *notificationReceiver,
+                                    std::string uri)
+            {
+                if (notificationReceiver){
+                    notificationReceiver->onNotificationReceived(uri);
+                }
+            };
+            auto f = std::bind(notifyFunc, m_pNotiReceiver, m_uri);
+            boost::thread notifyThread(f);
+
         }
 
         void BundleResource::setAttribute(const std::string &key,
@@ -76,28 +93,47 @@ namespace OIC
         {
             OIC_LOG_V(INFO, CONTAINER_TAG, "set attribute \(%s)'", std::string(key + "\', with " +
                      value.toString()).c_str());
+            std::lock_guard<std::mutex> lock(m_resourceAttributes_mutex);
+            m_resourceAttributes[key] = std::move(value);
+
+            if(notify){
+                // asynchronous notification
+                auto notifyFunc = [](NotificationReceiver *notificationReceiver,
+                                        std::string uri)
+                {
+                    if (notificationReceiver){
+                        notificationReceiver->onNotificationReceived(uri);
+                    }
+                };
+                auto f = std::bind(notifyFunc, m_pNotiReceiver, m_uri);
+                boost::thread notifyThread(f);
+            }
 
-            m_resourceAttributes[key] = value;
+        }
 
-            sendNotification();
+        void BundleResource::setAttribute(const std::string &key,
+                                                 RCSResourceAttributes::Value &value, bool notify)
+        {
+            setAttribute(key, RCSResourceAttributes::Value(value), notify);
         }
 
-        void BundleResource::setAttribute(const std::string &key, RCSResourceAttributes::Value &&value)
+        void BundleResource::setAttribute(const std::string &key,
+                RCSResourceAttributes::Value &&value)
         {
             setAttribute(key, std::move(value), true);
         }
 
-        RCSResourceAttributes::Value BundleResource::getAttribute(const std::string &key)
+        void BundleResource::setAttribute(const std::string &key,
+                        RCSResourceAttributes::Value &value)
         {
-            OIC_LOG_V(INFO, CONTAINER_TAG, "get attribute \'(%s)" , std::string(key + "\'").c_str());
-
-            return m_resourceAttributes.at(key);
+            setAttribute(key, value, true);
         }
 
-        void BundleResource::sendNotification()
+        RCSResourceAttributes::Value BundleResource::getAttribute(const std::string &key)
         {
-            if (m_pNotiReceiver)
-                m_pNotiReceiver->onNotificationReceived(m_uri);
+            OIC_LOG_V(INFO, CONTAINER_TAG, "get attribute \'(%s)" , std::string(key + "\'").c_str());
+            std::lock_guard<std::mutex> lock(m_resourceAttributes_mutex);
+            return m_resourceAttributes.at(key);
         }
     }
 }
index bf9b59b..b600f11 100644 (file)
@@ -140,14 +140,14 @@ void JavaBundleResource::handleSetAttributeRequest(const std::string &attributeN
 }
 
 
-void JavaBundleResource::handleSetAttributesRequest(RCSResourceAttributes &attrs){
+void JavaBundleResource::handleSetAttributesRequest(const RCSResourceAttributes &attrs){
     for (RCSResourceAttributes::iterator it = attrs.begin(); it != attrs.end(); ++it)
     {
         handleSetAttributeRequest(it->key(),std::move(it->value()));
     }
 }
 
-RCSResourceAttributes & JavaBundleResource::handleGetAttributesRequest()
+const RCSResourceAttributes JavaBundleResource::handleGetAttributesRequest()
 {
     std::list<string> attrsNames = getAttributeNames();
     for(std::list<string>::iterator iterator = attrsNames.begin();
index 0dc750b..c0c687c 100644 (file)
@@ -47,9 +47,9 @@ namespace OIC
 
             RCSResourceAttributes::Value handleGetAttributeRequest(const std::string& key);
 
-            virtual void handleSetAttributesRequest(RCSResourceAttributes &attrs);
+            virtual void handleSetAttributesRequest(const RCSResourceAttributes &attrs);
 
-            virtual RCSResourceAttributes& handleGetAttributesRequest();
+            virtual RCSResourceAttributes handleGetAttributesRequest();
 
             virtual void initAttributes();
         private:
index e2abbb4..df738b0 100644 (file)
@@ -302,7 +302,7 @@ namespace OIC
                     }
 
                     // to get notified if bundle resource attributes are updated
-                    resource->registerObserver((NotificationReceiver *) this);
+                    resource->registerObserver(this);
                     ret = 0;
                 }
             }
@@ -480,8 +480,10 @@ namespace OIC
         }
 
         void ResourceContainerImpl::addBundle(const std::string &bundleId,
-                                              const std::string &bundleUri, const std::string &bundlePath,
-                                              const std::string &activator, std::map< string, string > params)
+                                              const std::string &bundleUri,
+                                              const std::string &bundlePath,
+                                              const std::string &activator,
+                                              std::map< string, string > params)
         {
             (void) bundleUri;
 
index 98a146b..5cb1576 100644 (file)
@@ -98,12 +98,12 @@ class TestBundleResource: public BundleResource
     public:
         virtual void initAttributes() { }
 
-        virtual void handleSetAttributesRequest(RCSResourceAttributes &attr)
+        virtual void handleSetAttributesRequest(const RCSResourceAttributes &attr)
         {
             BundleResource::setAttributes(attr);
         }
 
-        virtual RCSResourceAttributes &handleGetAttributesRequest()
+        virtual RCSResourceAttributes handleGetAttributesRequest()
         {
             return BundleResource::getAttributes();
         }
index 0bbbf9c..dd91058 100644 (file)
@@ -52,16 +52,16 @@ class TestBundleResource : public BundleResource
     public:
         void initAttributes() { };
 
-        RCSResourceAttributes &handleGetAttributesRequest()
+        RCSResourceAttributes handleGetAttributesRequest()
         {
             return BundleResource::getAttributes();
         }
 
         void handleSetAttributesRequest(
-            RCSResourceAttributes &value)
+            const RCSResourceAttributes &value)
         {
             BundleResource::setAttributes(value);
         }
 };
 
-#endif /* TESTBUNDLE_H_ */
\ No newline at end of file
+#endif /* TESTBUNDLE_H_ */