system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-09-20 00:21:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

agent-manager: always invoke complete function for asynchronous nm_agent_manager_get_secrets()

Browse source 
Refactor agent-manager to always invoke the complete function for
nm_agent_manager_get_secrets().

In general, the complete function is always invoked asnychronously
when starting the operation. On the other hand, when cancelling the
operation or disposing the manager with pending operations, we now
(always) synchronously invoke the callback.

This makes it simpler for the user to reliably cancel the request
and perform potential cleanup.

This behavior bubbles up through NMSettingsConnection and NMActRequest,
and other callers that make directly or indicrectly make use of
nm_agent_manager_get_secrets().
This commit is contained in:
Thomas Haller 2015-09-03 10:24:09 +02:00
parent afb37d706f
commit 1b5664fed4
9 changed files with 190 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/devices/nm-device-ethernet.c
View file

@ -488,7 +488,9 @@ wired_secrets_cb (NMActRequest *req,
NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data);
NMDevice *dev = NM_DEVICE (self);
g_return_if_fail (req == nm_device_get_act_request (dev));
if (req != nm_device_get_act_request (dev))
return;
g_return_if_fail (nm_device_get_state (dev) == NM_DEVICE_STATE_NEED_AUTH);
g_return_if_fail (nm_act_request_get_connection (req) == connection);

4
src/devices/wifi/nm-device-wifi.c
View file

@ -1682,7 +1682,9 @@ wifi_secrets_cb (NMActRequest *req,
NMDevice *device = NM_DEVICE (user_data);
NMDeviceWifi *self = NM_DEVICE_WIFI (device);
g_return_if_fail (req == nm_device_get_act_request (device));
if (req != nm_device_get_act_request (device))
return;
g_return_if_fail (nm_device_get_state (device) == NM_DEVICE_STATE_NEED_AUTH);
g_return_if_fail (nm_act_request_get_connection (req) == connection);

34
src/devices/wwan/nm-modem.c
View file

@ -722,10 +722,7 @@ cancel_get_secrets (NMModem *self)
{
NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
if (priv->secrets_id) {
nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id);
priv->secrets_id = NULL;
}
nm_act_request_cancel_secrets (priv->act_request, priv->secrets_id);
}
static void
@ -742,13 +739,16 @@ modem_secrets_cb (NMActRequest *req,
priv->secrets_id = NULL;
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
return;
if (error)
nm_log_warn (LOGD_MB, "(%s): %s", nm_modem_get_uid (self), error->message);
g_signal_emit (self, signals[AUTH_RESULT], 0, error);
}
gboolean
void
nm_modem_get_secrets (NMModem *self,
const char *setting_name,
gboolean request_new,
@ -767,10 +767,8 @@ nm_modem_get_secrets (NMModem *self,
hint,
modem_secrets_cb,
self);
if (priv->secrets_id)
g_signal_emit (self, signals[AUTH_REQUESTED], 0);
return !!(priv->secrets_id);
g_return_if_fail (priv->secrets_id);
g_signal_emit (self, signals[AUTH_REQUESTED], 0);
}
/*****************************************************************************/
@ -790,8 +788,7 @@ nm_modem_act_stage1_prepare (NMModem *self,
NMDeviceStateReason *reason)
{
NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
NMActStageReturn ret;
GPtrArray *hints = NULL;
gs_unref_ptrarray GPtrArray *hints = NULL;
const char *setting_name = NULL;
NMSecretAgentGetSecretsFlags flags = NM_SECRET_AGENT_GET_SECRETS_FLAG_ALLOW_INTERACTION;
NMConnection *connection;
@ -820,18 +817,9 @@ nm_modem_act_stage1_prepare (NMModem *self,
hints ? g_ptr_array_index (hints, 0) : NULL,
modem_secrets_cb,
self);
if (priv->secrets_id) {
g_signal_emit (self, signals[AUTH_REQUESTED], 0);
ret = NM_ACT_STAGE_RETURN_POSTPONE;
} else {
*reason = NM_DEVICE_STATE_REASON_NO_SECRETS;
ret = NM_ACT_STAGE_RETURN_FAILURE;
}
if (hints)
g_ptr_array_free (hints, TRUE);
return ret;
g_return_val_if_fail (priv->secrets_id, NM_ACT_STAGE_RETURN_FAILURE);
g_signal_emit (self, signals[AUTH_REQUESTED], 0);
return NM_ACT_STAGE_RETURN_POSTPONE;
}
/*****************************************************************************/

