CommonAPI-D-Bus 3.1.10.13.1.10.1

6 files changed, 33 insertions, 30 deletions

diff --git a/CHANGES b/CHANGES
index a1ba61b..cc589e1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
 Changes
 =======
 
+v3.1.10.1
+- Fix deadlock in connection: If data is currently dispatched and disconnecting tooks place it waits until disconnecting is finished. The problem is that the 'connectionGuard_' mutex is locked which is tried to be locked in 'sendDBusMessageWithReplyAsync' as well. This leads to deadlock and is fixed by removing the 'dispatchMutex' and instead of this the 'connectionGuard_' mutex is now used.
+- Fix availability listeners: Use an incrementing index as an identifier for subscriptions instead of iterators to a list that will get modified. Store the receiver proxy with each listener, instead of assuming that all proxies that connect to an interface are the same.
+- Avoid seg fault when creating a connection failed.
+
 v3.1.10
 - If an 'DBusInstanceAvailabilityStatusChangedEvent' occurs and the manager proxy was deleted, a bad_weak_ptr occurs. This is now avoided by introducing a weak_ptr. If the weak_ptr can be locked, the manager proxy was not deleted yet.
 - Moved mutex lock in 'DBusMainLoop::registerWatch' to avoid deadlock.
@@ -13,8 +18,3 @@ v3.1.10
 - Fixed THMainloopIndependence and THMainloopIntegration segfault. The problem is that a 'DBusQueueDispatchSource' has a pointer to its related 'DBusQueueWatch' as member variable and in the destructor of the dispatch source the watch is accessed. So a segfault can occur when the watch is deleted before the dispatch source.
 - Replaced polling mechanism in 'DBusMainLoop' from sockets to WSAEvents in combination with named pipes.
 - Added support for the DBus message type 'Error'. This needed the introduction of a new class ('DBusErrorEvent'), adaptions on proxy side ('DBusProxyHelper') and on stub side ('DBusStubAdapterHelper') and a libdbus patch that avoids the deletion of a message reply when an error occured / when an error reply was sent. This is necessary to deserialize the arguments of the error reply.
-
-
-
-
-
diff --git a/include/CommonAPI/DBus/DBusConnection.hpp b/include/CommonAPI/DBus/DBusConnection.hpp
index acb163f..b5ec156 100644
--- a/include/CommonAPI/DBus/DBusConnection.hpp
+++ b/include/CommonAPI/DBus/DBusConnection.hpp
@@ -386,7 +386,6 @@ public:
     bool isWaitingOnFinishedDispatching_;
 
     std::set<std::thread::id> dispatchThreads_;
-    std::mutex dispatchMutex_;
     std::condition_variable dispatchCondition_;
 };
 
diff --git a/include/CommonAPI/DBus/DBusServiceRegistry.hpp b/include/CommonAPI/DBus/DBusServiceRegistry.hpp
index 8a56a91..497e909 100644
--- a/include/CommonAPI/DBus/DBusServiceRegistry.hpp
+++ b/include/CommonAPI/DBus/DBusServiceRegistry.hpp
@@ -59,8 +59,12 @@ class DBusServiceRegistry: public std::enable_shared_from_this<DBusServiceRegist
 
     // template class DBusServiceListener<> { typedef functor; typedef list; typedef subscription }
     typedef std::function<void(std::shared_ptr<DBusProxy>, const AvailabilityStatus& availabilityStatus)> DBusServiceListener;
-    typedef std::list<DBusServiceListener> DBusServiceListenerList;
-    typedef DBusServiceListenerList::iterator DBusServiceSubscription;
+    typedef long int DBusServiceSubscription;
+    struct DBusServiceListenerInfo {
+        DBusServiceListener listener;
+        std::weak_ptr<DBusProxy> proxy;
+    };
+    typedef std::map<DBusServiceSubscription, std::shared_ptr<DBusServiceListenerInfo>> DBusServiceListenerList;
 
     typedef std::function<void(const std::vector<std::string>& interfaces,
                                const AvailabilityStatus& availabilityStatus)> DBusManagedInterfaceListener;
