app_agent_pool: Fix initial module load agent device state reporting.

When the app_agent_pool module initially loads there is a race condition
between the thread loading agents.conf and the device state internal
processing thread.  If the device state internal processing thread handles
the agent creation state updates before the thread that loaded agents.conf
registers the device state provider callback then the cached agent state
is "Invalid".  When a consumer module like app_queue asks for the agent state
it gets the cached "Invalid" state instead of the real state from the provider.

* Moved loading the agents.conf configuration to the last thing setup by
app_agent_pool in load_module().  Now the device state provider callback
is registered before the config is loaded so the agent creation state
updates are guaranteed to get the initial device state.

* Removed some now redundant config cleanup on error in load_config().

* Added lock protection when accessing the device state in
agent_pvt_devstate_get() and eliminated the RAII_VAR() usage.

ASTERISK-24737 #close
Reported by: Steve Pitts

Review: https://reviewboard.asterisk.org/r/4390/
........

Merged revisions 431492 from http://svn.asterisk.org/svn/asterisk/branches/13


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@431493 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 17 insertions, 15 deletions

diff --git a/apps/app_agent_pool.c b/apps/app_agent_pool.c
index a36c77143..a8f2d2c1b 100644
--- a/apps/app_agent_pool.c
+++ b/apps/app_agent_pool.c
@@ -552,14 +552,10 @@ static int load_config(void)
 	aco_option_register(&cfg_info, "fullname", ACO_EXACT, agent_types, NULL, OPT_STRINGFIELD_T, 0, STRFLDSET(struct agent_cfg, full_name));
 
 	if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
-		goto error;
+		return -1;
 	}
 
 	return 0;
-
-error:
-	destroy_config();
-	return -1;
 }
 
 enum agent_state {
@@ -730,12 +726,17 @@ static struct ast_channel *agent_lock_logged(struct agent_pvt *agent)
  */
 static enum ast_device_state agent_pvt_devstate_get(const char *agent_id)
 {
-	RAII_VAR(struct agent_pvt *, agent, ao2_find(agents, agent_id, OBJ_KEY), ao2_cleanup);
+	enum ast_device_state dev_state = AST_DEVICE_INVALID;
+	struct agent_pvt *agent;
 
+	agent = ao2_find(agents, agent_id, OBJ_KEY);
 	if (agent) {
-		return agent->devstate;
+		agent_lock(agent);
+		dev_state = agent->devstate;
+		agent_unlock(agent);
+		ao2_ref(agent, -1);
 	}
-	return AST_DEVICE_INVALID;
+	return dev_state;
 }
 
 /*!
@@ -2642,7 +2643,7 @@ static int unload_module(void)
 	}
 
 	destroy_config();
-	ao2_ref(agents, -1);
+	ao2_cleanup(agents);
 	agents = NULL;
 	return 0;
 }
@@ -2656,12 +2657,6 @@ static int load_module(void)
 	if (!agents) {
 		return AST_MODULE_LOAD_FAILURE;
 	}
-	if (load_config()) {
-		ast_log(LOG_ERROR, "Unable to load config. Not loading module.\n");
-		ao2_ref(agents, -1);
-		agents = NULL;
-		return AST_MODULE_LOAD_DECLINE;
-	}
 
 	/* Init agent holding bridge v_table. */
 	bridge_init_agent_hold();
@@ -2687,6 +2682,13 @@ static int load_module(void)
 		unload_module();
 		return AST_MODULE_LOAD_FAILURE;
 	}
+
+	if (load_config()) {
+		ast_log(LOG_ERROR, "Unable to load config. Not loading module.\n");
+		unload_module();
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
 	return AST_MODULE_LOAD_SUCCESS;
 }