Bug fix: unsubscribe does not work in event callback (e.g. availability).\n Data structure DBusSignalHandlerTable in DBusProxyConnection.h changed.\n Changes in DBusConnection.cpp for signal handler functions.\n No influence on any APIs.2.1.6-p1

Change-Id: I026721e9b672f1d4732a6b6bf62c43a90ce75446

2 files changed, 66 insertions, 43 deletions

diff --git a/src/CommonAPI/DBus/DBusConnection.cpp b/src/CommonAPI/DBus/DBusConnection.cpp
index 1d8fd38..6deb376 100644
--- a/src/CommonAPI/DBus/DBusConnection.cpp
+++ b/src/CommonAPI/DBus/DBusConnection.cpp
@@ -698,12 +698,22 @@ DBusProxyConnection::DBusSignalHandlerToken DBusConnection::addSignalMemberHandl
                     interfaceMemberName,
                     interfaceMemberSignature);
     std::lock_guard < std::mutex > dbusSignalLock(signalGuard_);
-    const bool isFirstSignalMemberHandler = dbusSignalHandlerTable_.find(dbusSignalHandlerPath)
-                    == dbusSignalHandlerTable_.end();
-    dbusSignalHandlerTable_.insert(DBusSignalHandlerTable::value_type(dbusSignalHandlerPath, dbusSignalHandler));
+    auto signalEntry = dbusSignalHandlerTable_.find(dbusSignalHandlerPath);
+    const bool isFirstSignalMemberHandler = (signalEntry == dbusSignalHandlerTable_.end());
 
     if (isFirstSignalMemberHandler) {
         addLibdbusSignalMatchRule(objectPath, interfaceName, interfaceMemberName, justAddFilter);
+        std::set<DBusSignalHandler*> handlerList;
+        handlerList.insert(dbusSignalHandler);
+
+        dbusSignalHandlerTable_.insert({dbusSignalHandlerPath,
+            std::make_pair(std::make_shared<std::mutex>(), std::move(handlerList)) } );
+
+    } else {
+
+        signalEntry->second.first->lock();
+        signalEntry->second.second.insert(dbusSignalHandler);
+        signalEntry->second.first->unlock();
     }
 
     return dbusSignalHandlerPath;
