[asterisk-commits] mmichelson: branch group/CCSS r228416 - in /team/group/CCSS: include/asterisk...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Nov 6 11:03:27 CST 2009


Author: mmichelson
Date: Fri Nov  6 11:03:23 2009
New Revision: 228416

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=228416
Log:
Fix two major problems with regards to monitor destruction.

First, up until this point there was only a monitor destructor callback.
This would be called when the monitor was destroyed and was intended as a
way to clean up any private data allocated on the monitor. While this is
necessary, it doesn't quite do everything that we need to do. The reason is
that most monitor implementations are going to have to retain data for every
call for which CC may or will be requested. Up until this commit, that individual
per-call data had no means of being destroyed. The only way to accomplish it was
to wait until all agents were finished interacting with a particular monitor and
use the monitor's destructor callback to kill the allocated data. This commit
adds an instance_destructor callback for a monitor. This destructor is called
for each CC completion or failure on each device monitor. To drive home the point
that this is not meant to destroy data intrinsic to the monitor itself but rather
for a specific call, a monitor pointer is not supplied as a parameter for this callback,
only a core ID.

Second, and directly related to the first was that that if a CC transaction failed
prior to a monitor being created, there was no way for the called channel driver to
know to destroy any retained information regarding the call. The core instance destructor
has been adjusted to handle this situation now. If a core_instance is destroyed but no
monitor has yet been allocated for the call, we cheat a bit and call the would-be monitor's
instance_destructor callback. This way the called side's channel driver can deal with the
failure the same way no matter whether a monitor has actually been created or not.


Modified:
    team/group/CCSS/include/asterisk/ccss.h
    team/group/CCSS/main/ccss.c

Modified: team/group/CCSS/include/asterisk/ccss.h
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/include/asterisk/ccss.h?view=diff&rev=228416&r1=228415&r2=228416
==============================================================================
--- team/group/CCSS/include/asterisk/ccss.h (original)
+++ team/group/CCSS/include/asterisk/ccss.h Fri Nov  6 11:03:23 2009
@@ -530,6 +530,17 @@
 	 * the private_data itself.
 	 */
 	 void (*destructor)(void *monitor);
+	 /*!
+	  * Instance destroy callback
+	  *
+	  * This callback is called when a specific call completion transaction
+	  * fails or completes. The main difference between this and the monitor destructor
+	  * is that this will be called for every CC call that occurs, whereas the destructor
+	  * function is only called when all activities on a monitor are completed. To drive
+	  * the point home, this destructor only has a core ID as a parameter. Use this to
+	  * locate the correct data to destroy.
+	  */
+	 void (*instance_destructor)(const int core_id);
 };
 
 /*!

Modified: team/group/CCSS/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/main/ccss.c?view=diff&rev=228416&r1=228415&r2=228416
==============================================================================
--- team/group/CCSS/main/ccss.c (original)
+++ team/group/CCSS/main/ccss.c Fri Nov  6 11:03:23 2009
@@ -754,6 +754,7 @@
 	ast_log_dynamic_level(cc_logger_level, "Destroying link with parent %s and child %s\n",
 			link->parent->interface->name, link->child->interface->name);
 	ast_atomic_fetchadd_int(&link->child->num_requests, -1);
+	link->child->callbacks->instance_destructor(link->core_id);
 	link->child = cc_unref(link->child, "Unref link's child");
 	link->parent = cc_unref(link->parent, "Unref link's parent");
 	ast_free(link);
@@ -814,6 +815,7 @@
 static int cc_extension_monitor_unsuspend(struct ast_cc_monitor *monitor, const int core_id);
 static int cc_extension_monitor_cancel_available_timer(struct ast_cc_monitor *monitor, const int core_id, int *sched_id);
 static void cc_extension_monitor_destructor(void *monitor);
+static void cc_extension_monitor_instance_destructor(const int core_id);
 
 static struct ast_cc_monitor_callbacks extension_monitor_cbs = {
 	.type = "extension",
@@ -824,6 +826,7 @@
 	.unsuspend = cc_extension_monitor_unsuspend,
 	.cancel_available_timer = cc_extension_monitor_cancel_available_timer,
 	.destructor = cc_extension_monitor_destructor,
+	.instance_destructor = cc_extension_monitor_instance_destructor,
 };
 
 static int cc_extension_monitor_init(struct ast_cc_monitor *monitor, const int core_id)
@@ -936,6 +939,12 @@
 	return;
 }
 
+static void cc_extension_monitor_instance_destructor(const int core_id)
+{
+	/* Nothing special to do in this case */
+	return;
+}
+
 static int cc_generic_monitor_init(struct ast_cc_monitor *monitor, const int core_id);
 static int cc_generic_monitor_request_cc(struct ast_cc_monitor *monitor, const int core_id, struct ast_cc_monitor_link *parent_link);
 static int cc_generic_monitor_suspend(struct ast_cc_monitor *monitor, const int core_id);
@@ -943,6 +952,7 @@
 static int cc_generic_monitor_unsuspend(struct ast_cc_monitor *monitor, const int core_id);
 static int cc_generic_monitor_cancel_available_timer(struct ast_cc_monitor *monitor, const int core_id, int *sched_id);
 static void cc_generic_monitor_destructor(void *monitor);
+static void cc_generic_monitor_instance_destructor(const int core_id);
 
 static struct ast_cc_monitor_callbacks generic_monitor_cbs = {
 	.type = "generic",
@@ -953,6 +963,7 @@
 	.unsuspend = cc_generic_monitor_unsuspend,
 	.cancel_available_timer = cc_generic_monitor_cancel_available_timer,
 	.destructor = cc_generic_monitor_destructor,
+	.instance_destructor = cc_generic_monitor_instance_destructor,
 };
 
 /*!
@@ -1145,6 +1156,11 @@
 		gen_mon_pvt->sub = ast_event_unsubscribe(gen_mon_pvt->sub);
 	}
 	ast_free(gen_mon_pvt);
+	return;
+}
+
+static void cc_generic_monitor_instance_destructor(const int core_id)
+{
 	return;
 }
 
@@ -2031,10 +2047,43 @@
 {
 	struct cc_core_instance *core_instance = data;
 	ast_log_dynamic_level(cc_logger_level, "Destroying core instance %d\n", core_instance->core_id);
-	agent_destroy(core_instance->agent);
 	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
+		 * destructor function called directly with a NULL monitor argument. Since the
+		 * core_id will be present, the channel driver writer still will be able to
+		 * find the appropriate information to delete.
+		 */
+		/* 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);
+	}
+	agent_destroy(core_instance->agent);
 }
 
 static struct cc_core_instance *cc_core_init_instance(struct ast_channel *caller_chan,




More information about the asterisk-commits mailing list