[Asterisk-code-review] bridge softmix.c: Fix crash if channel fails to join mixing ... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review.

  https://gerrit.asterisk.org/2583

Change subject: bridge_softmix.c: Fix crash if channel fails to join mixing tech.
......................................................................

bridge_softmix.c: Fix crash if channel fails to join mixing tech.

softmix_bridge_join() failed because of an allocation failure.  To address
this, the softmix bridge technology now checks if the channel failed to
join softmix successfully.  In addition, the bridge now begins the process
of kicking the channel out of the bridge so we don't have channels
partially in the bridge for very long.

Change-Id: I97e2ade6a2bcd1214f24fb839fda948825b61a2b
---
M bridges/bridge_softmix.c
M include/asterisk/bridge_technology.h
M main/bridge.c
3 files changed, 17 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/83/2583/1

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index e3df18f..fe058e4 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -359,6 +359,9 @@
 	struct ast_format *slin_format;
 	int setup_fail;
 
+	/* The callers have already ensured that sc is never NULL. */
+	ast_assert(sc != NULL);
+
 	slin_format = ast_format_cache_get_slin_by_rate(rate);
 
 	ast_mutex_lock(&sc->lock);
@@ -714,7 +717,7 @@
 {
 	int res = 0;
 
-	if (!bridge->tech_pvt || (bridge_channel && !bridge_channel->tech_pvt)) {
+	if (!bridge->tech_pvt || !bridge_channel || !bridge_channel->tech_pvt) {
 		/* "Accept" the frame and discard it. */
 		return 0;
 	}
@@ -984,6 +987,11 @@
 		AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
 			struct softmix_channel *sc = bridge_channel->tech_pvt;
 
+			if (!sc) {
+				/* This channel failed to join successfully. */
+				continue;
+			}
+
 			/* Update the sample rate to match the bridge's native sample rate if necessary. */
 			if (update_all_rates) {
 				set_softmix_bridge_data(softmix_data->internal_rate, softmix_data->internal_mixing_interval, bridge_channel, 1);
@@ -1019,7 +1027,8 @@
 		AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
 			struct softmix_channel *sc = bridge_channel->tech_pvt;
 
-			if (bridge_channel->suspended) {
+			if (!sc || bridge_channel->suspended) {
+				/* This channel failed to join successfully or is suspended. */
 				continue;
 			}
 
diff --git a/include/asterisk/bridge_technology.h b/include/asterisk/bridge_technology.h
index 7de573a..7f5d746 100644
--- a/include/asterisk/bridge_technology.h
+++ b/include/asterisk/bridge_technology.h
@@ -107,6 +107,9 @@
 	 * \retval -1 on failure
 	 *
 	 * \note On entry, bridge is already locked.
+	 *
+	 * \note The bridge technology must tollerate a failed to join channel
+	 * until it can be kicked from the bridge.
 	 */
 	int (*join)(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
 	/*!
diff --git a/main/bridge.c b/main/bridge.c
index fd83cfb..dad7cfa 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -420,10 +420,12 @@
 		bridge->technology->name);
 	if (bridge->technology->join
 		&& bridge->technology->join(bridge, bridge_channel)) {
-		ast_debug(1, "Bridge %s: %p(%s) failed to join %s technology\n",
+		/* We cannot leave the channel partially in the bridge so we must kick it out */
+		ast_debug(1, "Bridge %s: %p(%s) failed to join %s technology (Kicking it out)\n",
 			bridge->uniqueid, bridge_channel, ast_channel_name(bridge_channel->chan),
 			bridge->technology->name);
 		bridge_channel->just_joined = 1;
+		ast_bridge_channel_leave_bridge(bridge_channel, BRIDGE_CHANNEL_STATE_END, 0);
 		return;
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/2583
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I97e2ade6a2bcd1214f24fb839fda948825b61a2b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>