[asterisk-commits] rmudgett: branch group/CCSS r240321 - /team/group/CCSS/main/ccss.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Jan 14 19:19:22 CST 2010


Author: rmudgett
Date: Thu Jan 14 19:19:20 2010
New Revision: 240321

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=240321
Log:
CCSS core instance constructor failure fix.

*  The core instance destructor should not attempt to dereference the
agent if the agent failed to get created.
*  Protect find_agent_callbacks() from ast_channel_get_cc_config_params()
failure.

Modified:
    team/group/CCSS/main/ccss.c

Modified: team/group/CCSS/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/main/ccss.c?view=diff&rev=240321&r1=240320&r2=240321
==============================================================================
--- team/group/CCSS/main/ccss.c (original)
+++ team/group/CCSS/main/ccss.c Thu Jan 14 19:19:20 2010
@@ -779,12 +779,14 @@
 {
 	struct cc_agent_backend *backend;
 	const struct ast_cc_agent_callbacks *callbacks = NULL;
-	enum ast_cc_agent_policies agent_policy;
+	struct ast_cc_config_params *cc_params;
 	char type[32];
 
-	agent_policy = ast_get_cc_agent_policy(ast_channel_get_cc_config_params(chan));
-
-	switch (agent_policy) {
+	cc_params = ast_channel_get_cc_config_params(chan);
+	if (!cc_params) {
+		return NULL;
+	}
+	switch (ast_get_cc_agent_policy(cc_params)) {
 	case AST_CC_AGENT_GENERIC:
 		ast_copy_string(type, "generic", sizeof(type));
 		break;
@@ -1677,11 +1679,15 @@
 		return;
 	}
 
-	if (!(core_instance = find_cc_core_instance(cc_interfaces->core_id)) &&
-			!(core_instance = cc_core_init_instance(inbound, cc_interfaces->interface_tree, cc_interfaces->core_id))) {
-		cc_interfaces->ignore = 1;
-		ast_channel_unlock(inbound);
-		return;
+	core_instance = find_cc_core_instance(cc_interfaces->core_id);
+	if (!core_instance) {
+		core_instance = cc_core_init_instance(inbound, cc_interfaces->interface_tree,
+			cc_interfaces->core_id);
+		if (!core_instance) {
+			cc_interfaces->ignore = 1;
+			ast_channel_unlock(inbound);
+			return;
+		}
 	}
 
 	ast_channel_unlock(inbound);
@@ -2138,45 +2144,69 @@
 }
 
 
+/*!
+ * \internal
+ * \brief Walk the CC interface tree and destroy monitor instances.
+ * \since 1.8
+ *
+ * \param interface_tree Interface tree to kill any monitor instances.
+ * \param core_id core_id of the CC transaction.
+ *
+ * \details
+ * I'll admit, this is a bit evil.
+ *
+ * Picture the following scenario: A called party can support CC, so
+ * it queues an AST_CONTROL_CC frame. Now, the channel driver will likely
+ * need to retain some information about the call. This information is expected
+ * to be destroyed when the monitor's destructor is called.
+ * However, consider that the caller never requests CC (i.e.
+ * the offer timer expires). In this case, the monitor never gets created,
+ * and so the retained data in the called party's channel driver is leaked.
+ *
+ * The way that we deal with this scenario is to traverse the list of dialed
+ * devices in the agent's interface_tree. Each device will have its monitor instance
+ * destructor function called directly, even though no monitor was ever created.
+ *
+ * \return Nothing
+ */
+static void kill_interface_tree_monitors(struct ast_cc_interface_tree *interface_tree, const int core_id)
+{
+	const struct ast_cc_monitor_callbacks *monitor_callbacks;
+	struct cc_tree_item *tree_item_iter;
+
+	/* Locking is necessary here since a channel thread could possibly be mucking
+	 * with this list while we try to traverse it
+	 */
+	AST_LIST_LOCK(interface_tree);
+	AST_LIST_TRAVERSE(interface_tree, tree_item_iter, next) {
+		if (tree_item_iter->interface->monitor_class == AST_CC_DEVICE_MONITOR) {
+			monitor_callbacks =
+				find_monitor_callbacks(tree_item_iter->interface->monitor_type);
+			if (!monitor_callbacks) {
+				ast_log(LOG_WARNING,
+					"We may have a memory leak here. Couldn't find callbacks for monitor type %s\n",
+					tree_item_iter->interface->monitor_type);
+				continue;
+			}
+			monitor_callbacks->instance_destructor(core_id);
+		}
+	}
+	AST_LIST_UNLOCK(interface_tree);
+}
+
 static void cc_core_instance_destructor(void *data)
 {
 	struct cc_core_instance *core_instance = data;
 	ast_log_dynamic_level(cc_logger_level, "Destroying core instance %d\n", core_instance->core_id);
 	if (core_instance->monitor) {
 		prune_links(core_instance->monitor, core_instance->core_id, NULL);
-	} else {
-		const struct ast_cc_monitor_callbacks *monitor_callbacks;
-		struct cc_tree_item *tree_item_iter;
-		/* I'll admit, this is a bit evil.
-		 *
-		 * Picture the following scenario: A called party can support CC, so
-		 * it queues an AST_CONTROL_CC frame. Now, the channel driver will likely
-		 * need to retain some information about the call. This information is expected
-		 * to be destroyed when the monitor's destructor is called.
-		 * However, consider that the caller never requests CC (i.e.
-		 * the offer timer expires). In this case, the monitor never gets created,
-		 * and so the retained data in the called party's channel driver is leaked.
-		 *
-		 * The way that we deal with this scenario is to traverse the list of dialed
-		 * devices in the agent's interface_tree. Each device will have its monitor instance
-		 * destructor function called directly, even though no monitor was ever created.
-		 */
-		/* Locking is necessary here since a channel thread could possibly be mucking
-		 * with this list while we try to traverse it
-		 */
-		AST_LIST_LOCK(core_instance->agent->interface_tree);
-		AST_LIST_TRAVERSE(core_instance->agent->interface_tree, tree_item_iter, next) {
-			if (tree_item_iter->interface->monitor_class == AST_CC_DEVICE_MONITOR) {
-				if (!(monitor_callbacks = find_monitor_callbacks(tree_item_iter->interface->monitor_type))) {
-					ast_log(LOG_WARNING, "We may have a memory leak here. Couldn't find callbacks for monitor type %s\n", tree_item_iter->interface->monitor_type);
-					continue;
-				}
-				monitor_callbacks->instance_destructor(core_instance->core_id);
-			}
-		}
-		AST_LIST_UNLOCK(core_instance->agent->interface_tree);
-	}
-	cc_unref(core_instance->agent, "Core instance is done with the agent now\n");
+	} else if (core_instance->agent) {
+		kill_interface_tree_monitors(core_instance->agent->interface_tree,
+			core_instance->core_id);
+	}
+	if (core_instance->agent) {
+		cc_unref(core_instance->agent, "Core instance is done with the agent now");
+	}
 }
 
 static struct cc_core_instance *cc_core_init_instance(struct ast_channel *caller_chan,
@@ -2184,6 +2214,7 @@
 {
 	char caller[AST_CHANNEL_NAME];
 	struct cc_core_instance *core_instance;
+	struct ast_cc_config_params *cc_params;
 	long agent_count;
 	int recall_core_id;
 
@@ -2195,27 +2226,37 @@
 
 	ast_cc_is_recall(caller_chan, &recall_core_id);
 
+	cc_params = ast_channel_get_cc_config_params(caller_chan);
+	if (!cc_params) {
+		ast_log_dynamic_level(cc_logger_level, "Could not get CC parameters for %s\n",
+			caller);
+		kill_interface_tree_monitors(called_tree, core_id);
+		return NULL;
+	}
 	agent_count = count_agents(caller, recall_core_id);
-	if (agent_count >= ast_get_cc_max_agents(ast_channel_get_cc_config_params(caller_chan))) {
+	if (agent_count >= ast_get_cc_max_agents(cc_params)) {
 		ast_log_dynamic_level(cc_logger_level, "Caller %s already has the maximum number of agents configured\n", caller);
+		kill_interface_tree_monitors(called_tree, core_id);
 		return NULL;
 	}
 
 	/* Generic agents can only have a single outstanding CC request per caller. */
-	if (ast_get_cc_agent_policy(ast_channel_get_cc_config_params(caller_chan)) == AST_CC_AGENT_GENERIC &&
-			agent_count > 0) {
+	if (agent_count > 0 && ast_get_cc_agent_policy(cc_params) == AST_CC_AGENT_GENERIC) {
 		ast_log_dynamic_level(cc_logger_level, "Generic agents can only have a single outstanding request\n");
+		kill_interface_tree_monitors(called_tree, core_id);
 		return NULL;
 	}
 
 	/* Next, we need to create the core instance for this call */
 	if (!(core_instance = ao2_t_alloc(sizeof(*core_instance), cc_core_instance_destructor, "Creating core instance for CC"))) {
+		kill_interface_tree_monitors(called_tree, core_id);
 		return NULL;
 	}
 
 	core_instance->core_id = core_id;
 	if (!(core_instance->agent = cc_agent_init(caller_chan, caller, core_instance->core_id, called_tree))) {
 		cc_unref(core_instance, "Couldn't allocate agent, unref core_instance");
+		kill_interface_tree_monitors(called_tree, core_id);
 		return NULL;
 	}
 




More information about the asterisk-commits mailing list