agents: fix removing requests from hash table while iterating it

GLib-CRITICAL **: g_hash_table_iter_next: assertion 'ri->version == ri->hash_table->version' failed

It is not allowed to modify hash table while it is iterated. Unfortunately,
request_remove_agent() may remove the request from the 'requests' hash table,
making it not usable in the loop hash table looping.

We need to store the request into a temporary list and call request_next_agent()
on them later (after the hash loop).

Test case:
1. start NM and nm-applet
2. activate a Wi-Fi WPA connection
3. nm-applet displays a dialog asking for a password
4. kill nm-applet
5. NetworkManager removes the nm-applet's secret agent
   and runs into removing the request from hash table in the
   iterating loop (via get_complete_cb)

 #0  get_complete_cb (parent=0x7f3f250f2970, secrets=0x0, agent_dbus_owner=0x0, agent_username=0x0, error=0x7f3f250f7830, user_data=0x7f3f25020e10)
     at settings/nm-agent-manager.c:1111
 #1  0x00007f3f23b46ea5 in req_complete_error (error=0x7f3f250f7830, req=0x7f3f250f2970) at settings/nm-agent-manager.c:509
 #2  request_next_agent (req=0x7f3f250f2970) at settings/nm-agent-manager.c:615
 #3  0x00007f3f23b48596 in request_remove_agent (agent=0x7f3f250f4a20, req=0x7f3f250f2970) at settings/nm-agent-manager.c:631
 #4  remove_agent (self=<optimized out>, owner=0x7f3f250dbff0 ":1.275") at settings/nm-agent-manager.c:130
 #5  0x00007f3f23b4868d in impl_agent_manager_unregister (self=0x7f3f25020e10, context=0x7f3f250f5480) at settings/nm-agent-manager.c:374

 #0  0x00007f3f1fb9c4e9 in g_logv (log_domain=0x7f3f1fbfef4e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fff156b77c0) at gmessages.c:989
 #1  0x00007f3f1fb9c63f in g_log (log_domain=log_domain@entry=0x7f3f1fbfef4e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL,
     format=format@entry=0x7f3f1fc0889a "%s: assertion '%s' failed") at gmessages.c:1025
 #2  0x00007f3f1fb9c679 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7f3f1fbfef4e "GLib",
     pretty_function=pretty_function@entry=0x7f3f1fc03c30 <__PRETTY_FUNCTION__.4571> "g_hash_table_iter_next",
     expression=expression@entry=0x7f3f1fc038f0 "ri->version == ri->hash_table->version") at gmessages.c:1034
 #3  0x00007f3f1fb849c0 in g_hash_table_iter_next (iter=<optimized out>, key=<optimized out>, value=<optimized out>) at ghash.c:733
 #4  0x00007f3f23b484e5 in remove_agent (self=<optimized out>, owner=0x7f3f250dbff0 ":1.275") at settings/nm-agent-manager.c:129
 #5  0x00007f3f23b4868d in impl_agent_manager_unregister (self=0x7f3f25020e10, context=0x7f3f250f5480) at settings/nm-agent-manager.c:374
This commit is contained in:
Jiří Klimeš
2013-11-21 10:31:28 +01:00
parent 308f2c08dd
commit 593f1aadec

View File

@@ -15,7 +15,7 @@
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Copyright (C) 2010 - 2011 Red Hat, Inc.
* Copyright (C) 2010 - 2013 Red Hat, Inc.
*/
#include <config.h>
@@ -74,7 +74,9 @@ static void request_add_agent (Request *req,
NMSecretAgent *agent,
NMSessionMonitor *session_monitor);
static void request_remove_agent (Request *req, NMSecretAgent *agent);
static void request_remove_agent (Request *req, NMSecretAgent *agent, GSList **pending_reqs);
static void request_next_agent (Request *req);
static void impl_agent_manager_register (NMAgentManager *self,
const char *identifier,
@@ -113,6 +115,7 @@ remove_agent (NMAgentManager *self, const char *owner)
NMSecretAgent *agent;
GHashTableIter iter;
gpointer data;
GSList *pending_reqs = NULL;
g_return_val_if_fail (owner != NULL, FALSE);
@@ -124,10 +127,17 @@ remove_agent (NMAgentManager *self, const char *owner)
nm_log_dbg (LOGD_AGENTS, "(%s) agent unregistered or disappeared",
nm_secret_agent_get_description (agent));
/* Remove this agent to any in-progress secrets requests */
/* Remove this agent from any in-progress secrets requests */
g_hash_table_iter_init (&iter, priv->requests);
while (g_hash_table_iter_next (&iter, NULL, &data))
request_remove_agent ((Request *) data, agent);
request_remove_agent ((Request *) data, agent, &pending_reqs);
/* We cannot call request_next_agent() from from within hash iterating loop,
* because it may remove the request from the hash table, which invalidates
* the iterator. So, only remove the agent from requests. And store the requests
* that should be sent to other agent to a temporary list to proceed afterwards.
*/
g_slist_free_full (pending_reqs, (GDestroyNotify) request_next_agent);
/* And dispose of the agent */
g_hash_table_remove (priv->agents, owner);
@@ -619,7 +629,7 @@ request_next_agent (Request *req)
}
static void
request_remove_agent (Request *req, NMSecretAgent *agent)
request_remove_agent (Request *req, NMSecretAgent *agent, GSList **pending_reqs)
{
g_return_if_fail (req != NULL);
g_return_if_fail (agent != NULL);
@@ -629,7 +639,7 @@ request_remove_agent (Request *req, NMSecretAgent *agent)
if (agent == req->current) {
nm_log_dbg (LOGD_AGENTS, "(%s) current agent removed from secrets request %p/%s",
nm_secret_agent_get_description (agent), req, req->detail);
request_next_agent (req);
*pending_reqs = g_slist_prepend (*pending_reqs, req);
} else {
nm_log_dbg (LOGD_AGENTS, "(%s) agent removed from secrets request %p/%s",
nm_secret_agent_get_description (agent), req, req->detail);