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.

* Fix the test_channel_feature_hooks.c unit tests.  The test channel must
have a valid codec to join the simple_bridge technology.  This patch makes
joining a bridge more strict by not allowing partially joined channels to
remain in the bridge.

Change-Id: I97e2ade6a2bcd1214f24fb839fda948825b61a2b

diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index e3df18fe510a9c96fdf41d58411a11ff546677b4..fe058e4e6413af9821ec35aab345825b89305415 100644 (file)
--- a/bridges/bridge_softmix.c
+++ b/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;
                        }
 

diff --git a/include/asterisk/bridge_technology.h b/include/asterisk/bridge_technology.h
index 7de573a23e0a3e488dd129b6c8c3af4ef6bf5b92..7f5d746f893fdc7e38a2ff62d7de77f200f5ecf6 100644 (file)
--- a/include/asterisk/bridge_technology.h
+++ b/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);
        /*!

diff --git a/main/bridge.c b/main/bridge.c
index fd83cfb7b977f4533b5a7a7e7cf53465b06104b7..dad7cfa7ce4e6e5aca63ec329dfdbdb481a1f951 100644 (file)
--- a/main/bridge.c
+++ b/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;
        }
 

diff --git a/tests/test_channel_feature_hooks.c b/tests/test_channel_feature_hooks.c
index fbc9786cc59495e9fdd8fe9c55c7e2ce660d8117..c5d3b9b866cb98f8dd29eb2699d1a4c58be6dd06 100644 (file)
--- a/tests/test_channel_feature_hooks.c
+++ b/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);