Add name argument to BridgeWait() so multiple holding bridges may be used

Changes arguments for BridgeWait from BridgeWait(role, options) to
BridgeWait(bridge_name, role, options). Now multiple holding bridges may
be created and referenced by this application.

(closes issue ASTERISK-21922)
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/2642/


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

apps/app_bridgewait.c

@@ -47,6 +47,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/app.h"
 #include "asterisk/bridge.h"
 #include "asterisk/musiconhold.h"
+#include "asterisk/astobj2.h"
 
 /*** DOCUMENTATION
 	<application name="BridgeWait" language="en_US">
@@ -54,6 +55,12 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 			Put a call into the holding bridge.
 		</synopsis>
 		<syntax>
+			<parameter name="name">
+				<para>Name of the holding bridge to join. This is a handle for <literal>BridgeWait</literal>
+				only and does not affect the actual bridges that are created. If not provided,
+				the reserved name <literal>default</literal> will be used.
+				</para>
+			</parameter>
 			<parameter name="role" required="false">
 				<para>Defines the channel's purpose for entering the holding bridge. Values are case sensitive.
 				</para>
@@ -99,19 +106,81 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 		</syntax>
 		<description>
 			<para>This application places the incoming channel into a holding bridge.
-			The channel will then wait in the holding bridge until some
+			The channel will then wait in the holding bridge until some event occurs
-			event occurs which removes it from the holding bridge.</para>
+			which removes it from the holding bridge.</para>
+			<note><para>This application will answer calls which haven't already
+			been answered.</para></note>
 		</description>
 	</application>
  ***/
-/* BUGBUG Add bridge name/id parameter to specify which holding bridge to join (required) */
-/* BUGBUG The channel may or may not be answered with the r option. */
-/* BUGBUG You should not place an announcer into a holding bridge with unanswered channels. */
 
-static char *app = "BridgeWait";
+#define APP_NAME "BridgeWait"
-static struct ast_bridge *holding_bridge;
+#define DEFAULT_BRIDGE_NAME "default"
 
