From ae3cd5be3948673d7dc6f4ae362e332a43ece858 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Wed, 15 Apr 2026 17:23:44 +0200 Subject: [PATCH 1/3] 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 a34598e4eea9dbe638f3332f7cac865f5cdeeb8d Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 16 Apr 2026 11:31:22 +0200 Subject: [PATCH 2/3] 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 895478f11bfed4a4d724b1bd9ab44a390c426f50 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Thu, 16 Apr 2026 17:08:43 +0200 Subject: [PATCH 3/3] 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;