From 29104467eb05f92c5688477d1d7fdb18da2b04ec Mon Sep 17 00:00:00 2001 From: Philip Rauwolf Date: Fri, 1 Mar 2013 12:49:35 +0100 Subject: [PATCH] Fixed occasional deadlock that occurred when dbus connection got destroyed --- src/CommonAPI/DBus/DBusConnection.cpp | 11 ++++++----- src/CommonAPI/DBus/DBusServiceRegistry.cpp | 21 ++++++++++++--------- src/CommonAPI/DBus/DBusServiceRegistry.h | 4 +++- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/CommonAPI/DBus/DBusConnection.cpp b/src/CommonAPI/DBus/DBusConnection.cpp index e0a32e0..68af24c 100644 --- a/src/CommonAPI/DBus/DBusConnection.cpp +++ b/src/CommonAPI/DBus/DBusConnection.cpp @@ -92,15 +92,16 @@ bool DBusConnection::connect(DBusError& dbusError) { void DBusConnection::disconnect() { if (isConnected()) { - stopDispatching_ = true; - if (!dbusSignalMatchRulesMap_.empty()) { dbus_connection_remove_filter(libdbusConnection_, &onLibdbusSignalFilterThunk, this); } + dbus_connection_close(libdbusConnection_); + + stopDispatching_ = true; + dispatchThread_.join(); - dbus_connection_close(libdbusConnection_); dbus_connection_unref(libdbusConnection_); libdbusConnection_ = NULL; @@ -119,8 +120,8 @@ DBusProxyConnection::ConnectionStatusEvent& DBusConnection::getConnectionStatusE const std::shared_ptr DBusConnection::getDBusServiceRegistry() { std::shared_ptr serviceRegistry = dbusServiceRegistry_.lock(); if (!serviceRegistry) { - auto blub = this->shared_from_this(); - serviceRegistry = std::make_shared(blub); + serviceRegistry = std::make_shared(this->shared_from_this()); + serviceRegistry->init(); dbusServiceRegistry_ = serviceRegistry; } diff --git a/src/CommonAPI/DBus/DBusServiceRegistry.cpp b/src/CommonAPI/DBus/DBusServiceRegistry.cpp index f883797..a54116c 100644 --- a/src/CommonAPI/DBus/DBusServiceRegistry.cpp +++ b/src/CommonAPI/DBus/DBusServiceRegistry.cpp @@ -16,25 +16,28 @@ namespace DBus { DBusServiceRegistry::DBusServiceRegistry(std::shared_ptr dbusProxyConnection): dbusDaemonProxy_(std::make_shared(dbusProxyConnection)), dbusServicesStatus_(AvailabilityStatus::UNKNOWN) { +} + +DBusServiceRegistry::~DBusServiceRegistry() { + dbusDaemonProxy_->getNameOwnerChangedEvent().unsubscribe(dbusDaemonProxyNameOwnerChangedEventSubscription_); + dbusDaemonProxy_->getProxyStatusEvent().unsubscribe(dbusDaemonProxyStatusEventSubscription_); + std::cout << "Crushing stuff" << std::endl; +} +void DBusServiceRegistry::init() { dbusDaemonProxyStatusEventSubscription_ = dbusDaemonProxy_->getProxyStatusEvent().subscribeCancellableListener( - std::bind(&DBusServiceRegistry::onDBusDaemonProxyStatusEvent, this, std::placeholders::_1)); + std::bind(&DBusServiceRegistry::onDBusDaemonProxyStatusEvent, this->shared_from_this(), std::placeholders::_1)); dbusDaemonProxyNameOwnerChangedEventSubscription_ = dbusDaemonProxy_->getNameOwnerChangedEvent().subscribeCancellableListener( std::bind(&DBusServiceRegistry::onDBusDaemonProxyNameOwnerChangedEvent, - this, + this->shared_from_this(), std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); } -DBusServiceRegistry::~DBusServiceRegistry() { - dbusDaemonProxy_->getNameOwnerChangedEvent().unsubscribe(dbusDaemonProxyNameOwnerChangedEventSubscription_); - dbusDaemonProxy_->getProxyStatusEvent().unsubscribe(dbusDaemonProxyStatusEventSubscription_); -} - bool DBusServiceRegistry::waitDBusServicesAvailable(std::unique_lock& lock, std::chrono::milliseconds& timeout) { bool dbusServicesStatusIsKnown = (dbusServicesStatus_ != AvailabilityStatus::UNKNOWN); @@ -324,7 +327,7 @@ SubscriptionStatus DBusServiceRegistry::onDBusDaemonProxyStatusEvent(const Avail dbusServicesStatus_ = AvailabilityStatus::UNKNOWN; dbusDaemonProxy_->listNamesAsync(std::bind( &DBusServiceRegistry::onListNamesCallback, - this, + this->shared_from_this(), std::placeholders::_1, std::placeholders::_2)); break; @@ -478,7 +481,7 @@ void DBusServiceRegistry::resolveDBusServiceInstances(DBusServiceList::iterator& // search for remote instances DBusDaemonProxy::GetManagedObjectsAsyncCallback callback = std::bind(&DBusServiceRegistry::onGetManagedObjectsCallback, - this, + this->shared_from_this(), std::placeholders::_1, std::placeholders::_2, dbusServiceName); diff --git a/src/CommonAPI/DBus/DBusServiceRegistry.h b/src/CommonAPI/DBus/DBusServiceRegistry.h index 1db2dfd..510c339 100644 --- a/src/CommonAPI/DBus/DBusServiceRegistry.h +++ b/src/CommonAPI/DBus/DBusServiceRegistry.h @@ -42,7 +42,7 @@ class DBusProxyConnection; class DBusDaemonProxy; -class DBusServiceRegistry { +class DBusServiceRegistry: public std::enable_shared_from_this { public: enum class DBusServiceState { UNKNOWN, @@ -65,6 +65,8 @@ class DBusServiceRegistry { virtual ~DBusServiceRegistry(); + void init(); + bool isServiceInstanceAlive(const std::string& dbusInterfaceName, const std::string& dbusConnectionName, const std::string& dbusObjectPath); Subscription subscribeAvailabilityListener(const std::string& commonApiAddress, -- 2.7.4