-AST_MUTEX_DEFINE_STATIC(bridgewait_lock);
+static struct ao2_container *wait_bridge_wrappers;
+
+struct wait_bridge_wrapper {
+	struct ast_bridge *bridge;     /*!< Bridge being wrapped by this wrapper */
+	char name[0];                  /*!< Name of the holding bridge wrapper */
+};
+
+static void wait_bridge_wrapper_destructor(void *obj)
+{
+	struct wait_bridge_wrapper *wrapper = obj;
+	if (wrapper->bridge) {
+		ast_bridge_destroy(wrapper->bridge);
+	}
+}
+
+static struct wait_bridge_wrapper *wait_bridge_wrapper_find_by_name(const char *bridge_name)
+{
+	return ao2_find(wait_bridge_wrappers, bridge_name, OBJ_KEY);
+}
+
+static int wait_bridge_hash_fn(const void *obj, const int flags)
+{
+	const struct wait_bridge_wrapper *entry;
+	const char *key;
+
+	switch (flags & (OBJ_POINTER | OBJ_KEY | OBJ_PARTIAL_KEY)) {
+	case OBJ_KEY:
+		key = obj;
+		return ast_str_hash(key);
+	case OBJ_POINTER:
+		entry = obj;
+		return ast_str_hash(entry->name);
+	default:
+		/* Hash can only work on something with a full key. */
+		ast_assert(0);
+		return 0;
+	}
+}
+
+static int wait_bridge_sort_fn(const void *obj_left, const void *obj_right, const int flags)
+{
+	const struct wait_bridge_wrapper *left = obj_left;
+	const struct wait_bridge_wrapper *right = obj_right;
+	const char *right_key = obj_right;
+	int cmp;
+
+	switch (flags & (OBJ_POINTER | OBJ_KEY | OBJ_PARTIAL_KEY)) {
+	case OBJ_POINTER:
+		right_key = right->name;
+		/* Fall through */
+	case OBJ_KEY:
+		cmp = strcmp(left->name, right_key);
+		break;
+	case OBJ_PARTIAL_KEY:
+		cmp = strncmp(left->name, right_key, strlen(right_key));
+		break;
+	default:
+		/* Sort can only work on something with a full or partial key. */
+		ast_assert(0);
+		cmp = 0;
+		break;
+	}
+	return cmp;
+}
 
 enum bridgewait_flags {
 	MUXFLAG_MOHCLASS = (1 << 0),
@@ -169,6 +238,7 @@ static int apply_option_moh(struct ast_channel *chan, const char *class_arg)
 static int apply_option_entertainment(struct ast_channel *chan, const char *entertainment_arg)
 {
 	char entertainment = entertainment_arg[0];
+
 	switch (entertainment) {
 	case 'm':
 		return ast_channel_set_bridge_role_option(chan, "holding_participant", "idle_mode", "musiconhold");
@@ -232,6 +302,98 @@ static int process_options(struct ast_channel *chan, struct ast_flags *flags, ch
 	return 0;
 }
 
+/*!
+ * \internal
+ * \since 12.0.0
+ * \brief Allocate a new holding bridge wrapper with the given bridge name and bridge ID.
+ *
+ * \param bridge_name name of the bridge wrapper
+ * \param bridge the bridge being wrapped
+ *
+ * \retval Pointer to the newly allocated holding bridge wrapper
+ * \retval NULL if allocation failed. The bridge will be destroyed if this function fails.
+ */
+static struct wait_bridge_wrapper *wait_bridge_wrapper_alloc(const char *bridge_name, struct ast_bridge *bridge)
+{
+	struct wait_bridge_wrapper *bridge_wrapper;
+
+	bridge_wrapper = ao2_alloc_options(sizeof(*bridge_wrapper) + strlen(bridge_name) + 1,
+		wait_bridge_wrapper_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK);
+
+	if (!bridge_wrapper) {
+		ast_bridge_destroy(bridge);
+		return NULL;
+	}
+
+	strcpy(bridge_wrapper->name, bridge_name);
+	bridge_wrapper->bridge = bridge;
+
+	if (!ao2_link(wait_bridge_wrappers, bridge_wrapper)) {
+		ao2_cleanup(bridge_wrapper);
+		return NULL;
+	}
+
+	return bridge_wrapper;
+}
+
+static struct wait_bridge_wrapper *get_wait_bridge_wrapper(const char *bridge_name)
+{
+	struct wait_bridge_wrapper * wrapper;
+	struct ast_bridge *bridge = NULL;
+
+	SCOPED_AO2LOCK(lock, wait_bridge_wrappers);
+
+	if ((wrapper = wait_bridge_wrapper_find_by_name(bridge_name))) {
+		return wrapper;
+	}
+
+	bridge = ast_bridge_base_new(AST_BRIDGE_CAPABILITY_HOLDING,
+		AST_BRIDGE_FLAG_MERGE_INHIBIT_TO | AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM
+		| AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM | AST_BRIDGE_FLAG_TRANSFER_PROHIBITED
+		| AST_BRIDGE_FLAG_DISSOLVE_EMPTY);
+
+	if (!bridge) {
+		return NULL;
+	}
+
+	/* The bridge reference is unconditionally passed. */
+	return wait_bridge_wrapper_alloc(bridge_name, bridge);
+}
+
+/*!
+ * \internal
+ * \since 12.0.0
+ * \brief If we are down to the last reference of a wrapper and it's still contained within the list, remove it from the list.
+ *
+ * \param wrapper reference to wait bridge wrapper being checked for list culling - will be cleared on exit
+ */
+static void wait_wrapper_removal(struct wait_bridge_wrapper *wrapper)
+{
+	if (!wrapper) {
+		return;
+	}
+
+	ao2_lock(wait_bridge_wrappers);
+	if (ao2_ref(wrapper, 0) == 2) {
+		/* Either we have the only real reference or else wrapper isn't in the container anyway. */
+		ao2_unlink(wait_bridge_wrappers, wrapper);
+	}
+	ao2_unlock(wait_bridge_wrappers);
+
+	ao2_cleanup(wrapper);
+}
+
+/*!
+ * \internal
+ * \since 12.0.0
+ * \brief Application callback for the bridgewait application
+ *
+ * \param chan channel running the application
+ * \param data Arguments to the application
+ *
+ * \retval 0 Ran successfully and the call didn't hang up
+ * \retval -1 Failed or the call was hung up by the time the channel exited the holding bridge
+ */
 static enum wait_bridge_roles validate_role(const char *role)
 {
 	if (!strcmp(role, "participant")) {
@@ -245,33 +407,28 @@ static enum wait_bridge_roles validate_role(const char *role)
 
 static int bridgewait_exec(struct ast_channel *chan, const char *data)
 {
+	char *bridge_name = DEFAULT_BRIDGE_NAME;
 	struct ast_bridge_features chan_features;
 	struct ast_flags flags = { 0 };
 	char *parse;
+	int bridge_join_failed = 0;
 	enum wait_bridge_roles role = ROLE_PARTICIPANT;
 	char *opts[OPT_ARG_ARRAY_SIZE] = { NULL, };
 
 	AST_DECLARE_APP_ARGS(args,
+		AST_APP_ARG(name);
 		AST_APP_ARG(role);
 		AST_APP_ARG(options);
 		AST_APP_ARG(other);		/* Any remaining unused arguments */
 	);
 
-	ast_mutex_lock(&bridgewait_lock);
-	if (!holding_bridge) {
-		holding_bridge = ast_bridge_base_new(AST_BRIDGE_CAPABILITY_HOLDING,
-			AST_BRIDGE_FLAG_MERGE_INHIBIT_TO | AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM
-				| AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM | AST_BRIDGE_FLAG_TRANSFER_PROHIBITED);
-	}
-	ast_mutex_unlock(&bridgewait_lock);
-	if (!holding_bridge) {
-		ast_log(LOG_ERROR, "Could not create holding bridge for '%s'.\n", ast_channel_name(chan));
-		return -1;
-	}
-
 	parse = ast_strdupa(data);
 	AST_STANDARD_APP_ARGS(args, parse);
 
+	if (!ast_strlen_zero(args.name)) {
+		bridge_name = args.name;
+	}
+
 	if (!ast_strlen_zero(args.role)) {
 		role = validate_role(args.role);
 		if (role == ROLE_INVALID) {
@@ -282,6 +439,8 @@ static int bridgewait_exec(struct ast_channel *chan, const char *data)
 
 	if (ast_bridge_features_init(&chan_features)) {
 		ast_bridge_features_cleanup(&chan_features);
+		ast_log(LOG_ERROR, "'%s' failed to enter the waiting bridge - could not set up channel features\n",
+			ast_channel_name(chan));
 		return -1;
 	}
 
@@ -289,28 +448,66 @@ static int bridgewait_exec(struct ast_channel *chan, const char *data)
 		ast_app_parse_options(bridgewait_opts, &flags, opts, args.options);
 	}
 
+	/* Answer the channel if needed */
+	if (ast_channel_state(chan) != AST_STATE_UP) {
+		ast_answer(chan);
+	}
+
 	if (process_options(chan, &flags, opts, &chan_features, role)) {
 		ast_bridge_features_cleanup(&chan_features);
 		return -1;
 	}
 
-	ast_bridge_join(holding_bridge, chan, NULL, &chan_features, NULL, 0);
+	for (;;) {
+		RAII_VAR(struct wait_bridge_wrapper *, bridge_wrapper, get_wait_bridge_wrapper(bridge_name), wait_wrapper_removal);
+
+		if (!bridge_wrapper) {
+			ast_log(LOG_WARNING, "Failed to find or create waiting bridge '%s' for '%s'.\n", bridge_name, ast_channel_name(chan));
+			bridge_join_failed = 1;
+			break;
+		}
+
+		ast_verb(3, "%s is entering waiting bridge %s:%s\n", ast_channel_name(chan), bridge_name, bridge_wrapper->bridge->uniqueid);
+
+		if (ast_bridge_join(bridge_wrapper->bridge, chan, NULL, &chan_features, NULL, 0)) {
+			/* It's possible for a holding bridge to vanish out from under us since we can't lock it.
+			 * Unlink the wrapper and then loop if the bridge we try to enter is dissolved. */
+			ast_verb(3, "Waiting bridge '%s:%s' is no longer joinable. Creating new bridge and trying again.\n",
+				bridge_name, bridge_wrapper->bridge->uniqueid);
+			ao2_unlink(wait_bridge_wrappers, bridge_wrapper);
+			continue;
+		}
+
+		break;
+	}
 
 	ast_bridge_features_cleanup(&chan_features);
+
+	if (bridge_join_failed) {
+		return -1;
+	}
+
 	return ast_check_hangup_locked(chan) ? -1 : 0;
 }
 
 static int unload_module(void)
 {
-	ao2_cleanup(holding_bridge);
+	ao2_cleanup(wait_bridge_wrappers);
-	holding_bridge = NULL;
 
-	return ast_unregister_application(app);
+	return ast_unregister_application(APP_NAME);
 }
 
 static int load_module(void)
 {
-	return ast_register_application_xml(app, bridgewait_exec);
+	wait_bridge_wrappers = ao2_container_alloc_hash(
+		AO2_ALLOC_OPT_LOCK_MUTEX, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT,
+		37, wait_bridge_hash_fn, wait_bridge_sort_fn, NULL);
+
+	if (!wait_bridge_wrappers) {
+		return -1;
+	}
+
+	return ast_register_application_xml(APP_NAME, bridgewait_exec);
 }
 
 AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Place the channel into a holding bridge application");

include/asterisk/bridge.h

@@ -428,7 +428,8 @@ void ast_bridge_notify_masquerade(struct ast_channel *chan);
  * \note Absolutely _NO_ locks should be held before calling
  * this function since it blocks.
  *
- * \retval state that channel exited the bridge with
+ * \retval 0 if the channel successfully joined the bridge before it exited.
+ * \retval -1 if the channel failed to join the bridge
  *
  * Example usage:
  *
@@ -447,7 +448,7 @@ void ast_bridge_notify_masquerade(struct ast_channel *chan);
  * If channel specific features are enabled a pointer to the features structure
  * can be specified in the features parameter.
  */
-enum bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
+int ast_bridge_join(struct ast_bridge *bridge,
 	struct ast_channel *chan,
 	struct ast_channel *swap,
 	struct ast_bridge_features *features,

include/asterisk/bridge_channel_internal.h

@@ -117,11 +117,14 @@ void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel);
  *
  * \param bridge_channel The Channel in the bridge
  *
+ * \retval 0 bridge channel successfully joined the bridge
+ * \retval -1 bridge channel failed to join the bridge
+ *
  * \note This API call starts the bridge_channel's processing of events while
  * it is in the bridge. It will return when the channel has been instructed to
  * leave the bridge.
  */
-void bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel);
+int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel);
 
 /*!
  * \internal

main/bridge.c

@@ -1420,7 +1420,7 @@ void ast_bridge_notify_masquerade(struct ast_channel *chan)
  * Need to update the features parameter doxygen when this
  * change is made to be like ast_bridge_impart().
  */
-enum bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
+int ast_bridge_join(struct ast_bridge *bridge,
 	struct ast_channel *chan,
 	struct ast_channel *swap,
 	struct ast_bridge_features *features,
@@ -1428,21 +1428,21 @@ enum bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
 	int pass_reference)
 {
 	struct ast_bridge_channel *bridge_channel;
-	enum bridge_channel_state state;
+	int res;
 
 	bridge_channel = bridge_channel_internal_alloc(bridge);
 	if (pass_reference) {
 		ao2_ref(bridge, -1);
 	}
 	if (!bridge_channel) {
-		state = BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE;
+		res = -1;
 		goto join_exit;
 	}
 /* BUGBUG features cannot be NULL when passed in. When it is changed to allocated we can do like ast_bridge_impart() and allocate one. */
 	ast_assert(features != NULL);
 	if (!features) {
 		ao2_ref(bridge_channel, -1);
-		state = BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE;
+		res = -1;
 		goto join_exit;
 	}
 	if (tech_args) {
@@ -1458,8 +1458,7 @@ enum bridge_channel_state ast_bridge_join(struct ast_bridge *bridge,
 	bridge_channel->swap = swap;
 	bridge_channel->features = features;
 
-	bridge_channel_internal_join(bridge_channel);
+	res = bridge_channel_internal_join(bridge_channel);
-	state = bridge_channel->state;
 
 	/* Cleanup all the data in the bridge channel after it leaves the bridge. */
 	ast_channel_lock(chan);
@@ -1481,7 +1480,7 @@ join_exit:;
 		ast_softhangup_nolock(chan, AST_SOFTHANGUP_ASYNCGOTO);
 		ast_channel_unlock(chan);
 	}
-	return state;
+	return res;
 }
 
 /*! \brief Thread responsible for imparted bridged channels to be departed */

main/bridge_channel.c

@@ -1794,8 +1794,9 @@ static void bridge_channel_event_join_leave(struct ast_bridge_channel *bridge_ch
 }
 
 /*! \brief Join a channel to a bridge and handle anything the bridge may want us to do */
-void bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
+int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
 {
+	int res = 0;
 	ast_format_copy(&bridge_channel->read_format, ast_channel_readformat(bridge_channel->chan));
 	ast_format_copy(&bridge_channel->write_format, ast_channel_writeformat(bridge_channel->chan));
 
@@ -1826,6 +1827,7 @@ void bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
 
 	if (bridge_channel_internal_push(bridge_channel)) {
 		ast_bridge_channel_leave_bridge(bridge_channel, BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE);
+		res = -1;
 	}
 	bridge_reconfigured(bridge_channel->bridge, 1);
 
@@ -1881,6 +1883,8 @@ void bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
 	ast_channel_unlock(bridge_channel->chan);
 
 	ast_bridge_channel_restore_formats(bridge_channel);
+
+	return res;
 }
 
 int bridge_channel_internal_queue_blind_transfer(struct ast_channel *transferee,