[asterisk-commits] rmudgett: branch rmudgett/bridge_tasks r383537 - in /team/rmudgett/bridge_tas...

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Thu Mar 21 13:36:49 2013
New Revision: 383537

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=383537
Log:
* Convert hangup hooks list to a container for thread safety.

* Fail adding an interval hook if could not push it onto the heap.  

Modified:
    team/rmudgett/bridge_tasks/include/asterisk/bridging_features.h
    team/rmudgett/bridge_tasks/main/bridging.c

Modified: team/rmudgett/bridge_tasks/include/asterisk/bridging_features.h
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/bridge_tasks/include/asterisk/bridging_features.h?view=diff&rev=383537&r1=383536&r2=383537
==============================================================================
--- team/rmudgett/bridge_tasks/include/asterisk/bridging_features.h (original)
+++ team/rmudgett/bridge_tasks/include/asterisk/bridging_features.h Thu Mar 21 13:36:49 2013
@@ -194,10 +194,8 @@
 /* BUGBUG dtmf_hooks needs to be an ao2_container so it would be possible to iterate without keeping a lock */
 	/*! Attached DTMF feature hooks */
 	AST_LIST_HEAD_NOLOCK(, ast_bridge_hook) dtmf_hooks;
-/* BUGBUG hangup_hooks needs to be an ao2_container so it would be possible to iterate without keeping a lock */
-	/*! Attached hangup interception hooks */
-	AST_LIST_HEAD_NOLOCK(, ast_bridge_hook) hangup_hooks;
-/* BUGBUG use of interval_hooks needs to be made ao2 safe */
+	/*! Attached hangup interception hooks container */
+	struct ao2_container *hangup_hooks;
 	/*! Attached interval hooks */
 	struct ast_heap *interval_hooks;
 	/*! Used to determine when interval based features should be checked */

Modified: team/rmudgett/bridge_tasks/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/bridge_tasks/main/bridging.c?view=diff&rev=383537&r1=383536&r2=383537
==============================================================================
--- team/rmudgett/bridge_tasks/main/bridging.c (original)
+++ team/rmudgett/bridge_tasks/main/bridging.c Thu Mar 21 13:36:49 2013
@@ -564,20 +564,21 @@
 	features = bridge_channel->features;
 	if (features) {
 		struct ast_bridge_hook *hook;
+		struct ao2_iterator iter;
 
 		/* Run any hangup hooks. */
-		AST_LIST_TRAVERSE_SAFE_BEGIN(&features->hangup_hooks, hook, entry) {
+		iter = ao2_iterator_init(features->hangup_hooks, 0);
+		for (; (hook = ao2_iterator_next(&iter)); ao2_ref(hook, -1)) {
 			int failed;
 
 			failed = hook->callback(bridge_channel->bridge, bridge_channel, hook->hook_pvt);
 			if (failed) {
 				ast_debug(1, "Hangup hook %p is being removed from bridge channel %p(%s)\n",
 					hook, bridge_channel, ast_channel_name(bridge_channel->chan));
-				AST_LIST_REMOVE_CURRENT(entry);
-				ao2_ref(hook, -1);
+				ao2_unlink(features->hangup_hooks, hook);
 			}
 		}
-		AST_LIST_TRAVERSE_SAFE_END;
+		ao2_iterator_destroy(&iter);
 	}
 
 	/* Default hangup action. */
@@ -2698,6 +2699,7 @@
 	ast_bridge_hook_pvt_destructor destructor)
 {
 	struct ast_bridge_hook *hook;
+	int res;
 
 	/* Allocate new hook and setup it's various variables */
 	hook = bridge_hook_generic(sizeof(*hook), callback, hook_pvt, destructor);
@@ -2705,10 +2707,11 @@
 		return -1;
 	}
 
-	/* Once done we add it onto the list. */
-	AST_LIST_INSERT_TAIL(&features->hangup_hooks, hook, entry);
-
-	return 0;
+	/* Once done we put it in the container. */
+	res = ao2_link(features->hangup_hooks, hook) ? 0 : -1;
+	ao2_ref(hook, -1);
+
+	return res;
 }
 
 void ast_bridge_features_set_talk_detector(struct ast_bridge_features *features,
@@ -2728,6 +2731,7 @@
 	ast_bridge_hook_pvt_destructor destructor)
 {
 	struct ast_bridge_hook *hook;
+	int res;
 
 	if (!interval || !callback || !features || !features->interval_hooks) {
 		return -1;
@@ -2753,12 +2757,14 @@
 	ast_debug(1, "Putting interval hook %p with interval %u in the heap on features %p\n",
 		hook, hook->parms.timer.interval, features);
 	ast_heap_wrlock(features->interval_hooks);
-	if (ast_heap_push(features->interval_hooks, hook)) {
+	res = ast_heap_push(features->interval_hooks, hook);
+	if (res) {
+		/* Could not push the hook onto the heap. */
 		ao2_ref(hook, -1);
 	}
 	ast_heap_unlock(features->interval_hooks);
 
-	return 0;
+	return res ? -1 : 0;
 }
 
 int ast_bridge_features_enable(struct ast_bridge_features *features, enum ast_bridge_builtin_feature feature, const char *dtmf, void *config, ast_bridge_hook_pvt_destructor destructor)
@@ -2847,7 +2853,11 @@
 	AST_LIST_HEAD_INIT_NOLOCK(&features->dtmf_hooks);
 
 	/* Initialize the hangup hooks list, just in case */
-	AST_LIST_HEAD_INIT_NOLOCK(&features->hangup_hooks);
+	features->hangup_hooks =
+		ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_MUTEX, 0, NULL, NULL);
+	if (!features->hangup_hooks) {
+		return -1;
+	}
 
 	/* Initialize the interval hook heap */
 	features->interval_hooks = ast_heap_create(8, interval_hook_time_cmp,
@@ -2888,10 +2898,9 @@
 		features->talker_pvt_data = NULL;
 	}
 
-	/* Destroy each hangup hook. */
-	while ((hook = AST_LIST_REMOVE_HEAD(&features->hangup_hooks, entry))) {
-		ao2_ref(hook, -1);
-	}
+	/* Destroy the hangup hook container. */
+	ao2_cleanup(features->hangup_hooks);
+	features->hangup_hooks = NULL;
 
 	/* Destroy each DTMF feature hook. */
 	while ((hook = AST_LIST_REMOVE_HEAD(&features->dtmf_hooks, entry))) {