From c87dbe9922e136409e229b3f715c1f46ffcb7de9 Mon Sep 17 00:00:00 2001 From: Matthew Jordan Date: Sun, 8 Jun 2014 18:11:54 +0000 Subject: [PATCH] bridges/bridge_native_rtp: Reconfigure bridge on removal of framehook This patch is a re-do of r414122. When r414122 was merged, a major problem with it was uncovered. UNBRIDGE soft hangup flags have a catastrophic effect on the pbx core if they leak out from the bridge layer: the channel gets hung up. With the number of threads involved in a blind transfer, and with the initial patch, it was likely that this would occur. This caused a large number of test failures This patch is nearly identical with the one proposed in r414122, save for the following changes: - We explicitly clear the UNBRIDGE flag when setting an after goto on a channel in a bridge - Defensively, if we encounter an UNBRIDGE flag in the pbx core, we handle it https://reviewboard.asterisk.org/r/3585/ git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@415443 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- bridges/bridge_native_rtp.c | 12 ++++++++---- include/asterisk/channel.h | 16 ++++++++++++++++ main/bridge_after.c | 5 +++++ main/bridge_channel.c | 4 ++-- main/channel.c | 14 +++++++++++++- main/framehook.c | 4 ++++ main/pbx.c | 6 ++++++ 7 files changed, 54 insertions(+), 7 deletions(-) diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c index 2be2dbf421..725a6f67fe 100644 --- a/bridges/bridge_native_rtp.c +++ b/bridges/bridge_native_rtp.c @@ -234,8 +234,10 @@ static void native_rtp_bridge_stop(struct ast_bridge *bridge, struct ast_channel break; case AST_RTP_GLUE_RESULT_REMOTE: if (!target) { - glue0->update_peer(c0->chan, NULL, NULL, NULL, NULL, 0); - if (glue1) { + if (ast_channel_is_leaving_bridge(c0->chan)) { + glue0->update_peer(c0->chan, NULL, NULL, NULL, NULL, 0); + } + if (glue1 && ast_channel_is_leaving_bridge(c1->chan)) { glue1->update_peer(c1->chan, NULL, NULL, NULL, NULL, 0); } } else { @@ -488,8 +490,10 @@ static void native_rtp_bridge_leave(struct ast_bridge *bridge, struct ast_bridge ast_rtp_instance_set_bridged(tinstance, NULL); } - /* Direct RTP may have occurred, tear it down */ - glue->update_peer(bridge_channel->chan, NULL, NULL, NULL, NULL, 0); + if (ast_channel_is_leaving_bridge(bridge_channel->chan)) { + /* Direct RTP may have occurred, tear it down */ + glue->update_peer(bridge_channel->chan, NULL, NULL, NULL, NULL, 0); + } native_rtp_bridge_stop(bridge, NULL); } diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index 2998fe84ac..92345f4a64 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -4283,6 +4283,22 @@ struct ast_bridge *ast_channel_get_bridge(const struct ast_channel *chan); */ int ast_channel_is_bridged(const struct ast_channel *chan); +/*! + * \brief Determine if a channel is leaving a bridge, but \em not hung up + * \since 12.4.0 + * + * \param chan The channel to test + * + * \note If a channel is hung up, it is implicitly leaving any bridge it + * may be in. This function is used to test if a channel is leaving a bridge + * but may survive the experience, if it has a place to go to (dialplan or + * otherwise) + * + * \retval 0 The channel is not leaving the bridge or is hung up + * \retval non-zero The channel is leaving the bridge + */ +int ast_channel_is_leaving_bridge(struct ast_channel *chan); + /*! * \brief Get the channel's bridge peer only if the bridge is two-party. * \since 12.0.0 diff --git a/main/bridge_after.c b/main/bridge_after.c index 3d1d2f3622..fbe4e605a5 100644 --- a/main/bridge_after.c +++ b/main/bridge_after.c @@ -451,6 +451,11 @@ int ast_bridge_setup_after_goto(struct ast_channel *chan) struct after_bridge_goto_ds *after_bridge; int goto_failed = -1; + /* We are going to be leaving the bridging system now; + * clear any pending UNBRIDGE flags + */ + ast_channel_clear_softhangup(chan, AST_SOFTHANGUP_UNBRIDGE); + /* Determine if we are going to setup a dialplan location and where. */ if (ast_channel_softhangup_internal_flag(chan) & AST_SOFTHANGUP_ASYNCGOTO) { /* An async goto has already setup a location. */ diff --git a/main/bridge_channel.c b/main/bridge_channel.c index 2b37f25b5b..9a1e40bba8 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -1757,8 +1757,8 @@ void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel) * outgoing channel, clear the outgoing flag. */ if (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING) - && (ast_channel_softhangup_internal_flag(bridge_channel->chan) & (AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE) - || bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT)) { + && (ast_channel_is_leaving_bridge(bridge_channel->chan) + || bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT)) { ast_debug(2, "Channel %s will survive this bridge; clearing outgoing (dialed) flag\n", ast_channel_name(bridge_channel->chan)); ast_clear_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING); } diff --git a/main/channel.c b/main/channel.c index b6efde048b..319adf9df4 100644 --- a/main/channel.c +++ b/main/channel.c @@ -2596,7 +2596,7 @@ void ast_channel_clear_softhangup(struct ast_channel *chan, int flag) /*! \brief Softly hangup a channel, don't lock */ int ast_softhangup_nolock(struct ast_channel *chan, int cause) { - ast_debug(1, "Soft-Hanging up channel '%s'\n", ast_channel_name(chan)); + ast_debug(1, "Soft-Hanging (%#04x) up channel '%s'\n", cause, ast_channel_name(chan)); /* Inform channel driver that we need to be hung up, if it cares */ ast_channel_softhangup_internal_flag_add(chan, cause); ast_queue_frame(chan, &ast_null_frame); @@ -10151,6 +10151,18 @@ int ast_channel_is_bridged(const struct ast_channel *chan) return ast_channel_internal_bridge(chan) != NULL; } +int ast_channel_is_leaving_bridge(struct ast_channel *chan) +{ + int hangup_flags = ast_channel_softhangup_internal_flag(chan); + int hangup_test = hangup_flags & (AST_SOFTHANGUP_ASYNCGOTO | AST_SOFTHANGUP_UNBRIDGE); + + /* This function should only return true if either ASYNCGOTO + * or UNBRIDGE is set, or both flags are set. It should return + * false if any other flag is set. + */ + return (hangup_test && (hangup_test == hangup_flags)); +} + struct ast_channel *ast_channel_bridge_peer(struct ast_channel *chan) { struct ast_channel *peer; diff --git a/main/framehook.c b/main/framehook.c index de8c612aaf..84719ef491 100644 --- a/main/framehook.c +++ b/main/framehook.c @@ -189,6 +189,10 @@ int ast_framehook_detach(struct ast_channel *chan, int id) } AST_LIST_TRAVERSE_SAFE_END; + if (ast_channel_is_bridged(chan)) { + ast_softhangup_nolock(chan, AST_SOFTHANGUP_UNBRIDGE); + } + return res; } diff --git a/main/pbx.c b/main/pbx.c index d29f57568b..2c5037df09 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -6310,6 +6310,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, while (!(res = ast_spawn_extension(c, ast_channel_context(c), ast_channel_exten(c), ast_channel_priority(c), S_COR(ast_channel_caller(c)->id.number.valid, ast_channel_caller(c)->id.number.str, NULL), &found, 1))) { + + /* Defensively clear the UNBRIDGE flag in case it leaked + * out of the bridging framework. UNBRIDE never implies + * that a channel is hung up. + */ + ast_channel_clear_softhangup(c, AST_SOFTHANGUP_UNBRIDGE); if (!ast_check_hangup(c)) { ast_channel_priority_set(c, ast_channel_priority(c) + 1); continue; -- 2.47.2