Fix invoking attribute updated listener of RCSResourceObject.

There was a possible deadlock issue when the callback is invoked.
The mutex should be unlocked when the control is passed by invoking a callback to the users so that an object of the class can be used for the users without any concern regarding deadlock.

Change-Id: Iea35ab16858c7a8be67b87726b0e25ea6e94d73c
Signed-off-by: coderhyme <jhyo.kim@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/2959
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Madan Lanka <lanka.madan@samsung.com>

diff --git a/service/resource-encapsulation/include/RCSResourceObject.h b/service/resource-encapsulation/include/RCSResourceObject.h
index 14ac8df..5c80e56 100755 (executable)
--- a/service/resource-encapsulation/include/RCSResourceObject.h
+++ b/service/resource-encapsulation/include/RCSResourceObject.h
@@ -428,24 +428,28 @@ namespace OIC
             template< typename K, typename V >
             void setAttributeInternal(K&&, V&&);
 
+            bool applyAcceptanceMethod(const RCSSetResponse&, const RCSResourceAttributes&);
+
         private:
             const uint8_t m_properties;
 
             OCResourceHandle m_resourceHandle;
+
             RCSResourceAttributes m_resourceAttributes;
 
             GetRequestHandler m_getRequestHandler;
             SetRequestHandler m_setRequestHandler;
+
             AutoNotifyPolicy m_autoNotifyPolicy;
             SetRequestHandlerPolicy m_setRequestHandlerPolicy;
 
-            std::unordered_map< std::string, AttributeUpdatedListener >
-                    m_keyAttributesUpdatedListeners;
+            std::unordered_map< std::string, std::shared_ptr< AttributeUpdatedListener > >
+                    m_attributeUpdatedListeners;
 
             mutable std::unique_ptr< AtomicThreadId > m_lockOwner;
             mutable std::mutex m_mutex;
 
-            std::mutex m_mutexKeyAttributeUpdate;
+            std::mutex m_mutexAttributeUpdatedListeners;
 
         };
 

diff --git a/service/resource-encapsulation/src/serverBuilder/src/RCSResourceObject.cpp b/service/resource-encapsulation/src/serverBuilder/src/RCSResourceObject.cpp
index 261caf2..575cb2b 100644 (file)
--- a/service/resource-encapsulation/src/serverBuilder/src/RCSResourceObject.cpp
+++ b/service/resource-encapsulation/src/serverBuilder/src/RCSResourceObject.cpp
@@ -196,10 +196,10 @@ namespace OIC
                 m_setRequestHandler{ },
                 m_autoNotifyPolicy { AutoNotifyPolicy::UPDATED },
                 m_setRequestHandlerPolicy { SetRequestHandlerPolicy::NEVER },
-                m_keyAttributesUpdatedListeners{ },
+                m_attributeUpdatedListeners{ },
                 m_lockOwner{ },
                 m_mutex{ },
-                m_mutexKeyAttributeUpdate{ }
+                m_mutexAttributeUpdatedListeners{ }
         {
             m_lockOwner.reset(new AtomicThreadId);
         }
@@ -357,22 +357,26 @@ namespace OIC
         void RCSResourceObject::addAttributeUpdatedListener(const std::string& key,
                 AttributeUpdatedListener h)
         {
-            std::lock_guard<std::mutex> lock(m_mutexKeyAttributeUpdate);
-            m_keyAttributesUpdatedListeners[key] = std::move(h);
+            std::lock_guard< std::mutex > lock(m_mutexAttributeUpdatedListeners);
+
+            m_attributeUpdatedListeners[key] =
+                    std::make_shared< AttributeUpdatedListener >(std::move(h));
         }
 
         void RCSResourceObject::addAttributeUpdatedListener(std::string&& key,
                 AttributeUpdatedListener h)
         {
-           std::lock_guard<std::mutex> lock(m_mutexKeyAttributeUpdate);
-           m_keyAttributesUpdatedListeners[std::move(key)] = std::move(h);
+            std::lock_guard< std::mutex > lock(m_mutexAttributeUpdatedListeners);
+
+            m_attributeUpdatedListeners[std::move(key)] =
+                    std::make_shared< AttributeUpdatedListener >(std::move(h));
         }
 
         bool RCSResourceObject::removeAttributeUpdatedListener(const std::string& key)
         {
-           std::lock_guard<std::mutex> lock(m_mutexKeyAttributeUpdate);
+            std::lock_guard< std::mutex > lock(m_mutexAttributeUpdatedListeners);
 
-           return m_keyAttributesUpdatedListeners.erase(key) != 0;
+            return m_attributeUpdatedListeners.erase(key) != 0;
         }
 
         bool RCSResourceObject::testValueUpdated(const std::string& key,
@@ -480,35 +484,52 @@ namespace OIC
             return sendResponse(*this, request, invokeHandler(attrs, request, m_getRequestHandler));
         }
 
-        OCEntityHandlerResult RCSResourceObject::handleRequestSet(
-                std::shared_ptr< OC::OCResourceRequest > request)
+        bool RCSResourceObject::applyAcceptanceMethod(const RCSSetResponse& response,
+                const RCSResourceAttributes& requstAttrs)
         {
-            assert(request != nullptr);
-
-            auto attrs = getAttributesFromOCRequest(request);
-            auto response = invokeHandler(attrs, request, m_setRequestHandler);
             auto requestHandler = response.getHandler();
 
             assert(requestHandler != nullptr);
 
-            AttrKeyValuePairs replaced = requestHandler->applyAcceptanceMethod(
-                    response.getAcceptanceMethod(), *this, attrs);
+            auto replaced = requestHandler->applyAcceptanceMethod(response.getAcceptanceMethod(),
+                    *this, requstAttrs);
 
             OC_LOG_V(WARNING, LOG_TAG, "replaced num %d", replaced.size());
             for (const auto& attrKeyValPair : replaced)
             {
-                std::lock_guard<std::mutex> lock(m_mutexKeyAttributeUpdate);
+                std::shared_ptr< AttributeUpdatedListener > foundListener;
+                {
+                    std::lock_guard< std::mutex > lock(m_mutexAttributeUpdatedListeners);
+
+                    auto it = m_attributeUpdatedListeners.find(attrKeyValPair.first);
+                    if (it != m_attributeUpdatedListeners.end())
+                    {
+                        foundListener = it->second;
+                    }
+                }
 
-                auto keyAttrListener = m_keyAttributesUpdatedListeners.find(attrKeyValPair.first);
-                if(keyAttrListener != m_keyAttributesUpdatedListeners.end())
+                if (foundListener)
                 {
-                    keyAttrListener-> second(attrKeyValPair.second, attrs[attrKeyValPair.first]);
+                    (*foundListener)(attrKeyValPair.second, requstAttrs.at(attrKeyValPair.first));
                 }
             }
 
+            return !replaced.empty();
+        }
+
+        OCEntityHandlerResult RCSResourceObject::handleRequestSet(
+                std::shared_ptr< OC::OCResourceRequest > request)
+        {
+            assert(request != nullptr);
+
+            auto attrs = getAttributesFromOCRequest(request);
+            auto response = invokeHandler(attrs, request, m_setRequestHandler);
+
+            auto attrsChanged = applyAcceptanceMethod(response, attrs);
+
             try
             {
-                autoNotify(!replaced.empty(), m_autoNotifyPolicy);
+                autoNotify(attrsChanged, m_autoNotifyPolicy);
                 return sendResponse(*this, request, response);
             } catch (const RCSPlatformException& e) {
                 OC_LOG_V(ERROR, LOG_TAG, "Error : %s ", e.what());