@@ -713,41 +723,18 @@ bool DBusConnection::removeSignalMemberHandler(const DBusSignalHandlerToken& dbu
                                                const DBusSignalHandler* dbusSignalHandler) {
     bool lastHandlerRemoved = false;
 
-    std::lock_guard<std::mutex> dbusSignalLock(signalGuard_);
-    auto equalRangeIteratorPair = dbusSignalHandlerTable_.equal_range(dbusSignalHandlerToken);
-    if (equalRangeIteratorPair.first != equalRangeIteratorPair.second) {
-        // advance to the next element
-        auto iteratorToNextElement = equalRangeIteratorPair.first;
-        iteratorToNextElement++;
-
-        // check if the first element was the only element
-        const bool isLastSignalMemberHandler = iteratorToNextElement == equalRangeIteratorPair.second;
+    auto signalEntry = dbusSignalHandlerTable_.find(dbusSignalHandlerToken);
+    if (signalEntry != dbusSignalHandlerTable_.end()) {
 
-        if (isLastSignalMemberHandler) {
-            const std::string& objectPath = std::get<0>(dbusSignalHandlerToken);
-            const std::string& interfaceName = std::get<1>(dbusSignalHandlerToken);
-            const std::string& interfaceMemberName = std::get<2>(dbusSignalHandlerToken);
+        signalEntry->second.first->lock();
 
-            removeLibdbusSignalMatchRule(objectPath, interfaceName, interfaceMemberName);
-            lastHandlerRemoved = true;
-        }
-
-        if(dbusSignalHandler == NULL) {
-            // remove all handlers
-            dbusSignalHandlerTable_.erase(dbusSignalHandlerToken);
-        } else {
-            // just remove specific handler
-            while(equalRangeIteratorPair.first != equalRangeIteratorPair.second) {
-                if(equalRangeIteratorPair.first->second == dbusSignalHandler) {
-                    equalRangeIteratorPair.first = dbusSignalHandlerTable_.erase(equalRangeIteratorPair.first);
-                }
-                else {
-                    equalRangeIteratorPair.first++;
-                }
-            }
+        auto selectedHandler = signalEntry->second.second.find(const_cast<DBusSignalHandler*>(dbusSignalHandler));
+        if (selectedHandler != signalEntry->second.second.end()) {
+            signalEntry->second.second.erase(selectedHandler);
+            lastHandlerRemoved = (signalEntry->second.second.empty());
         }
+        signalEntry->second.first->unlock();
     }
-
     return lastHandlerRemoved;
 }
 
@@ -864,6 +851,7 @@ bool DBusConnection::addLibdbusSignalMatchRule(const std::string& dbusMatchRule)
 
     // add the libdbus message signal filter
     if (!libdbusSignalMatchRulesCount_) {
+
         libdbusSuccess = (bool) dbus_connection_add_filter(
             libdbusConnection_,
             &onLibdbusSignalFilterThunk,
@@ -894,7 +882,7 @@ bool DBusConnection::addLibdbusSignalMatchRule(const std::string& dbusMatchRule)
  * @return
  */
 bool DBusConnection::removeLibdbusSignalMatchRule(const std::string& dbusMatchRule) {
-    //assert(libdbusSignalMatchRulesCount_ > 0);
+
     if(libdbusSignalMatchRulesCount_ == 0)
         return true;
 
@@ -999,10 +987,12 @@ void DBusConnection::addLibdbusSignalMatchRule(const std::string& objectPath,
     assert(success.second);
 
     if (isConnected()) {
+        bool libdbusSuccess = true;
         suspendDispatching();
         // add the libdbus message signal filter
         if (isFirstMatchRule) {
-            const dbus_bool_t libdbusSuccess = dbus_connection_add_filter(libdbusConnection_,
+
+            libdbusSuccess = dbus_connection_add_filter(libdbusConnection_,
                             &onLibdbusSignalFilterThunk,
                             this,
                             NULL);
@@ -1017,6 +1007,10 @@ void DBusConnection::addLibdbusSignalMatchRule(const std::string& objectPath,
             assert(!dbusError);
         }
 
+        if (libdbusSuccess) {
+            libdbusSignalMatchRulesCount_++;
+        }
+
         resumeDispatching();
     }
 }
@@ -1107,6 +1101,34 @@ void DBusConnection::initLibdbusSignalFilterAfterConnect() {
 
 template<typename DBusSignalHandlersTable>
 void notifyDBusSignalHandlers(DBusSignalHandlersTable& dbusSignalHandlerstable,
+                              typename DBusSignalHandlersTable::iterator& signalEntry,
+                              const CommonAPI::DBus::DBusMessage& dbusMessage,
+                              ::DBusHandlerResult& dbusHandlerResult) {
+    if (signalEntry == dbusSignalHandlerstable.end() || signalEntry->second.second.empty()) {
+        dbusHandlerResult = DBUS_HANDLER_RESULT_HANDLED;
+        return;
+    }
+
+    signalEntry->second.first->lock();
+
+    auto handlerEntry = signalEntry->second.second.begin();
+    while (handlerEntry != signalEntry->second.second.end()) {
+        DBusProxyConnection::DBusSignalHandler* dbusSignalHandler = *handlerEntry;
+
+        auto dbusSignalHandlerSubscriptionStatus = dbusSignalHandler->onSignalDBusMessage(dbusMessage);
+
+        if (dbusSignalHandlerSubscriptionStatus == SubscriptionStatus::CANCEL) {
+            handlerEntry = signalEntry->second.second.erase(handlerEntry);
+        } else {
+            handlerEntry++;
+        }
+    }
+    dbusHandlerResult = DBUS_HANDLER_RESULT_HANDLED;
+    signalEntry->second.first->unlock();
+}
+
+template<typename DBusSignalHandlersTable>
+void notifyDBusOMSignalHandlers(DBusSignalHandlersTable& dbusSignalHandlerstable,
                               std::pair<typename DBusSignalHandlersTable::iterator,
                                         typename DBusSignalHandlersTable::iterator>& equalRange,
                               const CommonAPI::DBus::DBusMessage& dbusMessage,
@@ -1152,24 +1174,24 @@ void notifyDBusSignalHandlers(DBusSignalHandlersTable& dbusSignalHandlerstable,
     ::DBusHandlerResult dbusHandlerResult = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
     signalGuard_.lock();
-    auto dbusSignalHandlerIteratorPair = dbusSignalHandlerTable_.equal_range(DBusSignalHandlerPath(
+    auto signalEntry = dbusSignalHandlerTable_.find(DBusSignalHandlerPath(
         objectPath,
         interfaceName,
         interfaceMemberName,
         interfaceMemberSignature));
-    notifyDBusSignalHandlers(dbusSignalHandlerTable_,
-                             dbusSignalHandlerIteratorPair,
-                             dbusMessage,
-                             dbusHandlerResult);
     signalGuard_.unlock();
 
+    notifyDBusSignalHandlers(dbusSignalHandlerTable_,
+                                    signalEntry, dbusMessage, dbusHandlerResult);
+
+
     if (dbusMessage.hasInterfaceName("org.freedesktop.DBus.ObjectManager")) {
         const char* dbusSenderName = dbusMessage.getSenderName();
         assert(dbusSenderName);
 
         dbusObjectManagerSignalGuard_.lock();
         auto dbusObjectManagerSignalHandlerIteratorPair = dbusObjectManagerSignalHandlerTable_.equal_range(dbusSenderName);
-        notifyDBusSignalHandlers(dbusObjectManagerSignalHandlerTable_,
+        notifyDBusOMSignalHandlers(dbusObjectManagerSignalHandlerTable_,
                                  dbusObjectManagerSignalHandlerIteratorPair,
                                  dbusMessage,
                                  dbusHandlerResult);
diff --git a/src/CommonAPI/DBus/DBusProxyConnection.h b/src/CommonAPI/DBus/DBusProxyConnection.h
index d7c88d8..43c370e 100644
--- a/src/CommonAPI/DBus/DBusProxyConnection.h
+++ b/src/CommonAPI/DBus/DBusProxyConnection.h
@@ -27,6 +27,7 @@
 #include <memory>
 #include <tuple>
 #include <unordered_map>
+#include <set>
 #include <utility>
 #include <vector>
 
@@ -59,7 +60,7 @@ class DBusProxyConnection {
 
     // objectPath, interfaceName, interfaceMemberName, interfaceMemberSignature
     typedef std::tuple<std::string, std::string, std::string, std::string> DBusSignalHandlerPath;
-    typedef std::unordered_multimap<DBusSignalHandlerPath, DBusSignalHandler*> DBusSignalHandlerTable;
+    typedef std::unordered_map<DBusSignalHandlerPath, std::pair<std::shared_ptr<std::mutex>, std::set<DBusSignalHandler* >>> DBusSignalHandlerTable;
     typedef DBusSignalHandlerPath DBusSignalHandlerToken;
 
     typedef Event<AvailabilityStatus> ConnectionStatusEvent;