From 3372155b32a2cf3b05e1a2a13f6f8413069de33f Mon Sep 17 00:00:00 2001 From: Juergen Gehring Date: Tue, 29 Jul 2014 17:00:43 +0200 Subject: 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. Change-Id: I026721e9b672f1d4732a6b6bf62c43a90ce75446 --- src/CommonAPI/DBus/DBusConnection.cpp | 106 +++++++++++++++++++------------ src/CommonAPI/DBus/DBusProxyConnection.h | 3 +- 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 handlerList; + handlerList.insert(dbusSignalHandler); + + dbusSignalHandlerTable_.insert({dbusSignalHandlerPath, + std::make_pair(std::make_shared(), 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 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)); + 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 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 +void notifyDBusOMSignalHandlers(DBusSignalHandlersTable& dbusSignalHandlerstable, std::pair& 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 #include #include +#include #include #include @@ -59,7 +60,7 @@ class DBusProxyConnection { // objectPath, interfaceName, interfaceMemberName, interfaceMemberSignature typedef std::tuple DBusSignalHandlerPath; - typedef std::unordered_multimap DBusSignalHandlerTable; + typedef std::unordered_map, std::set>> DBusSignalHandlerTable; typedef DBusSignalHandlerPath DBusSignalHandlerToken; typedef Event ConnectionStatusEvent; -- cgit v1.2.1