Merge changes from topic 'system_stress_patches' into 13

* changes:
  Bridge system: Fix memory leaks and double frees on impart failure.
  bridge_softmix.c: Fix crash if channel fails to join mixing tech.

apps/confbridge/conf_chan_announce.c

@@ -199,7 +199,6 @@ int conf_announce_channel_push(struct ast_channel *ast)
 	/* Impart the output channel into the bridge */
 	if (ast_bridge_impart(p->bridge, chan, NULL, features,
 		AST_BRIDGE_IMPART_CHAN_DEPARTABLE)) {
-		ast_bridge_features_destroy(features);
 		ast_channel_unref(chan);
 		return -1;
 	}

bridges/bridge_softmix.c

@@ -359,6 +359,9 @@ static void set_softmix_bridge_data(int rate, int interval, struct ast_bridge_ch
 	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 @@ static int softmix_bridge_write(struct ast_bridge *bridge, struct ast_bridge_cha
 {
 	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 @@ static int softmix_mixing_loop(struct ast_bridge *bridge)
 		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 @@ static int softmix_mixing_loop(struct ast_bridge *bridge)
 		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;
 			}
 

include/asterisk/bridge_technology.h

@@ -107,6 +107,9 @@ struct ast_bridge_technology {
 	 * \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);
 	/*!

include/asterisk/features.h

@@ -51,6 +51,7 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
 /*!
  * \brief Bridge a call, and add additional flags to the bridge
  *
+ * \details
  * This does the same thing as \ref ast_bridge_call, except that once the bridge
  * is created, the provided flags are set on the bridge. The provided flags are
  * added to the bridge's flags; they will not clear any flags already set.
@@ -70,6 +71,7 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
  * \brief Add an arbitrary channel to a bridge
  * \since 12.0.0
  *
+ * \details
  * The channel that is being added to the bridge can be in any state: unbridged,
  * bridged, answered, unanswered, etc. The channel will be added asynchronously,
  * meaning that when this function returns once the channel has been added to
@@ -87,11 +89,16 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
  * \param features Features for this channel in the bridge
  * \param play_tone Indicates if a tone should be played to the channel
  * \param xfersound Sound that should be used to indicate transfer with play_tone
+ *
+ * \note The features parameter must be NULL or obtained by
+ * ast_bridge_features_new().  You must not dereference features
+ * after calling even if the call fails.
+ *
  * \retval 0 Success
  * \retval -1 Failure
  */
 int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
-		struct ast_bridge_features *features, int play_tone, const char *xfersound);
+	struct ast_bridge_features *features, int play_tone, const char *xfersound);
 
 
 

main/bridge.c

@@ -420,10 +420,12 @@ static void bridge_channel_complete_join(struct ast_bridge *bridge, struct ast_b
 		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;
 	}
 
@@ -2485,6 +2487,9 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
 	if (chan_bridge) {
 		struct ast_bridge_channel *bridge_channel;
 
+		/* The channel is in a bridge so it is not getting any new features. */
+		ast_bridge_features_destroy(features);
+
 		ast_bridge_lock_both(bridge, chan_bridge);
 		bridge_channel = bridge_find_channel(chan_bridge, chan);
 
@@ -2507,9 +2512,6 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
 		bridge_dissolve_check_stolen(chan_bridge, bridge_channel);
 		ast_bridge_unlock(chan_bridge);
 		ast_bridge_unlock(bridge);
-
-		/* The channel was in a bridge so it is not getting any new features. */
-		ast_bridge_features_destroy(features);
 	} else {
 		/* Slightly less easy case. We need to yank channel A from
 		 * where he currently is and impart him into our bridge.
@@ -2517,6 +2519,7 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
 		yanked_chan = ast_channel_yank(chan);
 		if (!yanked_chan) {
 			ast_log(LOG_WARNING, "Could not gain control of channel %s\n", ast_channel_name(chan));
+			ast_bridge_features_destroy(features);
 			return -1;
 		}
 		if (ast_channel_state(yanked_chan) != AST_STATE_UP) {

main/core_unreal.c

@@ -808,7 +808,6 @@ int ast_unreal_channel_push_to_bridge(struct ast_channel *ast, struct ast_bridge
 	/* Impart the semi2 channel into the bridge */
 	if (ast_bridge_impart(bridge, chan, NULL, features,
 		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
-		ast_bridge_features_destroy(features);
 		ast_channel_unref(chan);
 		return -1;
 	}

main/features.c

@@ -1104,7 +1104,6 @@ static int bridge_exec(struct ast_channel *chan, const char *data)
 		xfer_cfg ? xfer_cfg->xfersound : NULL);
 	ao2_cleanup(xfer_cfg);
 	if (bridge_add_failed) {
-		ast_bridge_features_destroy(peer_features);
 		ast_bridge_features_cleanup(&chan_features);
 		ast_bridge_destroy(bridge, 0);
 		goto done;

tests/test_channel_feature_hooks.c

@@ -40,6 +40,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/bridge.h"
 #include "asterisk/bridge_basic.h"
 #include "asterisk/features.h"
+#include "asterisk/format_cache.h"
 
 #define TEST_CATEGORY "/channels/features/"
 
@@ -47,6 +48,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #define TEST_BACKEND_NAME "Features Test Logging"
 
+#define TEST_CHANNEL_FORMAT		ast_format_slin
+
 /*! \brief A channel technology used for the unit tests */
 static struct ast_channel_tech test_features_chan_tech = {
 	.type = CHANNEL_TECH_NAME,
@@ -94,6 +97,11 @@ static void wait_for_unbridged(struct ast_channel *channel)
 #define START_CHANNEL(channel, name, number) do { \
 	channel = ast_channel_alloc(0, AST_STATE_UP, number, name, number, number, \
 		"default", NULL, NULL, 0, CHANNEL_TECH_NAME "/" name); \
+	ast_channel_nativeformats_set(channel, test_features_chan_tech.capabilities); \
+	ast_channel_set_rawwriteformat(channel, TEST_CHANNEL_FORMAT); \
+	ast_channel_set_rawreadformat(channel, TEST_CHANNEL_FORMAT); \
+	ast_channel_set_writeformat(channel, TEST_CHANNEL_FORMAT); \
+	ast_channel_set_readformat(channel, TEST_CHANNEL_FORMAT); \
 	ast_channel_unlock(channel); \
 	} while (0)
 
@@ -329,12 +337,19 @@ static int unload_module(void)
 	AST_TEST_UNREGISTER(test_features_channel_interval);
 
 	ast_channel_unregister(&test_features_chan_tech);
+	ao2_cleanup(test_features_chan_tech.capabilities);
+	test_features_chan_tech.capabilities = NULL;
 
 	return 0;
 }
 
 static int load_module(void)
 {
+	test_features_chan_tech.capabilities = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+	if (!test_features_chan_tech.capabilities) {
+		return AST_MODULE_LOAD_FAILURE;
+	}
+	ast_format_cap_append(test_features_chan_tech.capabilities, TEST_CHANNEL_FORMAT, 0);
 	ast_channel_register(&test_features_chan_tech);
 
 	AST_TEST_REGISTER(test_features_channel_dtmf);