diff options
| -rw-r--r-- | apps/confbridge/conf_chan_announce.c | 1 | ||||
| -rw-r--r-- | bridges/bridge_softmix.c | 13 | ||||
| -rw-r--r-- | include/asterisk/bridge_technology.h | 3 | ||||
| -rw-r--r-- | include/asterisk/features.h | 9 | ||||
| -rw-r--r-- | main/bridge.c | 11 | ||||
| -rw-r--r-- | main/core_unreal.c | 1 | ||||
| -rw-r--r-- | main/features.c | 1 | ||||
| -rw-r--r-- | tests/test_channel_feature_hooks.c | 15 |
8 files changed, 44 insertions, 10 deletions
diff --git a/apps/confbridge/conf_chan_announce.c b/apps/confbridge/conf_chan_announce.c index 6596a8537..ee4660687 100644 --- a/apps/confbridge/conf_chan_announce.c +++ b/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; } diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c index e3df18fe5..fe058e4e6 100644 --- 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 7de573a23..7f5d746f8 100644 --- 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/include/asterisk/features.h b/include/asterisk/features.h index b63124c2f..a4aed5d18 100644 --- a/include/asterisk/features.h +++ b/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); diff --git a/main/bridge.c b/main/bridge.c index ee1d8cec6..ee5ad735b 100644 --- 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; } @@ -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) { diff --git a/main/core_unreal.c b/main/core_unreal.c index e9b7a8d66..f2404dfca 100644 --- a/main/core_unreal.c +++ b/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; } diff --git a/main/features.c b/main/features.c index 1810b1556..b96cbd68c 100644 --- a/main/features.c +++ b/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; diff --git a/tests/test_channel_feature_hooks.c b/tests/test_channel_feature_hooks.c index fbc9786cc..c5d3b9b86 100644 --- 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); |