[asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

rmudgett reviewboard at asterisk.org
Wed Aug 20 19:03:41 CDT 2014


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



/branches/12/res/stasis/command.h
<https://reviewboard.asterisk.org/r/3908/#comment23589>

    Remove this special function and use ast_free_ptr instead.  This is what ast_free_ptr is for.



/branches/12/res/stasis/command.h
<https://reviewboard.asterisk.org/r/3908/#comment23588>

    Remove this special function and use __ao2_cleanup instead.  This is what __ao2_cleanup is for.



/branches/12/res/stasis/command.c
<https://reviewboard.asterisk.org/r/3908/#comment23592>

    You should call the data_destructor here if it is present and then clear the data_destructor pointer.  Otherwise, you are extending the life of the data object possibly too long from what it was before.



/branches/12/res/stasis/command.c
<https://reviewboard.asterisk.org/r/3908/#comment23591>

    Call the data_destructor here if there is one to guarantee that the data will be destroyed regardless of successfully queueing the command.  Otherwise, you could return -1 and the caller would not know if the data was destroyed.
    
    It might be better to have command_create() do this instead if the command object could not be created.  This way the other places that create commands would be covered as well.


- rmudgett


On Aug. 20, 2014, 4:21 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3908/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2014, 4:21 p.m.)
> 
> 
> Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
> 
> 
> Bugs: ASTERISK-24147
>     https://issues.asterisk.org/jira/browse/ASTERISK-24147
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Here's a basic rundown of what was happening:
> 
> 1. Stasis application with a channel in it, channel is in a bridge.  Playback actions for the channel are queued, the stasis control is running them.
> 
> 2. The channel hangs up. Because the stasis control detects a hangup and the depart is expected to be handled when the channel leaves the bridge, the stasis application execution function exits taking the control along with it. When the PBX thread exits, the channel is considered fully hung up and leaves the bridging core. At this point we are running after bridge callbacks with, one of which holds a pointer to a dead control object that we are trying to do all queue actions to along with other things.  This causes the crash.
> 
> In order to fix this, I made sure that if the control execution loop is exited without pulling the channel out of the bridge that the channel depart would occur manually and the control would be marked so that the after bridge cb wouldn't attempt to exit the bridge in this case. This wasn't actually causing problems, but Richard and I both have concerns about an after bridge callback attempting to depart a channel from a bridge... it seems a little out there.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/stasis/stasis_bridge.c 421424 
>   /branches/12/res/stasis/control.c 421424 
>   /branches/12/res/stasis/command.c 421424 
>   /branches/12/res/stasis/command.h 421424 
>   /branches/12/res/res_stasis_recording.c 421424 
>   /branches/12/res/res_stasis_playback.c 421424 
>   /branches/12/res/res_stasis_answer.c 421424 
>   /branches/12/res/res_stasis.c 421424 
>   /branches/12/include/asterisk/stasis_app_impl.h 421424 
> 
> Diff: https://reviewboard.asterisk.org/r/3908/diff/
> 
> 
> Testing
> -------
> 
> Made sure the crash that was happening no longer occurs
> 
> Tested long queues of playback to check what would happen with the additional playbacks.  Each subsequent playback from the one currently running fails (in an expected manner) and reports its failure over stasis.  Nothing too odd there.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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


More information about the asterisk-dev mailing list