From a14abe9c1dfced9822fc03d9c2622e338566c796 Mon Sep 17 00:00:00 2001 From: Andrea Ricchi Date: Wed, 1 Jul 2026 16:18:25 +0200 Subject: [PATCH 1/2] build: use CMAKE_CURRENT_SOURCE_DIR for LICENSE install Replaced CMAKE_SOURCE_DIR with CMAKE_CURRENT_SOURCE_DIR when specifying the path to the LICENSE file for installation. Using CMAKE_SOURCE_DIR breaks the installation if this project is included as a subproject (e.g., via add_subdirectory) in a larger CMake build, because it would resolve to the top-level parent project's root directory. CMAKE_CURRENT_SOURCE_DIR ensures the LICENSE file is correctly resolved relative to this specific CMakeLists.txt file. Gh-Issue: #40 Signed-off-by: Andrea Ricchi --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c6a281..1c3cc6b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,7 @@ install( NAMESPACE Amarula:: COMPONENT ${PROJECT_NAME}-dev) install( - FILES ${CMAKE_SOURCE_DIR}/LICENSE + FILES ${CMAKE_CURRENT_SOURCE_DIR}/LICENSE DESTINATION ${CMAKE_INSTALL_DATADIR}/Amarula/${PROJECT_NAME} COMPONENT ${PROJECT_NAME}) include(CMakePackageConfigHelpers) From a32c5e7f578e30200beea7fba1b2ae880a0fc81a Mon Sep 17 00:00:00 2001 From: Andrea Ricchi Date: Wed, 1 Jul 2026 16:18:54 +0200 Subject: [PATCH 2/2] gconnman_manager: Do not hold mtx_ while invoking changed callbacks on_services_changed_cb, get_proxies_cb and on_technology_added_removed_cb all invoke the user-registered services_changed_cb_/technologies_changed_cb_ while still holding mtx_. If the callback calls back into Manager::services()/technologies() (both of which lock the same non-recursive mtx_), the callback dispatch thread deadlocks on itself. This is easy to hit in practice: a consumer's services-changed handler typically wants to read the current service list, which is the obvious thing to call from inside the callback. Copy the updated list and the callback out under the lock, release the lock, then invoke the callback with the copies. This also means any work the callback does (which we don't control) no longer serializes against unrelated Manager accessors while it runs. Signed-off-by: Andrea Ricchi --- src/dbus/gconnman_manager.cpp | 86 +++++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index a1ec398..4a37ac7 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -83,7 +83,6 @@ void Manager::process_services_changed( } } services_ = new_order_of_services; - services_changed_cb_(services_); } void Manager::setup_agent() { @@ -261,16 +260,29 @@ void Manager::get_proxies_cb(GObject* proxy, GAsyncResult* res, if (success) { proxies = self->template arrays_to_proxies(out_properties); g_variant_unref(out_properties); - std::lock_guard const lock(self->mtx_); if constexpr (std::is_same_v) { - self->services_ = proxies; - if (!self->services_.empty()) { - self->services_changed_cb_(self->services_); + OnServListChangedCallback callback; + { + std::lock_guard const lock(self->mtx_); + self->services_ = proxies; + if (!self->services_.empty()) { + callback = self->services_changed_cb_; + } + } + if (callback) { + callback(proxies); } } else { - self->technologies_ = proxies; - if (!self->technologies_.empty()) { - self->technologies_changed_cb_(self->technologies_); + OnTechListChangedCallback callback; + { + std::lock_guard const lock(self->mtx_); + self->technologies_ = proxies; + if (!self->technologies_.empty()) { + callback = self->technologies_changed_cb_; + } + } + if (callback) { + callback(proxies); } } @@ -314,25 +326,34 @@ void Manager::on_technology_added_removed_cb(GDBusProxy* /*proxy*/, GVariant* parameters, gpointer user_data) { auto* self = static_cast(user_data); - std::lock_guard const lock(self->mtx_); - if (g_strcmp0(signal_name, "g-signal::TechnologyAdded") == 0U) { - const auto technology = - self->template dict_to_proxy(parameters); - self->technologies_.push_back(technology); - - } else if (g_strcmp0(signal_name, "g-signal::TechnologyRemoved") == 0U) { - const auto object_path = - std::string(g_variant_get_string(parameters, nullptr)); - - self->technologies_.erase( - std::remove_if(self->technologies_.begin(), - self->technologies_.end(), - [&object_path](const auto& technology) { - return technology->objPath() == object_path; - }), - self->technologies_.end()); + Manager::ProxyList updated_technologies; + OnTechListChangedCallback callback; + { + std::lock_guard const lock(self->mtx_); + if (g_strcmp0(signal_name, "g-signal::TechnologyAdded") == 0U) { + const auto technology = + self->template dict_to_proxy(parameters); + self->technologies_.push_back(technology); + + } else if (g_strcmp0(signal_name, "g-signal::TechnologyRemoved") == + 0U) { + const auto object_path = + std::string(g_variant_get_string(parameters, nullptr)); + + self->technologies_.erase( + std::remove_if(self->technologies_.begin(), + self->technologies_.end(), + [&object_path](const auto& technology) { + return technology->objPath() == object_path; + }), + self->technologies_.end()); + } + updated_technologies = self->technologies_; + callback = self->technologies_changed_cb_; + } + if (callback) { + callback(updated_technologies); } - self->technologies_changed_cb_(self->technologies_); } void Manager::on_services_changed_cb(GDBusProxy* /*proxy*/, @@ -371,8 +392,17 @@ void Manager::on_services_changed_cb(GDBusProxy* /*proxy*/, } g_variant_unref(removed); - std::lock_guard const lock(self->mtx_); - self->process_services_changed(services_removed, services_changed); + Manager::ProxyList updated_services; + OnServListChangedCallback callback; + { + std::lock_guard const lock(self->mtx_); + self->process_services_changed(services_removed, services_changed); + updated_services = self->services_; + callback = self->services_changed_cb_; + } + if (callback) { + callback(updated_services); + } } void Manager::onTechnologiesChanged(OnTechListChangedCallback callback) {