Skip to content

Commit

Permalink
Merge branch 'master' into fix-memory-leak-2
Browse files Browse the repository at this point in the history
  • Loading branch information
thorsten-klein committed Mar 24, 2020
2 parents e61f45d + 9f85f0f commit ac7f4d3
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 8 deletions.
15 changes: 15 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
Changes
=======
v3.1.12.11
- Prevent crash when deregistering managed service (via 'deregisterManagedStub*') and
another service (via 'unregisterService') in parallel with same DBus connection

v3.1.12.10
- Unregister mappings in DBusAddressTranslator (to free memory again) when
services/stubs are unregistered

v3.1.12.9
- Unregister mappings in DBusAddressTranslator (to free memory again) when
proxies are destroyed

v3.1.12.8
- Fixed crash in DBusConnectionTest

v3.1.12.7
- Adapted 'capi-dbus-add-support-for-custom-marshalling.patch' to be compatible with dbus version 1.12.x

Expand Down
3 changes: 3 additions & 0 deletions include/CommonAPI/DBus/DBusAddressTranslator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class DBusAddressTranslator {
*/
COMMONAPI_EXPORT bool isOrgFreedesktopDBusPeerMapped() const;

COMMONAPI_EXPORT void remove(const CommonAPI::Address &_address);

private:
COMMONAPI_EXPORT bool readConfiguration();

Expand All @@ -65,6 +67,7 @@ class DBusAddressTranslator {
std::map<DBusAddress, CommonAPI::Address> backwards_;
std::map<std::string, std::string> compatibility_;
std::map<CommonAPI::Address, std::tuple<std::string, std::string, std::string>> unversioned_;
std::map<CommonAPI::Address, DBusAddress> persistentAddresses_;

std::mutex mutex_;

Expand Down
26 changes: 26 additions & 0 deletions src/CommonAPI/DBus/DBusAddressTranslator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ DBusAddressTranslator::insert(
DBusAddress dbusAddress(_service, _path, _interface);

std::lock_guard<std::mutex> itsLock(mutex_);

persistentAddresses_[address] = dbusAddress;

auto fw = forwards_.find(address);
auto bw = backwards_.find(dbusAddress);
if (fw == forwards_.end() && bw == backwards_.end()) {
Expand Down Expand Up @@ -445,5 +448,28 @@ bool DBusAddressTranslator::isValidVersion(const std::string& _version) const {
return true;
}

void DBusAddressTranslator::remove(const CommonAPI::Address &_address) {
std::lock_guard<std::mutex> itsLock(mutex_);

DBusAddress dbusAddress;

// don't remove persisent addresses
auto persistent_it = persistentAddresses_.find(_address);
if(persistent_it != persistentAddresses_.end()) {
return;
}

auto forwards_it = forwards_.find(_address);
if (forwards_it != forwards_.end()) {
dbusAddress = forwards_it->second;
forwards_.erase(forwards_it->first);
}

auto backwards_it = backwards_.find(dbusAddress);
if (backwards_it != backwards_.end()) {
backwards_.erase(backwards_it->first);
}
}

} // namespace DBus
} // namespace CommonAPI
2 changes: 2 additions & 0 deletions src/CommonAPI/DBus/DBusFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ Factory::unregisterStub(const std::string &_domain, const std::string &_interfac

services_.erase(adapterResult->first);

DBusAddressTranslator::get()->remove(address);

itsLock.unlock();

decrementConnection(connection);
Expand Down
19 changes: 11 additions & 8 deletions src/CommonAPI/DBus/DBusObjectManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ bool DBusObjectManager::registerDBusStubAdapter(std::shared_ptr<DBusStubAdapter>
DBusInterfaceHandlerPath dbusStubAdapterHandlerPath(dbusStubAdapterObjectPath, dbusStubAdapterInterfaceName);
bool isRegistrationSuccessful = false;

objectPathLock_.lock();
std::lock_guard<std::recursive_mutex> itsLock(objectPathLock_);

isRegistrationSuccessful = addDBusInterfaceHandler(dbusStubAdapterHandlerPath, dbusStubAdapter);

if (isRegistrationSuccessful && dbusStubAdapter->hasFreedesktopProperties()) {
Expand Down Expand Up @@ -109,7 +110,6 @@ bool DBusObjectManager::registerDBusStubAdapter(std::shared_ptr<DBusStubAdapter>
dbusConnection->registerObjectPath(dbusStubAdapterObjectPath);
}
}
objectPathLock_.unlock();

return isRegistrationSuccessful;
}
Expand All @@ -121,7 +121,8 @@ bool DBusObjectManager::unregisterDBusStubAdapter(std::shared_ptr<DBusStubAdapte
DBusInterfaceHandlerPath dbusStubAdapterHandlerPath(dbusStubAdapterObjectPath, dbusStubAdapterInterfaceName);
bool isDeregistrationSuccessful = false;

objectPathLock_.lock();
std::lock_guard<std::recursive_mutex> itsLock(objectPathLock_);

isDeregistrationSuccessful = removeDBusInterfaceHandler(dbusStubAdapterHandlerPath, dbusStubAdapter);

if (isDeregistrationSuccessful && dbusStubAdapter->isManaging()) {
Expand Down Expand Up @@ -162,13 +163,13 @@ bool DBusObjectManager::unregisterDBusStubAdapter(std::shared_ptr<DBusStubAdapte
}
}

objectPathLock_.unlock();

return isDeregistrationSuccessful;
}


bool DBusObjectManager::exportManagedDBusStubAdapter(const std::string& parentObjectPath, std::shared_ptr<DBusStubAdapter> dbusStubAdapter) {
std::lock_guard<std::recursive_mutex> itsLock(objectPathLock_);

auto foundManagerStubIterator = managerStubs_.find(parentObjectPath);

if (managerStubs_.end() == foundManagerStubIterator) {
Expand All @@ -182,6 +183,8 @@ bool DBusObjectManager::exportManagedDBusStubAdapter(const std::string& parentOb


bool DBusObjectManager::unexportManagedDBusStubAdapter(const std::string& parentObjectPath, std::shared_ptr<DBusStubAdapter> dbusStubAdapter) {
std::lock_guard<std::recursive_mutex> itsLock(objectPathLock_);

auto foundManagerStubIterator = managerStubs_.find(parentObjectPath);

if (foundManagerStubIterator != managerStubs_.end()) {
Expand All @@ -208,20 +211,20 @@ bool DBusObjectManager::handleMessage(const DBusMessage& dbusMessage) {

DBusInterfaceHandlerPath handlerPath(objectPath, interfaceName);

objectPathLock_.lock();
std::unique_lock<std::recursive_mutex> itsLock(objectPathLock_);

auto handlerIterator = dbusRegisteredObjectsTable_.find(handlerPath);
const bool foundDBusInterfaceHandler = handlerIterator != dbusRegisteredObjectsTable_.end();
bool dbusMessageHandled = false;

if (foundDBusInterfaceHandler) {
std::shared_ptr<DBusInterfaceHandler> dbusStubAdapterBase = handlerIterator->second.front();
objectPathLock_.unlock();
itsLock.unlock();
dbusMessageHandled = dbusStubAdapterBase->onInterfaceDBusMessage(dbusMessage);
return dbusMessageHandled;
} else if (dbusMessage.hasInterfaceName("org.freedesktop.DBus.Introspectable")) {
dbusMessageHandled = onIntrospectableInterfaceDBusMessage(dbusMessage);
}
objectPathLock_.unlock();

return dbusMessageHandled;
}
Expand Down
2 changes: 2 additions & 0 deletions src/CommonAPI/DBus/DBusProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <CommonAPI/DBus/DBusUtils.hpp>
#include <CommonAPI/DBus/DBusProxyAsyncSignalMemberCallbackHandler.hpp>
#include <CommonAPI/DBus/DBusConnection.hpp>
#include <CommonAPI/DBus/DBusAddressTranslator.hpp>
#include <CommonAPI/Logger.hpp>

namespace CommonAPI {
Expand Down Expand Up @@ -216,6 +217,7 @@ DBusProxy::~DBusProxy() {
if (auto ptr = factory__.lock()) {
ptr->decrementConnection(connection_);
}
DBusAddressTranslator::get()->remove(getAddress());
}

bool DBusProxy::isAvailable() const {
Expand Down
1 change: 1 addition & 0 deletions src/test/DBusConnectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ TEST_F(DBusConnectionTest, SendingAsyncDBusMessagesWorks) {
dbusConnection_->disconnect();

interfaceHandlerDBusConnection->unregisterObjectPath(objectPath);
interfaceHandlerDBusConnection->setObjectPathMessageHandler(CommonAPI::DBus::DBusProxyConnection::DBusObjectPathMessageHandler());

ASSERT_TRUE(interfaceHandlerDBusConnection->releaseServiceName(service));
interfaceHandlerDBusConnection->disconnect();
Expand Down

0 comments on commit ac7f4d3

Please sign in to comment.