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 8b3dbe556cd67b47e14e99b3992c1f7a51c8d5d8..e43e0a14971120f9d12cc167be48963f203bf48c 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
@@ -120,6 +122,20 @@ namespace OIC
                 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
+                *
+                * @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
                 *
@@ -132,14 +148,15 @@ namespace OIC
                 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 cdfc9980828f9f3ba90079377d768020c83f6e0f..8ec6b873420c4fa067b560f8538aba1f16b8ea68 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 d26654d0b93a53a07c44cd95279feacf7d7a5d29..c48e72a274dabaebac9633da4e569903afa6370e 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 e264962cc467b5aa18024b8060328ecacf33543f..dcdca42eb1c39f012a669aea156ecc3b562e1e56 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 9bfe09fe4210a378bd774549142ac2c8e391b57b..899def05e4b11701a175a055f9b8d99f0391da38 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 e7ce3f3711fc53fe59d3043edab8a9f7e816fd58..ef26a7e94caaa8a2253713a27d410608844e2427 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 583094e1b49d633f675a18a4e23c07bd76d5e425..17edfcb990ca763e5f4affc5e8fee2b742489f7d 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 d51febb7be1d577081199f0d499f4dd96bdb4738..e55f9fe63922f330d72c413724bf9359a7706578 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 40c4aa0ee45cf61b737955fe25f06826351725df..1aa0835f2ff0b340fcaadeccdc973bdeed50ded4 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 6490685b73f8d7b253d4693318a332b7ab7f450f..90ae1a10dd05817aa3776db69af9998c536f1056 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 4bd10a7db62f107510399230825191c06eb0cdc4..f8089041392443ef213527d125c383014b0f1241 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 bf9b59b722f495cfb1c874c91f84bd79a0207da3..b600f115f6448327f77f1e1d91ed4b0dcac1565d 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 0dc750bdee84fdb1f99d93486581e729c23c7bd2..c0c687c796df6cb292c55a7f5ed29f340f90de51 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 e2abbb4f6db2db705d04df11bba4d8b005712dec..df738b078ef430d8ae651f3b8a19938fc906f460 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 98a146baec55f8ee81e81d42d85c5cd7c92030e3..5cb15762ea08ec03246339b0a7a79dd0b81b29d0 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 0bbbf9cf6a71a117cb0db6e9acd924fcf3e705a8..dd910588a6e1b2ccc21cab2b40a513aa72a7ba9f 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_ */