8
src/devices/wwan/nm-modem.h
View file

@ -218,10 +218,10 @@ NMActStageReturn nm_modem_stage3_ip6_config_start (NMModem *modem,
void nm_modem_ip4_pre_commit (NMModem *modem, NMDevice *device, NMIP4Config *config);
gboolean nm_modem_get_secrets (NMModem *modem,
const char *setting_name,
gboolean request_new,
const char *hint);
void nm_modem_get_secrets (NMModem *modem,
const char *setting_name,
gboolean request_new,
const char *hint);
void nm_modem_deactivate (NMModem *modem, NMDevice *device);

53
src/nm-activation-request.c
View file

@ -96,12 +96,34 @@ get_secrets_cb (NMSettingsConnection *connection,
NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (info->self);
g_return_if_fail (info->call_id_s == call_id_s);
g_return_if_fail (g_slist_find (priv->secrets_calls, info));
priv->secrets_calls = g_slist_remove (priv->secrets_calls, info);
info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data);
if (info->callback)
info->callback (info->self, info, NM_CONNECTION (connection), error, info->callback_data);
g_free (info);
}
/**
* nm_act_request_get_secrets:
* @self:
* @setting_name:
* @flags:
* @hint:
* @callback:
* @callback_data:
*
* Asnychronously starts the request for secrets. This function cannot
* fail.
*
* The return call-id can be used to cancel the request. You are
* only allowed to cancel a still pending operation (once).
* The callback will always be invoked once, even for canceling
* or disposing of NMActRequest.
*
* Returns: a call-id.
*/
NMActRequestGetSecretsCallId
nm_act_request_get_secrets (NMActRequest *self,
const char *setting_name,
@ -152,7 +174,6 @@ nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId
{
NMActRequestPrivate *priv;
NMConnection *connection;
GSList *iter;
g_return_if_fail (self);
g_return_if_fail (NM_IS_ACT_REQUEST (self));
@ -160,21 +181,11 @@ nm_act_request_cancel_secrets (NMActRequest *self, NMActRequestGetSecretsCallId
priv = NM_ACT_REQUEST_GET_PRIVATE (self);
for (iter = priv->secrets_calls; iter; iter = g_slist_next (iter)) {
GetSecretsInfo *info = iter->data;
if (g_slist_find (priv->secrets_calls, call_id))
g_return_if_reached ();
/* Remove the matching info */
if (info == call_id) {
priv->secrets_calls = g_slist_remove_link (priv->secrets_calls, iter);
g_slist_free (iter);
connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self));
nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s);
g_free (info);
return;
}
}
g_return_if_reached ();
connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (self));
nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), call_id->call_id_s);
}
/********************************************************************/
@ -434,7 +445,6 @@ dispose (GObject *object)
{
NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE (object);
NMConnection *connection;
GSList *iter;
/* Clear any share rules */
if (priv->share_rules) {
@ -444,14 +454,13 @@ dispose (GObject *object)
/* Kill any in-progress secrets requests */
connection = nm_active_connection_get_connection (NM_ACTIVE_CONNECTION (object));
for (iter = priv->secrets_calls; connection && iter; iter = g_slist_next (iter)) {
GetSecretsInfo *info = iter->data;
while (priv->secrets_calls) {
GetSecretsInfo *info = priv->secrets_calls->data;
nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (connection), info->call_id_s);
g_free (info);
g_return_if_fail (!priv->secrets_calls || info != priv->secrets_calls->data);
}
g_slist_free (priv->secrets_calls);
priv->secrets_calls = NULL;
G_OBJECT_CLASS (nm_act_request_parent_class)->dispose (object);
}

12
src/ppp-manager/nm-ppp-manager.c
View file

@ -229,7 +229,7 @@ static void
remove_timeout_handler (NMPPPManager *manager)
{
NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager);
if (priv->ppp_timeout_handler) {
g_source_remove (priv->ppp_timeout_handler);
priv->ppp_timeout_handler = 0;
@ -241,11 +241,10 @@ cancel_get_secrets (NMPPPManager *self)
{
NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (self);
if (priv->secrets_id) {
if (priv->secrets_id)
nm_act_request_cancel_secrets (priv->act_req, priv->secrets_id);
priv->secrets_id = NULL;
}
priv->secrets_setting_name = NULL;
g_return_if_fail (!priv->secrets_id && !priv->secrets_setting_name);
}
static gboolean
@ -324,6 +323,9 @@ ppp_secrets_cb (NMActRequest *req,
g_return_if_fail (req == priv->act_req);
g_return_if_fail (call_id == priv->secrets_id);
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
goto out;
if (error) {
nm_log_warn (LOGD_PPP, "%s", error->message);
g_dbus_method_invocation_return_gerror (priv->pending_secrets_context, error);

119
src/settings/nm-agent-manager.c
View file

@ -453,8 +453,6 @@ struct _NMAgentManagerCallId {
guint idle_id;
gboolean completed;
union {
struct {
NMConnection *connection;
@ -524,16 +522,11 @@ request_free (Request *req)
if (req->idle_id)
g_source_remove (req->idle_id);
if (!req->completed) {
switch (req->request_type) {
case REQUEST_TYPE_CON_GET:
req->con.current_has_modify = FALSE;
if (req->current && req->current_call_id)
nm_secret_agent_cancel_secrets (req->current, req->current_call_id);
break;
default:
break;
}
if (req->current && req->current_call_id) {
/* cancel-secrets invokes the done-callback synchronously -- in which case
* the handler just return.
* Hence, we can proceed to free @req... */
nm_secret_agent_cancel_secrets (req->current, req->current_call_id);
}
g_object_unref (req->subject);
@ -551,16 +544,13 @@ request_free (Request *req)
}
static void
req_complete (Request *req,
GVariant *secrets,
const char *agent_dbus_owner,
const char *agent_username,
GError *error)
req_complete_release (Request *req,
GVariant *secrets,
const char *agent_dbus_owner,
const char *agent_username,
GError *error)
{
NMAgentManager *self = req->self;
NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
req->completed = TRUE;
switch (req->request_type) {
case REQUEST_TYPE_CON_GET:
@ -585,7 +575,38 @@ req_complete (Request *req,
g_return_if_reached ();
}
g_hash_table_remove (priv->requests, req);
request_free (req);
}
static void
req_complete_cancel (Request *req,
GQuark domain,
guint code,
const char *message)
{
GError *error = NULL;
nm_assert (req && req->self);
nm_assert (!g_hash_table_contains (NM_AGENT_MANAGER_GET_PRIVATE (req->self)->requests, req));
g_set_error_literal (&error, domain, code, message);
req_complete_release (req, NULL, NULL, NULL, error);
g_error_free (error);
}
static void
req_complete (Request *req,
GVariant *secrets,
const char *agent_dbus_owner,
const char *agent_username,
GError *error)
{
NMAgentManager *self = req->self;
NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
if (!g_hash_table_remove (priv->requests, req))
g_return_if_reached ();
req_complete_release (req, secrets, agent_dbus_owner, agent_username, error);
}
static void
@ -1133,6 +1154,29 @@ _con_get_try_complete_early (Request *req)
return FALSE;
}
/**
* nm_agent_manager_get_secrets:
* @self:
* @connection:
* @subject:
* @existing_secrets:
* @flags:
* @hints:
* @callback:
* @callback_data:
* @other_data2:
* @other_data3:
*
* Requests secrets for a connection.
*
* This function cannot fail. The callback will be invoked
* asynchrnously, but it will always be invoked exactly once.
* Even for cancellation and disposing of @self. In those latter
* cases, the callback is invoked synchrnously during the cancellation/
* disposal.
*
* Returns: a call-id to cancel the call.
*/
NMAgentManagerCallId
nm_agent_manager_get_secrets (NMAgentManager *self,
NMConnection *connection,
@ -1180,7 +1224,8 @@ nm_agent_manager_get_secrets (NMAgentManager *self,
req->con.get.other_data2 = other_data2;
req->con.get.other_data3 = other_data3;
g_hash_table_add (priv->requests, req);
if (!g_hash_table_add (priv->requests, req))
g_assert_not_reached ();
/* Kick off the request */
if (!(req->con.get.flags & NM_SECRET_AGENT_GET_SECRETS_FLAG_ONLY_SYSTEM))
@ -1200,6 +1245,8 @@ nm_agent_manager_cancel_secrets (NMAgentManager *self,
if (!g_hash_table_remove (NM_AGENT_MANAGER_GET_PRIVATE (self)->requests,
request_id))
g_return_if_reached ();
req_complete_cancel (request_id, G_IO_ERROR, G_IO_ERROR_CANCELLED, "Request cancelled");
}
/*************************************************************/
@ -1279,7 +1326,8 @@ nm_agent_manager_save_secrets (NMAgentManager *self,
nm_connection_get_id (connection),
subject);
req->con.connection = g_object_ref (connection);
g_hash_table_add (priv->requests, req);
if (!g_hash_table_add (priv->requests, req))
g_assert_not_reached ();
/* Kick off the request */
request_add_agents (self, req);
@ -1362,7 +1410,8 @@ nm_agent_manager_delete_secrets (NMAgentManager *self,
subject);
req->con.connection = g_object_ref (connection);
g_object_unref (subject);
g_hash_table_add (priv->requests, req);
if (!g_hash_table_add (priv->requests, req))
g_assert_not_reached ();
/* Kick off the request */
request_add_agents (self, req);
@ -1487,10 +1536,7 @@ nm_agent_manager_init (NMAgentManager *self)
NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self);
priv->agents = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref);
priv->requests = g_hash_table_new_full (g_direct_hash,
g_direct_equal,
(GDestroyNotify) request_free,
NULL);
priv->requests = g_hash_table_new (g_direct_hash, g_direct_equal);
}
static void
@ -1517,6 +1563,19 @@ dispose (GObject *object)
{
NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (object);
if (priv->requests) {
GHashTableIter iter;
Request *req;
g_hash_table_iter_init (&iter, priv->requests);
while (g_hash_table_iter_next (&iter, (gpointer *) &req, NULL)) {
g_hash_table_iter_remove (&iter);
req_complete_cancel (req, G_IO_ERROR, G_IO_ERROR_FAILED, "Disposing NMAgentManagerClass instance");
}
g_hash_table_destroy (priv->requests);
priv->requests = NULL;
}
g_slist_free_full (priv->chains, (GDestroyNotify) nm_auth_chain_unref);
priv->chains = NULL;
@ -1524,10 +1583,6 @@ dispose (GObject *object)
g_hash_table_destroy (priv->agents);
priv->agents = NULL;
}
if (priv->requests) {
g_hash_table_destroy (priv->requests);
priv->requests = NULL;
}
if (priv->auth_mgr) {
g_signal_handlers_disconnect_by_func (priv->auth_mgr,

50
src/settings/nm-settings-connection.c
View file

@ -117,7 +117,12 @@ typedef struct {
gboolean visible; /* Is this connection is visible by some session? */
GSList *reqs_int; /* in-progress internal secrets requests */
GSList *reqs_ext; /* in-progress external secrets requests (D-Bus) */
GSList *reqs_ext; /* in-progress external secrets requests (D-Bus). Note that
this list contains NMSettingsConnectionCallId, vs. NMAgentManagerCallId.
So, we should for example cancel the reqs_ext with nm_settings_connection_cancel_secrets()
instead of nm_agent_manager_cancel_secrets().
Actually, the difference doesn't matter, because both are indeed equivalent. */
/* Caches secrets from on-disk connections; were they not cached any
* call to nm_connection_clear_secrets() wipes them out and we'd have
@ -806,6 +811,16 @@ new_secrets_commit_cb (NMSettingsConnection *self,
}
}
static void
_callback_nop (NMSettingsConnection *self,
NMSettingsConnectionCallId call_id,
const char *agent_username,
const char *setting_name,
GError *error,
gpointer user_data)
{
}
static void
agent_secrets_done_cb (NMAgentManager *manager,
NMAgentManagerCallId call_id_a,
@ -830,14 +845,16 @@ agent_secrets_done_cb (NMAgentManager *manager,
gboolean agent_had_system = FALSE;
ForEachSecretFlags cmp_flags = { NM_SETTING_SECRET_FLAG_NONE, NM_SETTING_SECRET_FLAG_NONE };
priv->reqs_int = g_slist_remove (priv->reqs_int, call_id_a);
if (!callback)
callback = _callback_nop;
g_return_if_fail (g_slist_find (priv->reqs_int, call_id));
priv->reqs_int = g_slist_remove (priv->reqs_int, call_id);
if (error) {
_LOGD ("(%s:%p) secrets request error: (%d) %s",
setting_name,
call_id,
error->code,
error->message ? error->message : "(unknown)");
_LOGD ("(%s:%p) secrets request error: %s",
setting_name, call_id, error->message);
callback (self, call_id, NULL, setting_name, error, callback_data);
return;
@ -978,7 +995,13 @@ agent_secrets_done_cb (NMAgentManager *manager,
* Retrieves secrets from persistent storage and queries any secret agents for
* additional secrets.
*
* Returns: a call ID which may be used to cancel the ongoing secrets request
* With the returned call-id, the call can be cancelled. It is an error
* to cancel a call more then once or a call that already completed.
* The callback will always be invoked exactly once, also for cancellation
* and disposing of @self. In those latter cases, the callback will be invoked
* synchronously during cancellation/disposing.
*
* Returns: a call ID which may be used to cancel the ongoing secrets request.
**/
NMSettingsConnectionCallId
nm_settings_connection_get_secrets (NMSettingsConnection *self,
@ -1052,7 +1075,6 @@ nm_settings_connection_cancel_secrets (NMSettingsConnection *self,
_LOGD ("(%p) secrets canceled", call_id);
priv->reqs_int = g_slist_remove (priv->reqs_int, call_id);
nm_agent_manager_cancel_secrets (priv->agent_mgr, NM_AGENT_MANAGER_CALL_ID (call_id));
}
@ -1658,6 +1680,8 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self,
GDBusMethodInvocation *context = user_data;
GVariant *dict;
g_return_if_fail (g_slist_find (priv->reqs_ext, call_id));
priv->reqs_ext = g_slist_remove (priv->reqs_ext, call_id);
if (error)
@ -2361,8 +2385,11 @@ _cancel_all (NMAgentManager *agent_mgr, GSList **preqs)
while (*preqs) {
NMAgentManagerCallId call_id_a = (*preqs)->data;
*preqs = g_slist_delete_link (*preqs, *preqs);
/* Cancel will invoke the complete callback, which in
* turn deletes the current call-id from the list. */
nm_agent_manager_cancel_secrets (agent_mgr, call_id_a);
g_return_if_fail (!*preqs || (call_id_a != (*preqs)->data));
}
}
@ -2376,6 +2403,9 @@ dispose (GObject *object)
/* Cancel in-progress secrets requests */
if (priv->agent_mgr) {
/* although @reqs_ext theoretically contains NMSettingsConnectionCallId,
* we still cancel it with nm_agent_manager_cancel_secrets() -- it is
* actually correct. */
_cancel_all (priv->agent_mgr, &priv->reqs_ext);
_cancel_all (priv->agent_mgr, &priv->reqs_int);
}

8
src/vpn-manager/nm-vpn-connection.c
View file

@ -332,10 +332,8 @@ _set_vpn_state (NMVpnConnection *connection,
_state_to_ac_state (vpn_state));
/* Clear any in-progress secrets request */
if (priv->secrets_id) {
if (priv->secrets_id)
nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection), priv->secrets_id);
priv->secrets_id = NULL;
}
dispatcher_cleanup (connection);
@ -2024,6 +2022,9 @@ get_secrets_cb (NMSettingsConnection *connection,
priv->secrets_id = NULL;
if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
return;
if (error && priv->secrets_idx >= SECRETS_REQ_NEW) {
nm_log_err (LOGD_VPN, "Failed to request VPN secrets #%d: (%d) %s",
priv->secrets_idx + 1, error->code, error->message);
@ -2227,7 +2228,6 @@ dispose (GObject *object)
if (priv->secrets_id) {
nm_settings_connection_cancel_secrets (NM_SETTINGS_CONNECTION (priv->connection),
priv->secrets_id);
priv->secrets_id = NULL;
}
if (priv->cancellable) {