@@ -109,20 +113,21 @@ class DBusServiceRegistry: public std::enable_shared_from_this<DBusServiceRegist
  private:
     struct DBusInterfaceNameListenersRecord {
         DBusInterfaceNameListenersRecord()
-            : state(DBusRecordState::UNKNOWN) {
+            : state(DBusRecordState::UNKNOWN),
+              nextSubscriptionKey(0) {
         }
 
         DBusInterfaceNameListenersRecord(DBusInterfaceNameListenersRecord &&_other)
             : state(_other.state),
               listenerList(std::move(_other.listenerList)),
               listenersToRemove(std::move(_other.listenersToRemove)),
-              proxy(_other.proxy){
+              nextSubscriptionKey(_other.nextSubscriptionKey){
         }
 
         DBusRecordState state;
         DBusServiceListenerList listenerList;
         std::list<DBusServiceSubscription> listenersToRemove;
-        std::weak_ptr<DBusProxy> proxy;
+        DBusServiceSubscription nextSubscriptionKey;
     };
 
     typedef std::unordered_map<std::string, DBusInterfaceNameListenersRecord> DBusInterfaceNameListenersMap;
diff --git a/src/CommonAPI/DBus/DBusConnection.cpp b/src/CommonAPI/DBus/DBusConnection.cpp
index f34ff70..d33847a 100644
--- a/src/CommonAPI/DBus/DBusConnection.cpp
+++ b/src/CommonAPI/DBus/DBusConnection.cpp
@@ -419,9 +419,7 @@ bool DBusConnection::connect(DBusError &dbusError, bool startDispatchThread) {
 }
 
 void DBusConnection::disconnect() {
-    std::lock_guard<std::mutex> dbusConnectionLock(connectionGuard_);
-
-    std::unique_lock<std::mutex> dispatchLock(dispatchMutex_);
+    std::unique_lock<std::mutex> dbusConnectionLock(connectionGuard_);
 
     std::shared_ptr<DBusServiceRegistry> itsRegistry = DBusServiceRegistry::get(shared_from_this());
 
@@ -446,11 +444,11 @@ void DBusConnection::disconnect() {
         if(it == dispatchThreads_.end()) { //wait only if disconnect is NOT triggered by main loop
             while(isDispatching_) {
                 isWaitingOnFinishedDispatching_ = true;
-                dispatchCondition_.wait(dispatchLock);
+                dispatchCondition_.wait(dbusConnectionLock);
                 isWaitingOnFinishedDispatching_ = false;
             }
         }
-        dispatchLock.unlock();
+        dbusConnectionLock.unlock();
 
         dbus_connection_close(connection_);
 
@@ -988,7 +986,7 @@ void DBusConnection::decrementConnection() {
 }
 
 bool DBusConnection::setDispatching(bool _isDispatching) {
-    std::lock_guard<std::mutex> dispatchLock(dispatchMutex_);
+    std::lock_guard<std::mutex> dispatchLock(connectionGuard_);
 
     if(isDispatching_ == _isDispatching)
         return true;
diff --git a/src/CommonAPI/DBus/DBusFactory.cpp b/src/CommonAPI/DBus/DBusFactory.cpp
index 1c05455..cde564e 100644
--- a/src/CommonAPI/DBus/DBusFactory.cpp
+++ b/src/CommonAPI/DBus/DBusFactory.cpp
@@ -286,7 +286,8 @@ Factory::getConnection(const ConnectionId_t &_connectionId) {
         }
     }
 
-    incrementConnection(itsConnection);
+    if(itsConnection)
+        incrementConnection(itsConnection);
 
     return itsConnection;
 }
diff --git a/src/CommonAPI/DBus/DBusServiceRegistry.cpp b/src/CommonAPI/DBus/DBusServiceRegistry.cpp
index dc7daa8..7ad5987 100644
--- a/src/CommonAPI/DBus/DBusServiceRegistry.cpp
+++ b/src/CommonAPI/DBus/DBusServiceRegistry.cpp
@@ -166,6 +166,7 @@ DBusServiceRegistry::subscribeAvailabilityListener(
     }
 
     // LB TODO: check this as it looks STRANGE!!!
+
     if (availabilityStatus != AvailabilityStatus::UNKNOWN) {
         notificationThread_ = std::this_thread::get_id();
         if(auto itsProxy = _proxy.lock())
@@ -173,14 +174,16 @@ DBusServiceRegistry::subscribeAvailabilityListener(
         notificationThread_ = std::thread::id();
     }
 
-    dbusInterfaceNameListenersRecord.listenerList.push_front(std::move(serviceListener));
-    dbusInterfaceNameListenersRecord.proxy = _proxy;
-    dbusInterfaceNameListenersRecord.listenersToRemove.remove(
-            dbusInterfaceNameListenersRecord.listenerList.begin());
 
+    DBusServiceSubscription subscriptionKey = dbusInterfaceNameListenersRecord.nextSubscriptionKey++;
+    std::shared_ptr<DBusServiceListenerInfo> info = std::make_shared<DBusServiceListenerInfo>();
+    info->listener = std::move(serviceListener);
+    info->proxy = _proxy;
+    dbusInterfaceNameListenersRecord.listenerList.insert(std::make_pair(subscriptionKey, info));
+    dbusInterfaceNameListenersRecord.listenersToRemove.remove(subscriptionKey);
     dbusServicesMutex_.unlock();
 
-    return dbusInterfaceNameListenersRecord.listenerList.begin();
+    return subscriptionKey;
 }
 
 void
@@ -978,7 +981,6 @@ void DBusServiceRegistry::onDBusServiceNotAvailable(DBusServiceListenersRecord&
     for (auto dbusObjectPathListenersIterator = dbusServiceListenersRecord.dbusObjectPathListenersMap.begin();
                     dbusObjectPathListenersIterator != dbusServiceListenersRecord.dbusObjectPathListenersMap.end(); ) {
         auto& dbusInterfaceNameListenersMap = dbusObjectPathListenersIterator->second;
-
         notifyDBusObjectPathResolved(dbusInterfaceNameListenersMap, dbusInterfaceNamesCache);
 
         if (dbusInterfaceNameListenersMap.empty()) {
@@ -1041,7 +1043,6 @@ void DBusServiceRegistry::notifyDBusObjectPathResolved(DBusInterfaceNameListener
 
         const auto& dbusInterfaceNameIterator = dbusInterfaceNames.find(listenersDBusInterfaceName);
         const bool isDBusInterfaceNameAvailable = (dbusInterfaceNameIterator != dbusInterfaceNames.end());
-
         notifyDBusInterfaceNameListeners(dbusInterfaceNameListenersRecord, isDBusInterfaceNameAvailable);
 
         if (dbusInterfaceNameListenersRecord.listenerList.empty()) {
@@ -1068,7 +1069,6 @@ void DBusServiceRegistry::notifyDBusObjectPathChanged(DBusInterfaceNameListeners
 
         if (isDBusInterfaceNameObserved) {
             auto& dbusInterfaceNameListenersRecord = dbusInterfaceNameListenersIterator->second;
-
             notifyDBusInterfaceNameListeners(dbusInterfaceNameListenersRecord, isDBusInterfaceNameAvailable);
 
             if (dbusInterfaceNameListenersRecord.listenerList.empty())
@@ -1094,14 +1094,14 @@ void DBusServiceRegistry::notifyDBusInterfaceNameListeners(DBusInterfaceNameList
 
         auto itsRemoveListenerIt = std::find(dbusInterfaceNameListenersRecord.listenersToRemove.begin(),
                   dbusInterfaceNameListenersRecord.listenersToRemove.end(),
-                  dbusServiceListenerIterator);
+                  dbusServiceListenerIterator->first);
 
         if(itsRemoveListenerIt != dbusInterfaceNameListenersRecord.listenersToRemove.end()) {
-            dbusInterfaceNameListenersRecord.listenersToRemove.remove(dbusServiceListenerIterator);
+            dbusInterfaceNameListenersRecord.listenersToRemove.remove(dbusServiceListenerIterator->first);
             dbusServiceListenerIterator = dbusInterfaceNameListenersRecord.listenerList.erase(dbusServiceListenerIterator);
         } else {
-            if(auto itsProxy = dbusInterfaceNameListenersRecord.proxy.lock())
-                (*dbusServiceListenerIterator)(itsProxy, availabilityStatus);
+            if(auto itsProxy = dbusServiceListenerIterator->second->proxy.lock())
+                (dbusServiceListenerIterator->second->listener)(itsProxy, availabilityStatus);
             ++dbusServiceListenerIterator;
         }
     }