[asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

[rmudgett]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13076
-----------------------------------------------------------



/branches/12/apps/app_chanspy.c
<https://reviewboard.asterisk.org/r/3900/#comment23516>

    use nolock version



/branches/12/apps/app_mixmonitor.c
<https://reviewboard.asterisk.org/r/3900/#comment23517>

    idem



/branches/12/apps/app_stack.c
<https://reviewboard.asterisk.org/r/3900/#comment23515>

    Extra parens.  This line may be short enought to not need line continuation.



/branches/12/main/channel_internal_api.c
<https://reviewboard.asterisk.org/r/3900/#comment23518>

    Why were these changed?  They were already using _nolock suffix.
    ast_channel_set_unbridged() and ast_channel_set_unbridged_nolock()


- rmudgett


On Aug. 11, 2014, 1:23 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3900/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 1:23 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24027
>     https://issues.asterisk.org/jira/browse/ASTERISK-24027
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> An odd little issue that can interrupt features and even hangup calls entirely in an unwanted fashion.
> 
> Reproduction is fairly trivial, simply have a dynamic bridge feature that plays audio or executes an AGI or something that just generally takes a little while and is sensitive to hangups.
> Call into an extension that puts that feature on the channel and call another device.
> Execute the feature.
> While the feature is running, start mixmonitor on the channel executing the feature and watch as the feature is prematurely terminated and the call itself is hung up entirely.
> 
> Having the flag to re-evaluate the status of the bridge be a hangup flag seems to have been a little off point and trying to have everything that pays attention to hangups specifically ignore it seems a little wacky, so instead I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned it into its own thing.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/pbx.c 420558 
>   /branches/12/main/framehook.c 420558 
>   /branches/12/main/channel_internal_api.c 420558 
>   /branches/12/main/channel.c 420558 
>   /branches/12/main/bridge_channel.c 420558 
>   /branches/12/main/bridge_after.c 420558 
>   /branches/12/include/asterisk/channel.h 420558 
>   /branches/12/apps/app_stack.c 420558 
>   /branches/12/apps/app_mixmonitor.c 420558 
>   /branches/12/apps/app_chanspy.c 420558 
> 
> Diff: https://reviewboard.asterisk.org/r/3900/diff/
> 
> 
> Testing
> -------
> 
> Performed the above reproduction steps with the patch and the call no longer hangs up and the feature completes normally.  Mixmonitor captures all the audio as well.
> Made sure native RTP bridges would still be re-evaluated and become simple bridges when a hook such as mixmonitor is placed on one of the bridged channels.
> Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch didn't introduce any failures.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140811/1c60c645/attachment-0001.html>