From 7a96c995cbb702183189919078410f3166c168ad Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 1 Jan 2026 11:52:02 +0100 Subject: [PATCH 01/19] Fix clang warnings Fix modernize warnings in find_if calls. Fix unused include warning. Fix passing const values by value. Fix marking nodiscard warning Signed-off-by: Eduardo Gonzalez --- examples/connmanctl.cpp | 29 +++++++++++------------ include/amarula/dbus/connman/gmanager.hpp | 4 +++- src/dbus/gconnman_manager.cpp | 11 ++++----- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/connmanctl.cpp b/examples/connmanctl.cpp index c943fa6..27c2c5d 100644 --- a/examples/connmanctl.cpp +++ b/examples/connmanctl.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -19,7 +20,7 @@ auto main() -> int { bool connecting = false; Connman connman; const auto manager = connman.manager(); - manager->onRequestInputPassphrase([&](auto service) -> auto { + manager->onRequestInputPassphrase([&](const auto& service) -> auto { const auto name = service->properties().getName(); std::string passphrase; @@ -46,14 +47,13 @@ auto main() -> int { } } // Trim whitespace - line.erase(line.begin(), - std::find_if(line.begin(), line.end(), [](int character) { + line.erase(line.begin(), std::ranges::find_if(line, [](int character) { return std::isspace(character) == 0; })); - line.erase(std::find_if(line.rbegin(), line.rend(), - [](int character) { - return std::isspace(character) == 0; - }) + line.erase(std::ranges::find_if(std::ranges::reverse_view(line), + [](int character) { + return std::isspace(character) == 0; + }) .base(), line.end()); @@ -118,9 +118,8 @@ auto main() -> int { << service->objPath() << "\n"; } } else { - auto iterator = std::find_if( - services.begin(), services.end(), - [&arg](const auto& service) { + auto iterator = std::ranges::find_if( + services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -225,8 +224,8 @@ auto main() -> int { } const bool connect = (cmd == "connect"); const auto services = manager->services(); - auto iterator = std::find_if( - services.begin(), services.end(), [&arg](const auto& service) { + auto iterator = + std::ranges::find_if(services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -279,8 +278,8 @@ auto main() -> int { continue; } const auto services = manager->services(); - auto iterator = std::find_if( - services.begin(), services.end(), [&arg](const auto& service) { + auto iterator = + std::ranges::find_if(services, [&arg](const auto& service) { return service->objPath() == arg || service->properties().getName() == arg; }); @@ -311,4 +310,4 @@ auto main() -> int { } std::cout << "Exiting.\n"; return 0; -} \ No newline at end of file +} diff --git a/include/amarula/dbus/connman/gmanager.hpp b/include/amarula/dbus/connman/gmanager.hpp index e7d6393..8db7c4f 100644 --- a/include/amarula/dbus/connman/gmanager.hpp +++ b/include/amarula/dbus/connman/gmanager.hpp @@ -123,7 +123,9 @@ class Manager : public DBusProxy { void onTechnologiesChanged(OnTechListChangedCallback callback); void onServicesChanged(OnServListChangedCallback callback); - auto internalAgentPath() const -> std::string { return agent_->path_; }; + [[nodiscard]] auto internalAgentPath() const -> std::string { + return agent_->path_; + }; private: enum class InputType : std::uint8_t { diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index f6ad993..46921b3 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -12,9 +12,9 @@ #include #include #include -#include #include #include +#include #include #include #include @@ -95,11 +95,10 @@ void Manager::setup_agent() { std::lock_guard const lock(mtx_); GVariantBuilder builder; g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}")); - auto service_it = - std::find_if(services_.begin(), services_.end(), - [&service_path](const auto service) { - return service->objPath() == service_path; - }); + auto service_it = std::ranges::find_if( + services_, [&service_path](const auto& service) { + return service->objPath() == service_path; + }); if (service_it != services_.end()) { auto* parsed_fields = parse_fields(fields); From 27207dc05fc91b743d51b8843823d177a3e2bfe4 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 16:18:00 +0200 Subject: [PATCH 02/19] CMakeLists: Remove unused includes Remove unused CPackIFW and CTest include statements, this solves #18. Signed-off-by: Eduardo Gonzalez --- CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 70ff9eb..9c6a281 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,7 +88,6 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) add_subdirectory(tests) endif(BUILD_TESTS) if(BUILD_EXAMPLES) - include(CTest) add_subdirectory(examples) endif(BUILD_EXAMPLES) if(BUILD_DOCS) @@ -106,7 +105,6 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) ) set(CPACK_PACKAGE_FILE_NAME ${cpack_file_name}) include(CPack) - include(CPackIFW) cpack_add_component(${PROJECT_NAME}) endif(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) From d07c6b74bed063799c6626877d093c723e02ad27 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 11:15:51 +0200 Subject: [PATCH 03/19] gconnman_serv_test: Wired can not be removed Calling this method(remove) on Ethernet devices, hidden WiFi services or provisioned services will cause an error message. It is not possible to remove these kind of services. https://git.kernel.org/pub/scm/network/connman/connman.git/tree/doc/ service-api.txt#n98 Signed-off-by: Eduardo Gonzalez --- tests/gconnman_serv_test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/gconnman_serv_test.cpp b/tests/gconnman_serv_test.cpp index ff54298..5260e80 100644 --- a/tests/gconnman_serv_test.cpp +++ b/tests/gconnman_serv_test.cpp @@ -12,6 +12,7 @@ using Amarula::DBus::G::Connman::Connman; using Error = Amarula::DBus::G::Connman::ServProperties::Error; using State = Amarula::DBus::G::Connman::ServProperties::State; using Type = Amarula::DBus::G::Connman::TechProperties::Type; +using ServType = Amarula::DBus::G::Connman::ServProperties::Type; TEST(Connman, getServs) { bool called = false; @@ -96,7 +97,8 @@ TEST(Connman, ForgetAndDisconnectService) { const auto name = props.getName(); const auto state = props.getState(); - if (props.getError() != Error::None || props.isFavorite()) { + if ((props.getError() != Error::None || props.isFavorite()) && + props.getType() != ServType::Ethernet) { std::cout << "Removing service: " << name << '\n'; serv->remove([serv, name](bool success) { EXPECT_TRUE(success); From 9e0f59e3b623d70709c1b19fb747cbb6e3747e7b Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 18 Mar 2026 16:47:27 +0100 Subject: [PATCH 04/19] .gitignore: Ignore CMakeLists.txt.user Signed-off-by: Eduardo Gonzalez --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 195a574..9fed214 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ build* doc tags CMakeUserPresets.json +CMakeLists.txt.user From 52344df4e16101a32b98a48454103e82275069ba Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 7 Jan 2026 16:27:10 +0100 Subject: [PATCH 05/19] Proxy: remove passing proxy member in methods Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 18 +++++++++--------- src/dbus/gconnman_clock.cpp | 12 ++++++------ src/dbus/gconnman_manager.cpp | 10 +++++----- src/dbus/gconnman_service.cpp | 14 +++++++------- src/dbus/gconnman_technology.cpp | 6 +++--- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index d7a7f77..ca2a311 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -135,7 +135,7 @@ class DBusProxy : public std::enable_shared_from_this> { void getProperties(PropertiesCallback callback = nullptr) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy_, nullptr, "GetProperties", nullptr, + callMethod(nullptr, "GetProperties", nullptr, &DBusProxy::get_property_cb, data.release()); } @@ -226,25 +226,25 @@ class DBusProxy : public std::enable_shared_from_this> { self->template executeCallBack(counter, success); } - static void setProperty(GDBusProxy* proxy, const gchar* arg_name, - GVariant* arg_value, GCancellable* cancellable, - GAsyncReadyCallback callback, gpointer user_data + void setProperty(const gchar* arg_name, GVariant* arg_value, + GCancellable* cancellable, GAsyncReadyCallback callback, + gpointer user_data ) { std::array tuple_elements{ g_variant_new_string(arg_name), g_variant_new_variant(arg_value)}; GVariant* parameters = g_variant_new_tuple(tuple_elements.data(), 2); - g_dbus_proxy_call(proxy, "SetProperty", parameters, + g_dbus_proxy_call(proxy_, "SetProperty", parameters, G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); } - static void callMethod(GDBusProxy* proxy, GCancellable* cancellable, - const gchar* arg_name, GVariant* parameters, - GAsyncReadyCallback callback, gpointer user_data) { + void callMethod(GCancellable* cancellable, const gchar* arg_name, + GVariant* parameters, GAsyncReadyCallback callback, + gpointer user_data) { g_dbus_proxy_call( - proxy, arg_name, + proxy_, arg_name, (parameters != nullptr) ? parameters : g_variant_new_tuple(nullptr, 0), G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); diff --git a/src/dbus/gconnman_clock.cpp b/src/dbus/gconnman_clock.cpp index 1765413..2cc45a9 100644 --- a/src/dbus/gconnman_clock.cpp +++ b/src/dbus/gconnman_clock.cpp @@ -52,22 +52,22 @@ Clock::Clock(DBus* dbus) void Clock::setTime(uint64_t time, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIME_STR, g_variant_new_uint64(time), nullptr, + setProperty(TIME_STR, g_variant_new_uint64(time), nullptr, &Clock::finishAsyncCall, data.release()); } void Clock::setTimeZone(const std::string& timezone, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIMEZONE_STR, g_variant_new_string(timezone.c_str()), - nullptr, &Clock::finishAsyncCall, data.release()); + setProperty(TIMEZONE_STR, g_variant_new_string(timezone.c_str()), nullptr, + &Clock::finishAsyncCall, data.release()); } void Clock::setTimeUpdates(const Properties::TimeUpdate time_updates, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); setProperty( - proxy(), TIMEUPDATES_STR, + TIMEUPDATES_STR, g_variant_new_string((TIME_UPDATE_MAP.toString(time_updates)).data()), nullptr, &Clock::finishAsyncCall, data.release()); } @@ -76,7 +76,7 @@ void Clock::setTimeZoneUpdates( const Properties::TimeZoneUpdate time_zone_updates, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), TIMEZONEUPDATES_STR, + setProperty(TIMEZONEUPDATES_STR, g_variant_new_string( (TIME_ZONE_UPDATE_MAP.toString(time_zone_updates)).data()), nullptr, &Clock::finishAsyncCall, data.release()); @@ -92,7 +92,7 @@ void Clock::setTimeServers(const std::vector& servers, g_variant_builder_add_value(&builder, str_variant); } GVariant* servers_variant = g_variant_builder_end(&builder); - setProperty(proxy(), TIMESERVERS_STR, servers_variant, nullptr, + setProperty(TIMESERVERS_STR, servers_variant, nullptr, &Clock::finishAsyncCall, data.release()); g_variant_builder_clear(&builder); } diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index 46921b3..5f11a21 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -199,7 +199,7 @@ void Manager::setup_agent() { void Manager::setOfflineMode(bool offline_mode, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), OFFLINEMODE_STR, + setProperty(OFFLINEMODE_STR, g_variant_new_boolean(static_cast(offline_mode)), nullptr, &Manager::finishAsyncCall, data.release()); } @@ -276,12 +276,12 @@ void Manager::get_proxies_cb(GObject* proxy, GAsyncResult* res, } void Manager::get_technologies() { - callMethod(proxy(), nullptr, GETTECHNOLOGIES_STR, nullptr, + callMethod(nullptr, GETTECHNOLOGIES_STR, nullptr, &Manager::get_proxies_cb, this); } void Manager::get_services() { - callMethod(proxy(), nullptr, GETSERVICES_STR, nullptr, + callMethod(nullptr, GETSERVICES_STR, nullptr, &Manager::get_proxies_cb, this); } @@ -290,7 +290,7 @@ void Manager::registerAgent(const std::string& object_path, auto data = prepareCallback(std::move(callback)); GVariant* child = g_variant_new_object_path(object_path.c_str()); GVariant* parameters = g_variant_new_tuple(&child, 1); - callMethod(proxy(), nullptr, REGISTERAGENT_STR, parameters, + callMethod(nullptr, REGISTERAGENT_STR, parameters, &Manager::finishAsyncCall, data.release()); } @@ -299,7 +299,7 @@ void Manager::unregisterAgent(const std::string& object_path, auto data = prepareCallback(std::move(callback)); GVariant* child = g_variant_new_object_path(object_path.c_str()); GVariant* parameters = g_variant_new_tuple(&child, 1); - callMethod(proxy(), nullptr, UNREGISTERAGENT_STR, parameters, + callMethod(nullptr, UNREGISTERAGENT_STR, parameters, &Manager::finishAsyncCall, data.release()); } diff --git a/src/dbus/gconnman_service.cpp b/src/dbus/gconnman_service.cpp index 6e5fdf3..f5fbcd3 100644 --- a/src/dbus/gconnman_service.cpp +++ b/src/dbus/gconnman_service.cpp @@ -89,26 +89,26 @@ Service::Service(DBus* dbus, const gchar* obj_path) void Service::connect(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, CONNECT_STR, nullptr, - &Service::finishAsyncCall, data.release()); + callMethod(nullptr, CONNECT_STR, nullptr, &Service::finishAsyncCall, + data.release()); } void Service::disconnect(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, DISCONNECT_STR, nullptr, - &Service::finishAsyncCall, data.release()); + callMethod(nullptr, DISCONNECT_STR, nullptr, &Service::finishAsyncCall, + data.release()); } void Service::remove(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, REMOVE_STR, nullptr, &Service::finishAsyncCall, + callMethod(nullptr, REMOVE_STR, nullptr, &Service::finishAsyncCall, data.release()); } void Service::setAutoconnect(const bool autoconnect, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), AUTOCONNECT_STR, + setProperty(AUTOCONNECT_STR, g_variant_new_boolean(static_cast(autoconnect)), nullptr, &Service::finishAsyncCall, data.release()); } @@ -117,7 +117,7 @@ void Service::setNameServers(const std::vector& name_servers, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); auto variant = vector_to_as(name_servers); - setProperty(proxy(), NAMESERVERS_CONFIGURATION_STR, variant.get(), nullptr, + setProperty(NAMESERVERS_CONFIGURATION_STR, variant.get(), nullptr, &Service::finishAsyncCall, data.release()); } diff --git a/src/dbus/gconnman_technology.cpp b/src/dbus/gconnman_technology.cpp index ea1fbd8..d94a80b 100644 --- a/src/dbus/gconnman_technology.cpp +++ b/src/dbus/gconnman_technology.cpp @@ -30,15 +30,15 @@ Technology::Technology(DBus* dbus, const gchar* obj_path) void Technology::setPowered(bool powered, PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - setProperty(proxy(), POWERED_STR, + setProperty(POWERED_STR, g_variant_new_boolean(static_cast(powered)), nullptr, &Technology::finishAsyncCall, data.release()); } void Technology::scan(PropertiesSetCallback callback) { auto data = prepareCallback(std::move(callback)); - callMethod(proxy(), nullptr, SCAN_STR, nullptr, - &Technology::finishAsyncCall, data.release()); + callMethod(nullptr, SCAN_STR, nullptr, &Technology::finishAsyncCall, + data.release()); } void TechProperties::update(const gchar* key, GVariant* value) { From fa815c406686411f7972f44a31ef7c2957d51295 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 17:55:06 +0100 Subject: [PATCH 06/19] gproxy.hpp: Reuse callMethod Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index ca2a311..eabd425 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -235,9 +235,7 @@ class DBusProxy : public std::enable_shared_from_this> { g_variant_new_string(arg_name), g_variant_new_variant(arg_value)}; GVariant* parameters = g_variant_new_tuple(tuple_elements.data(), 2); - g_dbus_proxy_call(proxy_, "SetProperty", parameters, - G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, - user_data); + callMethod(cancellable, "SetProperty", parameters, callback, user_data); } void callMethod(GCancellable* cancellable, const gchar* arg_name, From 61dbfc94c8d0ca1d1ea86ff8affeb5df61f71e5f Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 16:55:02 +0100 Subject: [PATCH 07/19] tests: Add check thread id of callbacks Iterate all the test files in the CMake configuration in order to remove duplicated code. Define a class that iterates the global default context simulating what Qt libraries does. The latter class saves the thread id where the object is created and the thread id where the global default context it is iterated. Use the class on the tests to check that callbacks are not run on the main thread nor in the thread iterating the global default context. This helps to test the issue presented in #21. Signed-off-by: Eduardo Gonzalez --- tests/CMakeLists.txt | 22 ++--- tests/gconnman_clock_test.cpp | 45 +++++++-- tests/gconnman_serv_test.cpp | 166 ++++++++++++++++++++++++---------- tests/gconnman_tech_test.cpp | 96 +++++++++++++------- tests/qt_thread_bundle.hpp | 50 ++++++++++ 5 files changed, 281 insertions(+), 98 deletions(-) create mode 100644 tests/qt_thread_bundle.hpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fe5356e..432499a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,21 +20,21 @@ install( COMPONENT ${PROJECT_NAME}-dev) if(BUILD_CONNMAN) - add_executable(gconnman_clock_test gconnman_clock_test.cpp) - target_link_libraries(gconnman_clock_test PRIVATE GConnmanDbus gtest_main) + foreach(connman_test gconnman_clock_test gconnman_tech_test + gconnman_serv_test) + add_executable(${connman_test} ${connman_test}.cpp qt_thread_bundle.hpp) + target_link_libraries(${connman_test} PRIVATE GConnmanDbus gtest_main) - add_executable(gconnman_tech_test gconnman_tech_test.cpp) - target_link_libraries(gconnman_tech_test PRIVATE GConnmanDbus gtest_main) + target_include_directories(${connman_test} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - add_executable(gconnman_serv_test gconnman_serv_test.cpp) - target_link_libraries(gconnman_serv_test PRIVATE GConnmanDbus gtest_main) + install( + TARGETS ${connman_test} + EXPORT ${PROJECT_NAME}-config + COMPONENT ${PROJECT_NAME}-dev) + endforeach() - install( - TARGETS gconnman_clock_test gconnman_tech_test gconnman_serv_test - EXPORT ${PROJECT_NAME}-config - COMPONENT ${PROJECT_NAME}-dev) endif(BUILD_CONNMAN) - add_executable(log_test log_test.cpp) target_link_libraries(log_test PRIVATE GDbusProxy gtest_main) diff --git a/tests/gconnman_clock_test.cpp b/tests/gconnman_clock_test.cpp index b402906..095ee7c 100644 --- a/tests/gconnman_clock_test.cpp +++ b/tests/gconnman_clock_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using TimeUpdate = Amarula::DBus::G::Connman::ClockProperties::TimeUpdate; using TimeZoneUpdate = TimeUpdate; @@ -16,16 +18,29 @@ constexpr auto TEST_TIME_ZONE = "America/Vancouver"; constexpr int SLEEP_DURATION_SECONDS = 3; TEST(Connman, ClockSetTimeUpdates) { + const QtThreadBundle qt_thread_bundle; const Connman connman; - connman.clock()->onPropertyChanged([](auto& props) { - std::cout << "onPropertyChanged:\n"; - props.print(); - }); + connman.clock()->onPropertyChanged( + [main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto& props) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + std::cout << "onPropertyChanged:\n"; + props.print(); + }); connman.clock()->setTimeUpdates( - TimeUpdate::Auto, [&connman](auto success) { EXPECT_TRUE(success); }); + TimeUpdate::Auto, [&connman, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success); + }); } TEST(Connman, ClockSetTime) { + const QtThreadBundle qt_thread_bundle; const Connman connman; const guint time = TEST_TIME; connman.clock()->onPropertyChanged([](auto& props) { @@ -46,6 +61,7 @@ TEST(Connman, ClockSetTime) { } TEST(Connman, ClockSetTimeServers1) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -58,6 +74,7 @@ TEST(Connman, ClockSetTimeServers1) { } TEST(Connman, ClockSetTimeServers2) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -75,6 +92,7 @@ TEST(Connman, ClockSetTimeServers2) { } TEST(Connman, ClockSetTimeZoneUpdates) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -86,6 +104,7 @@ TEST(Connman, ClockSetTimeZoneUpdates) { } TEST(Connman, ClockSetTimeZone) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->onPropertyChanged([](auto& props) { std::cout << "onPropertyChanged:\n"; @@ -106,14 +125,26 @@ TEST(Connman, ClockSetTimeZone) { } TEST(Connman, ClockProxyInitialization) { - EXPECT_NO_THROW({ const Connman connman; }); + EXPECT_NO_THROW({ + const QtThreadBundle qt_thread_bundle; + const Connman connman; + }); } TEST(Connman, clockGetPropertiesNull) { + const QtThreadBundle qt_thread_bundle; const Connman connman; connman.clock()->getProperties(); } TEST(Connman, ClockGetProperties) { + const QtThreadBundle qt_thread_bundle; const Connman connman; - connman.clock()->getProperties([](auto& props) { props.print(); }); + connman.clock()->getProperties( + [main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto& props) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + props.print(); + }); } diff --git a/tests/gconnman_serv_test.cpp b/tests/gconnman_serv_test.cpp index 5260e80..49b1ff4 100644 --- a/tests/gconnman_serv_test.cpp +++ b/tests/gconnman_serv_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using Error = Amarula::DBus::G::Connman::ServProperties::Error; @@ -17,14 +19,23 @@ using ServType = Amarula::DBus::G::Connman::ServProperties::Type; TEST(Connman, getServs) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { + manager->onTechnologiesChanged([main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& technologies) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); ASSERT_FALSE(technologies.empty()) << "No technologies returned"; // Power on all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([&](const auto& prop) { + tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(prop.isPowered()) << "Technology " << prop.getName() << " was not powered ON"; @@ -34,7 +45,11 @@ TEST(Connman, getServs) { const auto prop = tech->properties(); const auto name = prop.getName(); if (!prop.isPowered()) { - tech->setPowered(true, [&, name](auto success) { + tech->setPowered(true, [main_tid, loop_tid, + name](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success) << "Set power for " << name << " did not succeed"; }); @@ -42,14 +57,19 @@ TEST(Connman, getServs) { } }); - manager->onServicesChanged([&](const auto& services) { - called = true; - ASSERT_FALSE(services.empty()); - for (const auto& serv : services) { - const auto props = serv->properties(); - props.print(); - } - }); + manager->onServicesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& services) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(services.empty()); + for (const auto& serv : services) { + const auto props = serv->properties(); + props.print(); + } + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -57,27 +77,38 @@ TEST(Connman, getServs) { TEST(Connman, setNameServers) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onServicesChanged([&](const auto& services) { - called = true; - ASSERT_FALSE(services.empty()); - for (const auto& serv : services) { - const auto props = serv->properties(); - const auto name = props.getName(); - props.print(); - serv->onPropertyChanged([](const auto& properties) { - std::cout << "onPropertyChange:\n"; - properties.print(); - }); - serv->setNameServers( - {"8.8.8.8", "4.4.4.4"}, [&, name](auto success) { - EXPECT_TRUE(success) << "Set setNameServers for " - << name << " did not succeed"; + manager->onServicesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& services) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(services.empty()); + for (const auto& serv : services) { + const auto props = serv->properties(); + const auto name = props.getName(); + props.print(); + serv->onPropertyChanged([](const auto& properties) { + std::cout << "onPropertyChange:\n"; + properties.print(); }); - } - }); + serv->setNameServers( + {"8.8.8.8", "4.4.4.4"}, + [name, main_tid, loop_tid](auto success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) << "Set setNameServers for " + << name << " did not succeed"; + }); + } + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -86,11 +117,18 @@ TEST(Connman, ForgetAndDisconnectService) { bool called = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onServicesChanged([&](const auto& services) { + manager->onServicesChanged([&called, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& services) { called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); ASSERT_FALSE(services.empty()) << "No services returned"; for (const auto& serv : services) { const auto props = serv->properties(); @@ -100,14 +138,21 @@ TEST(Connman, ForgetAndDisconnectService) { if ((props.getError() != Error::None || props.isFavorite()) && props.getType() != ServType::Ethernet) { std::cout << "Removing service: " << name << '\n'; - serv->remove([serv, name](bool success) { + serv->remove([serv, name, main_tid, + loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service removed: " << name << '\n'; serv->properties().print(); }); } else if (state == State::Ready || state == State::Online) { std::cout << "Disconnecting service: " << name << '\n'; - serv->disconnect([serv](bool success) { + serv->disconnect([serv, main_tid, loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service disconnected: " << serv->objPath() << '\n'; @@ -125,19 +170,31 @@ TEST(Connman, ConnectWifi) { bool called = false; bool called_request_input = false; { + const QtThreadBundle qt_thread_bundle; const Connman connman; const auto manager = connman.manager(); - manager->onRequestInputPassphrase([&](auto service) -> auto { - called_request_input = true; - std::cout << "Requesting input passphrase for service: " - << service->properties().getName() << '\n'; - const auto name = service->properties().getName(); - EXPECT_EQ(name, "connmantest"); - return std::pair{true, "amaruladev"}; - }); + manager->onRequestInputPassphrase( + [&called_request_input, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](auto service) -> auto { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + called_request_input = true; + std::cout << "Requesting input passphrase for service: " + << service->properties().getName() << '\n'; + const auto name = service->properties().getName(); + EXPECT_EQ(name, "connmantest"); + return std::pair{true, "amaruladev"}; + }); - manager->onServicesChanged([&](const auto& services) { + manager->onServicesChanged([&called, manager, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& services) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); called = true; ASSERT_FALSE(services.empty()) << "No services returned"; @@ -153,15 +210,30 @@ TEST(Connman, ConnectWifi) { if (state == State::Idle) { std::cout << "Connecting to service: " << name << '\n'; props.print(); - serv->onPropertyChanged([](const auto& properties) { - std::cout << "onPropertyChange:\n"; - properties.print(); - }); + serv->onPropertyChanged( + [main_tid, loop_tid](const auto& properties) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + std::cout << "onPropertyChange:\n"; + properties.print(); + }); manager->registerAgent( manager->internalAgentPath(), - [serv, manager](const auto success) { + [serv, manager, main_tid, + loop_tid](const auto success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); - serv->connect([serv, manager](bool success) { + serv->connect([serv, manager, main_tid, + loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success); std::cout << "Service connected successfully: " @@ -192,4 +264,4 @@ TEST(Connman, ConnectWifi) { ASSERT_TRUE(called) << "ServicesChanged callback was never called"; ASSERT_TRUE(called_request_input) << "Did not requested user input"; -} \ No newline at end of file +} diff --git a/tests/gconnman_tech_test.cpp b/tests/gconnman_tech_test.cpp index 47c84c4..4be4010 100644 --- a/tests/gconnman_tech_test.cpp +++ b/tests/gconnman_tech_test.cpp @@ -3,28 +3,36 @@ #include #include +#include "qt_thread_bundle.hpp" + using Amarula::DBus::G::Connman::Connman; using Type = Amarula::DBus::G::Connman::TechProperties::Type; TEST(Connman, getTechs) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - called = true; - ASSERT_FALSE(technologies.empty()); - for (const auto& tech : technologies) { - const auto props = tech->properties(); - EXPECT_FALSE(props.getName().empty()); - if (props.isConnected()) { - EXPECT_TRUE(props.isPowered()) - << "Technology is connected but not powered"; + manager->onTechnologiesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& technologies) { + called = true; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(technologies.empty()); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + EXPECT_FALSE(props.getName().empty()); + if (props.isConnected()) { + EXPECT_TRUE(props.isPowered()) + << "Technology is connected but not powered"; + } + props.print(); } - props.print(); - } - }); + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -33,18 +41,27 @@ TEST(Connman, PowerOnAllTechnologies) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - + manager->onTechnologiesChanged([&called, + main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid]( + const auto& technologies) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + ASSERT_FALSE(technologies.empty()); // Power on all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([&](const auto& prop) { + tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { EXPECT_TRUE(prop.isPowered()) << "Technology " << prop.getName() << " was not powered ON"; + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); std::cout << "onPropertyChanged:\n"; prop.print(); }); @@ -52,7 +69,11 @@ TEST(Connman, PowerOnAllTechnologies) { const auto name = prop.getName(); if (!prop.isPowered()) { std::cout << "Powering on technology: " << name << '\n'; - tech->setPowered(true, [&, name](auto success) { + tech->setPowered(true, [&called, name, main_tid, + loop_tid](auto success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); std::cout << "setPowered callback for " << name << ": " << (success ? "Success" : "Failure") << '\n'; EXPECT_TRUE(success) @@ -70,27 +91,36 @@ TEST(Connman, PowerOnAllTechnologies) { TEST(Connman, ScanWifiTechnology) { bool called = false; { + const QtThreadBundle qt_thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) << "No technologies returned"; + manager->onTechnologiesChanged( + [&called, main_tid = qt_thread_bundle.main_tid, + loop_tid = qt_thread_bundle.loop_tid](const auto& technologies) { + ASSERT_FALSE(technologies.empty()) + << "No technologies returned"; - for (const auto& tech : technologies) { - const auto props = tech->properties(); - const auto name = props.getName(); - if (props.getType() == Type::Wifi) { - std::cout << "Scanning technology with name: " << name - << "\n"; - tech->scan([&called, name](bool success) { - called = true; - EXPECT_TRUE(success); - std::cout << "Technology " << name - << " scanned successfully.\n"; - }); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + const auto name = props.getName(); + if (props.getType() == Type::Wifi) { + std::cout << "Scanning technology with name: " << name + << "\n"; + tech->scan( + [&called, name, main_tid, loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + called = true; + EXPECT_TRUE(success); + std::cout << "Technology " << name + << " scanned successfully.\n"; + }); + } } - } - }); + }); } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } diff --git a/tests/qt_thread_bundle.hpp b/tests/qt_thread_bundle.hpp new file mode 100644 index 0000000..c9a0c15 --- /dev/null +++ b/tests/qt_thread_bundle.hpp @@ -0,0 +1,50 @@ +#pragma once + +#include + +#include +#include + +struct QtThreadBundle { + std::thread::id main_tid; + std::thread::id loop_tid; + + QtThreadBundle() + : main_tid(std::this_thread::get_id()), + loop_(g_main_loop_new(nullptr, FALSE)), + qt_simulator_([this]() { + this->loop_tid = std::this_thread::get_id(); + g_idle_add(&QtThreadBundle::on_loop_started, this); + g_main_loop_run(this->loop_); + g_main_loop_unref(this->loop_); + }) { + { + std::unique_lock lock(mtx_); + cv_.wait(lock, [this] { return running_; }); + } + } + + ~QtThreadBundle() { + if (qt_simulator_.joinable()) { + g_main_loop_quit(loop_); + qt_simulator_.join(); + } + } + + private: + std::mutex mtx_; + std::condition_variable cv_; + bool running_{false}; + GMainLoop* loop_; + std::thread qt_simulator_; + + static auto on_loop_started(gpointer user_data) -> gboolean { + auto* self = static_cast(user_data); + { + std::lock_guard const lock(self->mtx_); + self->running_ = true; + } + self->cv_.notify_all(); + return G_SOURCE_REMOVE; + } +}; From cb19d5e0685f42ef2c1d7829b082d03945f320c3 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 20 Mar 2026 17:05:19 +0100 Subject: [PATCH 08/19] gdbus: Add thread default context Create a thread default context and iterate the later on the worker thread. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gdbus.hpp | 2 ++ src/dbus/gdbus.cpp | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/amarula/dbus/gdbus.hpp b/include/amarula/dbus/gdbus.hpp index 157561c..038e51d 100644 --- a/include/amarula/dbus/gdbus.hpp +++ b/include/amarula/dbus/gdbus.hpp @@ -18,6 +18,7 @@ class DBus { unsigned int pending_calls_{0}; GDBusConnection* connection_ = nullptr; GMainLoop* loop_{nullptr}; + GMainContext* ctx_{nullptr}; static auto on_loop_started(gpointer user_data) -> gboolean; @@ -35,6 +36,7 @@ class DBus { void onAnyAsyncStart(); [[nodiscard]] auto connection() const { return connection_; } + [[nodiscard]] auto context() const { return ctx_; } }; } // namespace Amarula::DBus::G diff --git a/src/dbus/gdbus.cpp b/src/dbus/gdbus.cpp index a4fb224..e4962f8 100644 --- a/src/dbus/gdbus.cpp +++ b/src/dbus/gdbus.cpp @@ -19,8 +19,10 @@ void DBus::onAnyAsyncDone() { void DBus::onAnyAsyncStart() { ++pending_calls_; } -DBus::DBus(const std::string& bus_name, const std::string& object_path) { +DBus::DBus(const std::string& bus_name, const std::string& object_path) + : ctx_{g_main_context_new()} { GError* error = nullptr; + g_main_context_push_thread_default(ctx_); connection_ = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &error); if (connection_ == nullptr) { std::string const msg = error->message; @@ -54,14 +56,15 @@ DBus::DBus(const std::string& bus_name, const std::string& object_path) { } g_variant_unref(result); + g_main_context_pop_thread_default(ctx_); start(); -} - -DBus::~DBus() { { std::unique_lock lock(mtx_); cv_.wait(lock, [this] { return running_; }); } +} + +DBus::~DBus() { { std::lock_guard const lock(mtx_); running_ = false; @@ -76,6 +79,7 @@ DBus::~DBus() { if (connection_ != nullptr) { g_object_unref(connection_); } + g_main_context_unref(ctx_); } auto DBus::on_loop_started(gpointer user_data) -> gboolean { @@ -91,13 +95,15 @@ auto DBus::on_loop_started(gpointer user_data) -> gboolean { void DBus::start() { if (!running_) { glib_thread_ = std::thread([this]() { - loop_ = g_main_loop_new(nullptr, FALSE); - g_idle_add(&DBus::on_loop_started, this); + g_main_context_invoke(ctx_, &DBus::on_loop_started, this); + g_main_context_push_thread_default(ctx_); + loop_ = g_main_loop_new(ctx_, FALSE); g_main_loop_run(loop_); + g_main_context_pop_thread_default(ctx_); g_main_loop_unref(loop_); loop_ = nullptr; }); } } -} // namespace Amarula::DBus::G \ No newline at end of file +} // namespace Amarula::DBus::G From fa6173faa71de46e20de22ad617d74f23e328b00 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 18 Mar 2026 16:26:18 +0100 Subject: [PATCH 09/19] gproxy: use g_main_context_invoke in proxy callMethod Wrap the callMethod method inside a g_main_context_invoke. The latter makes that the callbacks are executed in the worker thread provided by the library. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 46 ++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index eabd425..cad9ada 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -238,14 +238,48 @@ class DBusProxy : public std::enable_shared_from_this> { callMethod(cancellable, "SetProperty", parameters, callback, user_data); } - void callMethod(GCancellable* cancellable, const gchar* arg_name, + void callMethod(GCancellable* cancellable, const std::string& arg_name, GVariant* parameters, GAsyncReadyCallback callback, gpointer user_data) { - g_dbus_proxy_call( - proxy_, arg_name, - (parameters != nullptr) ? parameters - : g_variant_new_tuple(nullptr, 0), - G_DBUS_CALL_FLAGS_NONE, -1, cancellable, callback, user_data); + struct Data { + GDBusProxy* proxy; + std::string arg_name; + GVariant* parameters; + GCancellable* cancellable; + GAsyncReadyCallback callback; + gpointer user_data; + }; + + auto data = std::make_unique( + Data{proxy_, arg_name, + (parameters != nullptr) + ? g_variant_ref_sink(parameters) + : g_variant_ref_sink(g_variant_new_tuple(nullptr, 0)), + (cancellable != nullptr) + ? static_cast(g_object_ref(cancellable)) + : nullptr, + callback, user_data}); + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + g_dbus_proxy_call(data->proxy, data->arg_name.c_str(), + data->parameters, G_DBUS_CALL_FLAGS_NONE, -1, + data->cancellable, data->callback, + data->user_data); + + return G_SOURCE_REMOVE; + }, + data.release(), + [](gpointer user_data) { + std::unique_ptr data(static_cast(user_data)); + g_variant_unref(data->parameters); + if (data->cancellable != nullptr) { + g_object_unref(data->cancellable); + } + }); } }; From ee98a507da1586a24895b35fcc9dde3d8649f9bd Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Mon, 13 Apr 2026 10:22:54 +0200 Subject: [PATCH 10/19] gdbus: Expose stop method Add a stop method that will try to stop the context iteration and wait for the thread to join. Improve start method to be able to be called many times in conjunction with the stop method. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gdbus.hpp | 1 + src/dbus/gdbus.cpp | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/include/amarula/dbus/gdbus.hpp b/include/amarula/dbus/gdbus.hpp index 038e51d..87ed777 100644 --- a/include/amarula/dbus/gdbus.hpp +++ b/include/amarula/dbus/gdbus.hpp @@ -34,6 +34,7 @@ class DBus { void start(); void onAnyAsyncDone(); void onAnyAsyncStart(); + void stop(); [[nodiscard]] auto connection() const { return connection_; } [[nodiscard]] auto context() const { return ctx_; } diff --git a/src/dbus/gdbus.cpp b/src/dbus/gdbus.cpp index e4962f8..2eacf1d 100644 --- a/src/dbus/gdbus.cpp +++ b/src/dbus/gdbus.cpp @@ -58,13 +58,9 @@ DBus::DBus(const std::string& bus_name, const std::string& object_path) g_variant_unref(result); g_main_context_pop_thread_default(ctx_); start(); - { - std::unique_lock lock(mtx_); - cv_.wait(lock, [this] { return running_; }); - } } -DBus::~DBus() { +void DBus::stop() { { std::lock_guard const lock(mtx_); running_ = false; @@ -76,9 +72,17 @@ DBus::~DBus() { if (glib_thread_.joinable()) { glib_thread_.join(); } +} + +DBus::~DBus() { + stop(); + g_main_context_push_thread_default(ctx_); + while (g_main_context_iteration(ctx_, FALSE) != 0) { + } if (connection_ != nullptr) { g_object_unref(connection_); } + g_main_context_pop_thread_default(ctx_); g_main_context_unref(ctx_); } @@ -104,6 +108,10 @@ void DBus::start() { loop_ = nullptr; }); } + { + std::unique_lock lock(mtx_); + cv_.wait(lock, [this] { return running_; }); + } } } // namespace Amarula::DBus::G From a6ace740c8694319d8c8a5c94984a659f494af85 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Mon, 13 Apr 2026 10:32:11 +0200 Subject: [PATCH 11/19] connman: Make explicit the destruction order The proxies has to be destroyed before the context, because the context have to be iterated later to free some weak ref inside the proxy. https://gitlab.gnome.org/GNOME/glib/-/issues/3926 Stop dbus thread before destroying manager to avoid any callback accessing invalid memory. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/connman/gconnman.hpp | 2 +- src/dbus/gconnman.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/amarula/dbus/connman/gconnman.hpp b/include/amarula/dbus/connman/gconnman.hpp index 02e9f99..b8260f3 100644 --- a/include/amarula/dbus/connman/gconnman.hpp +++ b/include/amarula/dbus/connman/gconnman.hpp @@ -19,7 +19,7 @@ class Connman { auto operator=(const Connman&) -> Connman& = delete; Connman(Connman&&) = delete; auto operator=(Connman&&) -> Connman& = delete; - ~Connman() { dbus_.reset(); } + ~Connman(); [[nodiscard]] auto clock() const { return clock_; } [[nodiscard]] auto manager() const { return manager_; } diff --git a/src/dbus/gconnman.cpp b/src/dbus/gconnman.cpp index f03f823..4c4cdac 100644 --- a/src/dbus/gconnman.cpp +++ b/src/dbus/gconnman.cpp @@ -20,4 +20,11 @@ Connman::Connman() clock_->getProperties(); } +Connman::~Connman() { + dbus_->stop(); + manager_.reset(); + clock_.reset(); + dbus_.reset(); +} + } // namespace Amarula::DBus::G::Connman From b5838a65cb3b131c8ea64928d6f9abe5a920db3f Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 10:12:55 +0200 Subject: [PATCH 12/19] gproxy.hpp: Initialize the glib proxy on worker thread Initialize the glib proxy member in the worker thread and connect the signals in the worker thread. All this make the callbacks of the connected signals to run on the worker thread. The creation of the proxy is blocked until the worker thread creates it. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/gproxy.hpp | 113 +++++++++++++++++++++++++++----- 1 file changed, 98 insertions(+), 15 deletions(-) diff --git a/include/amarula/dbus/gproxy.hpp b/include/amarula/dbus/gproxy.hpp index cad9ada..57ef01e 100644 --- a/include/amarula/dbus/gproxy.hpp +++ b/include/amarula/dbus/gproxy.hpp @@ -83,6 +83,47 @@ class DBusProxy : public std::enable_shared_from_this> { } protected: + template + void connectSignal(const std::string& signal_name, Callback callback, + gpointer user_data) { + struct Data { + GDBusProxy* proxy; + std::string signal_name; + Callback callback; + gpointer user_data; + std::mutex mtx; + std::condition_variable cv; + bool done{false}; + }; + + Data data{proxy_, signal_name, callback, user_data}; + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + g_signal_connect(data->proxy, data->signal_name.c_str(), + G_CALLBACK(data->callback), data->user_data); + + return G_SOURCE_REMOVE; + }, + &data, + [](gpointer user_data) { + auto* data = static_cast(user_data); + { + std::lock_guard const lock(data->mtx); + data->done = true; + } + data->cv.notify_all(); + }); + + { + std::unique_lock lock(data.mtx); + data.cv.wait(lock, [&] { return data.done; }); + } + } + void updateProperties(GVariant* properties) { GVariantIter* iter = g_variant_iter_new(properties); GVariant* prop = nullptr; @@ -176,23 +217,65 @@ class DBusProxy : public std::enable_shared_from_this> { dbus_->onAnyAsyncDone(); } - explicit DBusProxy(DBus* dbus, const gchar* name, const gchar* obj_path, - const gchar* interface_name) + explicit DBusProxy(DBus* dbus, const std::string& name, + const std::string& obj_path, + const std::string& interface_name) : dbus_{dbus} { - GError* err = nullptr; - - proxy_ = g_dbus_proxy_new_sync(dbus_->connection(), - G_DBUS_PROXY_FLAGS_NONE, nullptr, name, - obj_path, interface_name, nullptr, &err); - if (proxy_ == nullptr) { - std::string const msg = - "Failed to create proxy: " + std::string(err->message); - g_error_free(err); - throw std::runtime_error(msg); + struct Data { + DBusProxy* proxy; + std::string name; + std::string obj_path; + std::string interface_name; + std::mutex mtx; + std::condition_variable cv; + bool done{false}; + std::string error; + }; + + auto data = Data{this, name, obj_path, interface_name}; + + g_main_context_invoke_full( + dbus_->context(), G_PRIORITY_HIGH, + [](gpointer user_data) -> gboolean { + auto* data = static_cast(user_data); + + GError* err = nullptr; + + data->proxy->proxy_ = g_dbus_proxy_new_sync( + data->proxy->dbus_->connection(), G_DBUS_PROXY_FLAGS_NONE, + nullptr, data->name.c_str(), data->obj_path.c_str(), + data->interface_name.c_str(), nullptr, &err); + + if (data->proxy->proxy_ == nullptr) { + data->error = + "Failed to create proxy: " + std::string(err->message); + g_error_free(err); + } else { + g_signal_connect( + data->proxy->proxy_, "g-signal::PropertyChanged", + G_CALLBACK(&DBusProxy::on_properties_changed_cb), + data->proxy); + } + + return G_SOURCE_REMOVE; + }, + &data, + [](gpointer user_data) { + auto* data = static_cast(user_data); + { + std::lock_guard const lock(data->mtx); + data->done = true; + } + data->cv.notify_all(); + }); + { + std::unique_lock lock(data.mtx); + data.cv.wait(lock, [&] { return data.done; }); + } + + if (!data.error.empty()) { + throw std::runtime_error(data.error); } - g_signal_connect(proxy_, "g-signal::PropertyChanged", - G_CALLBACK(&DBusProxy::on_properties_changed_cb), - this); } template From f284a686675bfe5610059ccb9e8e9f0821877833 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 10:45:49 +0200 Subject: [PATCH 13/19] gconnman_manager.cpp: Connect signals in worker thread This solves #21. Signed-off-by: Eduardo Gonzalez --- src/dbus/gconnman_manager.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index 5f11a21..ef0a18d 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -49,14 +49,12 @@ Manager::Manager(DBus* dbus, const std::string& agent_path) get_technologies(); get_services(); - g_signal_connect(proxy(), "g-signal::TechnologyRemoved", - G_CALLBACK(&Manager::on_technology_added_removed_cb), - this); - g_signal_connect(proxy(), "g-signal::TechnologyAdded", - G_CALLBACK(&Manager::on_technology_added_removed_cb), - this); - g_signal_connect(proxy(), "g-signal::ServicesChanged", - G_CALLBACK(&Manager::on_services_changed_cb), this); + connectSignal("g-signal::TechnologyRemoved", + &Manager::on_technology_added_removed_cb, this); + connectSignal("g-signal::TechnologyAdded", + &Manager::on_technology_added_removed_cb, this); + connectSignal("g-signal::ServicesChanged", &Manager::on_services_changed_cb, + this); } void Manager::process_services_changed( From 58014f10c1ba7edfebe954128b346a2705fe70f0 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 1 Apr 2026 14:52:00 +0200 Subject: [PATCH 14/19] gconnman_agent: Register object in worker thread Call the register object in the dbus in the worker thread. The latter makes the callback to be executed also in the worker thread. The creation of the agent block the thread until the worker thread register the object. This solves #21. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/connman/gagent.hpp | 4 +- src/dbus/gconnman_agent.cpp | 59 ++++++++++++++++++++----- src/dbus/gconnman_manager.cpp | 3 +- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/include/amarula/dbus/connman/gagent.hpp b/include/amarula/dbus/connman/gagent.hpp index 998e3e5..4c0f7d0 100644 --- a/include/amarula/dbus/connman/gagent.hpp +++ b/include/amarula/dbus/connman/gagent.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -15,8 +16,7 @@ class Agent { GDBusConnection *connection_{nullptr}; std::string path_{"/net/amarula/gconnman/agent"}; - explicit Agent(GDBusConnection *connection, - const std::string &path = std::string()); + explicit Agent(DBus *dbus, const std::string &path = std::string()); using RequestInputCallback = std::function; diff --git a/src/dbus/gconnman_agent.cpp b/src/dbus/gconnman_agent.cpp index 3cf559a..8078fcc 100644 --- a/src/dbus/gconnman_agent.cpp +++ b/src/dbus/gconnman_agent.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -28,8 +29,8 @@ static constexpr const char *INTROSPECTION_XML = " " ""; -Agent::Agent(GDBusConnection *connection, const std::string &path) - : connection_{connection} { +Agent::Agent(DBus *dbus, const std::string &path) + : connection_{dbus->connection()} { if (!path.empty()) { path_ = path; } @@ -42,16 +43,54 @@ Agent::Agent(GDBusConnection *connection, const std::string &path) throw std::runtime_error(msg); } - registration_id_ = g_dbus_connection_register_object( - connection, path_.c_str(), *(node_info_->interfaces), &INTERFACE_VTABLE, - this, nullptr, &err); + struct Data { + Agent *self; + std::mutex mtx; + std::condition_variable cv; + bool done{false}; + std::string error; + }; + + auto data = Data{this}; + + g_main_context_invoke_full( + dbus->context(), G_PRIORITY_HIGH, + [](gpointer user_data) -> gboolean { + auto *data = static_cast(user_data); + + GError *err = nullptr; + + data->self->registration_id_ = g_dbus_connection_register_object( + data->self->connection_, data->self->path_.c_str(), + *(data->self->node_info_->interfaces), &INTERFACE_VTABLE, + data->self, nullptr, &err); + + if (data->self->registration_id_ == 0) { + data->error = + "Failed to register agent: " + std::string(err->message); + g_error_free(err); + } + + return G_SOURCE_REMOVE; + }, + &data, + [](gpointer user_data) { + auto *data = static_cast(user_data); + { + std::lock_guard const lock(data->mtx); + data->done = true; + } + data->cv.notify_all(); + }); + + { + std::unique_lock lock(data.mtx); + data.cv.wait(lock, [&] { return data.done; }); + } - if (registration_id_ == 0) { - std::string const msg = - "Failed to register agent: " + std::string(err->message); - g_error_free(err); + if (!data.error.empty()) { g_dbus_node_info_unref(node_info_); - throw std::runtime_error(msg); + throw std::runtime_error(data.error); } } Agent::~Agent() { diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index ef0a18d..bfbcdb4 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -43,8 +43,7 @@ void ManaProperties::update(const gchar* key, GVariant* value) { Manager::Manager(DBus* dbus, const std::string& agent_path) : DBusProxy(dbus, SERVICE, MANAGER_PATH, MANAGER_INTERFACE), - agent_{ - std::unique_ptr(new Agent(dbus->connection(), agent_path))} { + agent_{std::unique_ptr(new Agent(dbus, agent_path))} { setup_agent(); get_technologies(); get_services(); From 699c836841dc2e7595f597a8959f82fae2314cc8 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Mon, 13 Apr 2026 14:21:33 +0200 Subject: [PATCH 15/19] code-format.yml: Set the develop branch Signed-off-by: Eduardo Gonzalez --- .github/workflows/code-format.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/code-format.yml b/.github/workflows/code-format.yml index 0dcc9b5..2a5c7e0 100644 --- a/.github/workflows/code-format.yml +++ b/.github/workflows/code-format.yml @@ -16,6 +16,8 @@ jobs: steps: - name: Checkout target repo uses: actions/checkout@v4 + with: + ref: develop # --- C/C++ --- - name: Install dependencies From ffbb90139c76f2cfaf08a65fc627c9223c24769f Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Mon, 13 Apr 2026 14:50:49 +0200 Subject: [PATCH 16/19] Fix logging for tests and examples Remove the use of the LCM_LOG_DEFAULT_ENABLED compile definition. LCM_LOG has to be enable always at runtime. Enable LCM_LOG at runtime for the tests and the example. This solves #24. Signed-off-by: Eduardo Gonzalez --- examples/CMakeLists.txt | 2 -- examples/connmanctl.cpp | 1 + include/amarula/log.hpp | 4 ---- tests/CMakeLists.txt | 5 ++--- tests/logging_init.cpp | 9 +++++++++ 5 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 tests/logging_init.cpp diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 5c49e5c..c0a2070 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -1,5 +1,3 @@ -add_compile_definitions(LCM_LOG_DEFAULT_ENABLED=1) - if(BUILD_CONNMAN) add_executable(connmanctl_dbus connmanctl.cpp) target_link_libraries(connmanctl_dbus PRIVATE GConnmanDbus) diff --git a/examples/connmanctl.cpp b/examples/connmanctl.cpp index 27c2c5d..74b1965 100644 --- a/examples/connmanctl.cpp +++ b/examples/connmanctl.cpp @@ -18,6 +18,7 @@ auto main() -> int { std::mutex cin_mutex; std::condition_variable cin_cv; bool connecting = false; + Amarula::Log::enable(true); Connman connman; const auto manager = connman.manager(); manager->onRequestInputPassphrase([&](const auto& service) -> auto { diff --git a/include/amarula/log.hpp b/include/amarula/log.hpp index 9069a41..01a7cb4 100644 --- a/include/amarula/log.hpp +++ b/include/amarula/log.hpp @@ -24,11 +24,7 @@ class Log { private: static auto state() -> std::atomic& { -#ifdef LCM_LOG_DEFAULT_ENABLED - static std::atomic flag{true}; -#else static std::atomic flag{false}; -#endif return flag; } }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 432499a..4278f0b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,5 +1,3 @@ -add_compile_definitions(LCM_LOG_DEFAULT_ENABLED=1) - include(FetchContent) FetchContent_Declare( googletest @@ -22,7 +20,8 @@ install( if(BUILD_CONNMAN) foreach(connman_test gconnman_clock_test gconnman_tech_test gconnman_serv_test) - add_executable(${connman_test} ${connman_test}.cpp qt_thread_bundle.hpp) + add_executable(${connman_test} ${connman_test}.cpp qt_thread_bundle.hpp + logging_init.cpp) target_link_libraries(${connman_test} PRIVATE GConnmanDbus gtest_main) target_include_directories(${connman_test} diff --git a/tests/logging_init.cpp b/tests/logging_init.cpp new file mode 100644 index 0000000..db28bd3 --- /dev/null +++ b/tests/logging_init.cpp @@ -0,0 +1,9 @@ +#include + +namespace { +struct EnableLogging { + EnableLogging() { Amarula::Log::enable(true); } +}; + +[[maybe_unused]] EnableLogging enable_logging; +} // namespace From 684c96ee9305bfa010cd80d3eb385563ba9896f2 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 15 Apr 2026 17:23:44 +0200 Subject: [PATCH 17/19] gagent: Do not block when requesting input Store a pointer to dbus to finish the request input in the gdbus managed context. Spawn a thread that execute the request input callback so the user can block as long as he wants for user input. Signed-off-by: Eduardo Gonzalez --- include/amarula/dbus/connman/gagent.hpp | 2 +- src/dbus/gconnman_agent.cpp | 47 +++++++++++++++++++------ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/include/amarula/dbus/connman/gagent.hpp b/include/amarula/dbus/connman/gagent.hpp index 4c0f7d0..9ce0707 100644 --- a/include/amarula/dbus/connman/gagent.hpp +++ b/include/amarula/dbus/connman/gagent.hpp @@ -13,7 +13,7 @@ class Connman; class Agent { GDBusNodeInfo *node_info_; guint registration_id_{0}; - GDBusConnection *connection_{nullptr}; + DBus *dbus_; std::string path_{"/net/amarula/gconnman/agent"}; explicit Agent(DBus *dbus, const std::string &path = std::string()); diff --git a/src/dbus/gconnman_agent.cpp b/src/dbus/gconnman_agent.cpp index 8078fcc..bc4d354 100644 --- a/src/dbus/gconnman_agent.cpp +++ b/src/dbus/gconnman_agent.cpp @@ -29,8 +29,7 @@ static constexpr const char *INTROSPECTION_XML = " " ""; -Agent::Agent(DBus *dbus, const std::string &path) - : connection_{dbus->connection()} { +Agent::Agent(DBus *dbus, const std::string &path) : dbus_{dbus} { if (!path.empty()) { path_ = path; } @@ -61,7 +60,7 @@ Agent::Agent(DBus *dbus, const std::string &path) GError *err = nullptr; data->self->registration_id_ = g_dbus_connection_register_object( - data->self->connection_, data->self->path_.c_str(), + data->self->dbus_->connection(), data->self->path_.c_str(), *(data->self->node_info_->interfaces), &INTERFACE_VTABLE, data->self, nullptr, &err); @@ -94,7 +93,7 @@ Agent::Agent(DBus *dbus, const std::string &path) } } Agent::~Agent() { - g_dbus_connection_unregister_object(connection_, registration_id_); + g_dbus_connection_unregister_object(dbus_->connection(), registration_id_); g_dbus_node_info_unref(node_info_); } @@ -120,17 +119,45 @@ void Agent::dispatch_method_call(GDBusMethodInvocation *invocation, GVariant *child_fields = g_variant_get_child_value(parameters, 1); service = g_variant_get_string(child_service, nullptr); + std::string service_str(service); fields = g_variant_ref(child_fields); g_variant_unref(child_service); g_variant_unref(child_fields); - auto *result = request_input_cb_(service, fields); - - GVariant *tuple = g_variant_new_tuple(&result, 1); - - g_dbus_method_invocation_return_value(invocation, tuple); - g_variant_unref(fields); + g_object_ref(invocation); + + std::thread([ctx = dbus_->context(), callback = request_input_cb_, + invocation, service_str = std::move(service_str), + fields]() { + GVariant *result = callback(service_str.c_str(), fields); + g_variant_unref(fields); + + struct Data { + GDBusMethodInvocation *invocation; + GVariant *result; + }; + + auto data = std::make_unique( + Data{.invocation = invocation, .result = result}); + + g_main_context_invoke_full( + ctx, G_PRIORITY_DEFAULT, + [](gpointer user_data) -> gboolean { + auto *data = static_cast(user_data); + + GVariant *tuple = g_variant_new_tuple(&data->result, 1); + g_dbus_method_invocation_return_value(data->invocation, + tuple); + + return G_SOURCE_REMOVE; + }, + data.release(), + [](gpointer user_data) { + std::unique_ptr data(static_cast(user_data)); + g_object_unref(data->invocation); + }); + }).detach(); return; } From 37fee8c73865400f9f06503cf5667049603f2345 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 16 Apr 2026 11:31:22 +0200 Subject: [PATCH 18/19] gconnman_manager: Do not lock services more than needed A lock is used to protect concurrent access to the services structure, once the service is found there is no need to continue locking the resource. The later allows to block the request input as long as the user want without locking the services and other members of the manager. Signed-off-by: Eduardo Gonzalez --- src/dbus/gconnman_manager.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index bfbcdb4..0ad2cd1 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -89,22 +89,29 @@ void Manager::process_services_changed( void Manager::setup_agent() { agent_->set_request_input_handler([this](const gchar* service_path, GVariant* fields) -> GVariant* { - std::lock_guard const lock(mtx_); GVariantBuilder builder; g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}")); - auto service_it = std::ranges::find_if( - services_, [&service_path](const auto& service) { - return service->objPath() == service_path; - }); - if (service_it != services_.end()) { + std::shared_ptr found_service; + { + std::lock_guard const lock(mtx_); + auto service_it = std::ranges::find_if( + services_, [&service_path](const auto& service) { + return service->objPath() == service_path; + }); + if (service_it != services_.end()) { + found_service = *service_it; + } + } + + if (found_service) { auto* parsed_fields = parse_fields(fields); auto input_requested = classify_input(parsed_fields); switch (input_requested) { case InputType::InputWpA2Passphrase: if (request_input_passphrase_cb_ != nullptr) { const auto passphrase = - request_input_passphrase_cb_(*service_it); + request_input_passphrase_cb_(found_service); g_variant_builder_add( &builder, "{sv}", "Passphrase", g_variant_new_string(passphrase.second.c_str())); @@ -113,7 +120,7 @@ void Manager::setup_agent() { case InputType::InputWpA2PassphraseWpsAlternative: if (request_input_passphrase_cb_ != nullptr) { const auto passphrase = - request_input_passphrase_cb_(*service_it); + request_input_passphrase_cb_(found_service); g_variant_builder_add( &builder, "{sv}", @@ -123,8 +130,8 @@ void Manager::setup_agent() { break; case InputType::InputHiddenNetworkName: if (request_input_hidden_network_name_cb_ != nullptr) { - const auto name = - request_input_hidden_network_name_cb_(*service_it); + const auto name = request_input_hidden_network_name_cb_( + found_service); GVariant* value = nullptr; if (name.first) { @@ -144,7 +151,7 @@ void Manager::setup_agent() { case InputType::InputWpaEnterprise: if (request_input_wpa_enterprise_cb_ != nullptr) { const auto identity_password = - request_input_wpa_enterprise_cb_(*service_it); + request_input_wpa_enterprise_cb_(found_service); g_variant_builder_add( &builder, "{sv}", "Identity", @@ -173,7 +180,7 @@ void Manager::setup_agent() { case InputType::InputWispr: if (request_input_wispr_enabled_cb_ != nullptr) { const auto user_password = - request_input_wispr_enabled_cb_(*service_it); + request_input_wispr_enabled_cb_(found_service); g_variant_builder_add( &builder, "{sv}", "Username", From 34c7b813b1b792831028babbd6bed0563feaadb7 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 16 Apr 2026 17:08:43 +0200 Subject: [PATCH 19/19] gconnman_manager: Fix parsing fields memory leak Unref missing GPtrArray memory and add function to properly free field description structs. Signed-off-by: Eduardo Gonzalez --- src/dbus/gconnman_manager.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/dbus/gconnman_manager.cpp b/src/dbus/gconnman_manager.cpp index 0ad2cd1..a1ec398 100644 --- a/src/dbus/gconnman_manager.cpp +++ b/src/dbus/gconnman_manager.cpp @@ -107,6 +107,7 @@ void Manager::setup_agent() { if (found_service) { auto* parsed_fields = parse_fields(fields); auto input_requested = classify_input(parsed_fields); + g_ptr_array_unref(parsed_fields); switch (input_requested) { case InputType::InputWpA2Passphrase: if (request_input_passphrase_cb_ != nullptr) { @@ -394,8 +395,20 @@ using FieldDescription = struct { gchar* requirement; }; +static void field_description_free(gpointer data) { + auto* desc = static_cast(data); + if (desc == nullptr) { + return; + } + + g_free(desc->field_name); + g_free(desc->type); + g_free(desc->requirement); + g_free(desc); +} + auto Manager::parse_fields(GVariant* fields) -> GPtrArray* { - GPtrArray* array = g_ptr_array_new_with_free_func(g_free); + GPtrArray* array = g_ptr_array_new_with_free_func(field_description_free); GVariantIter iter; const gchar* field_name = nullptr;