diff --git a/main/bridging.c b/main/bridging.c index b0c27966a2a6ae84077bb2fcbfac83cdc1ddd358..a4a1339a34ee30c9f4549c66f0b51caa411ff7e6 100644 --- a/main/bridging.c +++ b/main/bridging.c @@ -528,65 +528,80 @@ static void bridge_dissolve(struct ast_bridge *bridge) /*! * \internal - * \brief Determine whether a bridge channel leaving the bridge will cause it to dissolve or not. + * \brief Check if a bridge should dissolve and do it. * \since 12.0.0 * - * \param bridge_channel Channel causing the check - * \param bridge The bridge we are concerned with + * \param bridge_channel Channel causing the check. * - * \note the bridge should be locked prior to calling this function + * \note On entry, bridge_channel->bridge is already locked. * - * \retval 0 if the channel leaving shouldn't cause the bridge to dissolve - * \retval non-zero if the channel should cause the bridge to dissolve + * \return Nothing */ -static int bridge_check_will_dissolve(struct ast_bridge_channel *bridge_channel, struct ast_bridge *bridge, int assume_end_state) +static void bridge_dissolve_check(struct ast_bridge_channel *bridge_channel) { + struct ast_bridge *bridge = bridge_channel->bridge; + if (bridge->dissolved) { - /* The bridge is already dissolved. Don't try to dissolve it again. */ - return 0; + return; } if (!bridge->num_channels && ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_DISSOLVE_EMPTY)) { /* Last channel leaving the bridge turns off the lights. */ - return 1; + bridge_dissolve(bridge); + return; } - switch (assume_end_state ? AST_BRIDGE_CHANNEL_STATE_END : bridge_channel->state) { + switch (bridge_channel->state) { case AST_BRIDGE_CHANNEL_STATE_END: /* Do we need to dissolve the bridge because this channel hung up? */ if (ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_DISSOLVE_HANGUP) || (bridge_channel->features->usable && ast_test_flag(&bridge_channel->features->feature_flags, AST_BRIDGE_CHANNEL_FLAG_DISSOLVE_HANGUP))) { - return 1; + bridge_dissolve(bridge); + return; } - break; default: break; } - /* BUGBUG need to implement AST_BRIDGE_CHANNEL_FLAG_LONELY support here */ - return 0; +/* BUGBUG need to implement AST_BRIDGE_CHANNEL_FLAG_LONELY support here */ } /*! * \internal - * \brief Check if a bridge should dissolve and do it. + * \brief Check if a bridge should dissolve because of a stolen channel and do it. * \since 12.0.0 * - * \param bridge_channel Channel causing the check. + * \param bridge Bridge to check. + * \param bridge_channel Stolen channel causing the check. It is not in the bridge to check and may be in another bridge. * - * \note On entry, bridge_channel->bridge is already locked. + * \note On entry, bridge and bridge_channel->bridge are already locked. * * \return Nothing */ -static void bridge_dissolve_check(struct ast_bridge_channel *bridge_channel) +static void bridge_dissolve_check_stolen(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel) { - struct ast_bridge *bridge = bridge_channel->bridge; + if (bridge->dissolved) { + return; + } - if (bridge_check_will_dissolve(bridge_channel, bridge, 0)) { + if (bridge_channel->features->usable + && ast_test_flag(&bridge_channel->features->feature_flags, + AST_BRIDGE_CHANNEL_FLAG_DISSOLVE_HANGUP)) { + /* The stolen channel controlled the bridge it was stolen from. */ + bridge_dissolve(bridge); + return; + } + if (bridge->num_channels < 2 + && ast_test_flag(&bridge->feature_flags, AST_BRIDGE_FLAG_DISSOLVE_HANGUP)) { + /* + * The stolen channel has not left enough channels to keep the + * bridge alive. Assume the stolen channel hung up. + */ bridge_dissolve(bridge); + return; } } @@ -4322,10 +4337,10 @@ int ast_bridge_move(struct ast_bridge *dst_bridge, struct ast_bridge *src_bridge } 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) { RAII_VAR(struct ast_bridge *, chan_bridge, NULL, ao2_cleanup); - struct ast_channel *bridge_chan = NULL; + RAII_VAR(struct ast_channel *, yanked_chan, NULL, ao2_cleanup); ast_channel_lock(chan); chan_bridge = ast_channel_get_bridge(chan); @@ -4333,45 +4348,53 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan, if (chan_bridge) { RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup); - int hangup = 0; - - /* Simply moving the channel from the bridge won't perform the dissolve check - * so we need to manually check here to see if we should dissolve after moving. */ - ao2_lock(chan_bridge); - if ((bridge_channel = ast_channel_get_bridge_channel(chan))) { - hangup = bridge_check_will_dissolve(bridge_channel, chan_bridge, 1); - } - if (ast_bridge_move(bridge, chan_bridge, chan, NULL, 1)) { - ao2_unlock(chan_bridge); + ast_bridge_lock_both(bridge, chan_bridge); + bridge_channel = find_bridge_channel(chan_bridge, chan); + if (bridge_move_locked(bridge, chan_bridge, chan, NULL, 1)) { + ast_bridge_unlock(chan_bridge); + ast_bridge_unlock(bridge); return -1; } - if (hangup) { - bridge_dissolve(chan_bridge); - } - ao2_unlock(chan_bridge); + /* + * bridge_move_locked() will implicitly ensure that + * bridge_channel is not NULL. + */ + ast_assert(bridge_channel != NULL); + /* + * Additional checks if the channel we just stole dissolves the + * original bridge. + */ + 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. */ - bridge_chan = ast_channel_yank(chan); - if (!bridge_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)); return -1; } - if (ast_channel_state(bridge_chan) != AST_STATE_UP) { - ast_answer(bridge_chan); + if (ast_channel_state(yanked_chan) != AST_STATE_UP) { + ast_answer(yanked_chan); } - if (ast_bridge_impart(bridge, bridge_chan, NULL, features, 1)) { + ast_channel_ref(yanked_chan); + if (ast_bridge_impart(bridge, yanked_chan, NULL, features, 1)) { ast_log(LOG_WARNING, "Could not add %s to the bridge\n", ast_channel_name(chan)); + ast_hangup(yanked_chan); return -1; } } if (play_tone && !ast_strlen_zero(xfersound)) { - struct ast_channel *play_chan = bridge_chan ?: chan; + struct ast_channel *play_chan = yanked_chan ?: chan; RAII_VAR(struct ast_bridge_channel *, play_bridge_channel, NULL, ao2_cleanup); ast_channel_lock(play_chan); @@ -4379,8 +4402,8 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan, ast_channel_unlock(play_chan); if (!play_bridge_channel) { - ast_log(LOG_WARNING, "Unable to play tone for channel %s. Unable to get bridge channel\n", - ast_channel_name(play_chan)); + ast_log(LOG_WARNING, "Unable to play tone for channel %s. No longer in a bridge.\n", + ast_channel_name(play_chan)); } else { ast_bridge_channel_queue_playfile(play_bridge_channel, NULL, xfersound, NULL); }