From beac4410335320edca662ad9f0b2f3a625ca8a17 Mon Sep 17 00:00:00 2001 From: Philip Rauwolf Date: Mon, 4 Mar 2013 15:56:00 +0100 Subject: Prevented self-joining of the DBusConnection's dispatch thread on destruction --- src/CommonAPI/DBus/DBusConnection.cpp | 28 ++++++++++++++++++++-------- src/CommonAPI/DBus/DBusConnection.h | 8 ++++---- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/CommonAPI/DBus/DBusConnection.cpp b/src/CommonAPI/DBus/DBusConnection.cpp index 68af24c..1111cf5 100644 --- a/src/CommonAPI/DBus/DBusConnection.cpp +++ b/src/CommonAPI/DBus/DBusConnection.cpp @@ -72,8 +72,9 @@ bool DBusConnection::connect(DBusError& dbusError) { const ::DBusBusType libdbusType = static_cast(busType_); libdbusConnection_ = dbus_bus_get_private(libdbusType, &dbusError.libdbusError_); - if (dbusError) + if (dbusError) { return false; + } assert(libdbusConnection_); dbus_connection_set_exit_on_disconnect(libdbusConnection_, false); @@ -96,11 +97,17 @@ void DBusConnection::disconnect() { dbus_connection_remove_filter(libdbusConnection_, &onLibdbusSignalFilterThunk, this); } - dbus_connection_close(libdbusConnection_); - stopDispatching_ = true; - dispatchThread_.join(); + dbus_connection_close(libdbusConnection_); + + //It is possible for the disconnect to be called from within a callback, i.e. from within the dispatch + //thread. Self-join is prevented this way. + if (dispatchThread_.joinable() && std::this_thread::get_id() != dispatchThread_.get_id()) { + dispatchThread_.join(); + } else { + dispatchThread_.detach(); + } dbus_connection_unref(libdbusConnection_); libdbusConnection_ = NULL; @@ -119,7 +126,7 @@ DBusProxyConnection::ConnectionStatusEvent& DBusConnection::getConnectionStatusE const std::shared_ptr DBusConnection::getDBusServiceRegistry() { std::shared_ptr serviceRegistry = dbusServiceRegistry_.lock(); - if (!serviceRegistry) { + if (!serviceRegistry || dbusServiceRegistry_.expired()) { serviceRegistry = std::make_shared(this->shared_from_this()); serviceRegistry->init(); dbusServiceRegistry_ = serviceRegistry; @@ -269,8 +276,9 @@ DBusProxyConnection::DBusSignalHandlerToken DBusConnection::addSignalMemberHandl dbusSignalHandlerTable_.insert(DBusSignalHandlerTable::value_type(dbusSignalHandlerPath, dbusSignalHandler)); - if (isFirstSignalMemberHandler) + if (isFirstSignalMemberHandler) { addLibdbusSignalMatchRule(objectPath, interfaceName, interfaceMemberName); + } return dbusSignalHandlerPath; } @@ -402,6 +410,7 @@ void DBusConnection::addLibdbusSignalMatchRule(const std::string& objectPath, void DBusConnection::removeLibdbusSignalMatchRule(const std::string& objectPath, const std::string& interfaceName, const std::string& interfaceMemberName) { + auto selfReference = this->shared_from_this(); DBusSignalMatchRuleTuple dbusSignalMatchRuleTuple(objectPath, interfaceName, interfaceMemberName); auto matchRuleIterator = dbusSignalMatchRulesMap_.find(dbusSignalMatchRuleTuple); const bool matchRuleFound = matchRuleIterator != dbusSignalMatchRulesMap_.end(); @@ -422,8 +431,9 @@ void DBusConnection::removeLibdbusSignalMatchRule(const std::string& objectPath, dbusSignalMatchRulesMap_.erase(matchRuleIterator); const bool isLastMatchRule = dbusSignalMatchRulesMap_.empty(); - if (isLastMatchRule) + if (isLastMatchRule) { dbus_connection_remove_filter(libdbusConnection_, &onLibdbusSignalFilterThunk, this); + } } void DBusConnection::initLibdbusObjectPathHandlerAfterConnect() { @@ -492,6 +502,7 @@ void DBusConnection::initLibdbusSignalFilterAfterConnect() { ::DBusHandlerResult DBusConnection::onLibdbusSignalFilter(::DBusMessage* libdbusMessage) { assert(libdbusMessage); + auto selfReference = this->shared_from_this(); // handle only signal messages if (dbus_message_get_type(libdbusMessage) != DBUS_MESSAGE_TYPE_SIGNAL) @@ -521,8 +532,9 @@ void DBusConnection::initLibdbusSignalFilterAfterConnect() { auto dbusSignalHandlerSubscription = equalRangeIteratorPair.first; equalRangeIteratorPair.first++; dbusSignalHandlerTable_.erase(dbusSignalHandlerSubscription); - } else + } else { equalRangeIteratorPair.first++; + } } return DBUS_HANDLER_RESULT_HANDLED; diff --git a/src/CommonAPI/DBus/DBusConnection.h b/src/CommonAPI/DBus/DBusConnection.h index 6337b9c..1c1e6a7 100644 --- a/src/CommonAPI/DBus/DBusConnection.h +++ b/src/CommonAPI/DBus/DBusConnection.h @@ -42,6 +42,8 @@ class DBusConnection: public DBusProxyConnection, public std::enable_shared_from WRAPPED }; + DBusConnection(BusType busType); + inline static std::shared_ptr getBus(const BusType& busType); inline static std::shared_ptr wrapLibDBus(::DBusConnection* libDbusConnection); inline static std::shared_ptr getSessionBus(); @@ -102,8 +104,6 @@ class DBusConnection: public DBusProxyConnection, public std::enable_shared_from std::thread dispatchThread_; bool stopDispatching_; - DBusConnection(BusType busType); - void addLibdbusSignalMatchRule(const std::string& objectPath, const std::string& interfaceName, const std::string& interfaceMemberName); @@ -157,11 +157,11 @@ class DBusConnection: public DBusProxyConnection, public std::enable_shared_from }; std::shared_ptr DBusConnection::getBus(const BusType& busType) { - return std::shared_ptr(new DBusConnection(busType)); + return std::make_shared(busType); } std::shared_ptr DBusConnection::wrapLibDBus(::DBusConnection* libDbusConnection) { - return std::shared_ptr(new DBusConnection(libDbusConnection)); + return std::make_shared(libDbusConnection); } std::shared_ptr DBusConnection::getSessionBus() { -- cgit v1.2.1