[asterisk-dev] [Code Review] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish

[rmudgett]

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

Ship it!


Ship It!

- rmudgett


On March 14, 2014, 11:55 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3338/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 11:55 a.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> ARI tests were occasionally showing a channel being stuck forever in Stasis. After looking at a live system where the problem was occurring, it became clear that the channel was stuck trying to wait for a playback to finish. However, it was also clear that the playback had been abandoned long ago.
> 
> When playing back a file to a channel that is in a bridge, the idea was to queue the playfile action onto the corresponding bridge channel, and then wait until the ARI custom playback function completed to signal that the playback had finished. There were several ways that this could fail:
> * If the bridge channel were not found in the bridge, then we would never attempt to queue the playfile action, but we would still try to wait for it to finish happening.
> * If queuing the playfile action did not succeed, then we would still attempt to wait for the action to occur.
> * If the playback action was successfully queued, but the bridge channel were removed from the bridge before the playfile action could start, then the playback would never happen, and we'd wait forever.
> 
> The responsibility of knowing whether a playfile action has conpleted or ever will occur is the bridge's, since ARI can never fully know. With this in mind, I have created a new bridge channel API call to queue a playfile action synchronously. The way it is done, synchronous queuing operations could be created for other bridge actions quite easily. A new frametype, AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a synchronization ID and the actual payload of the bridge action that is to be queued. When the frame is queued, a synchronization structure is created and added to a linked list. The procedure then blocks until it is told that the frame has been disposed of.
> 
> When the frame gets freed (which will occur whether the playfile action succeeds or does not happen), the freer of the frame signals the waiting procedure that the playfile action has terminated, and control returns to whoever queued the playfile action in the first place.
> 
> From ARI's perspective, this greatly simplifies its code. Most of the res_stasis_playback changes are code removal. The bridge_channel changes, though, are pretty large, which is why I've included Richard on this issue.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_stasis_playback.c 410558 
>   /branches/12/main/frame.c 410558 
>   /branches/12/main/channel.c 410558 
>   /branches/12/main/bridge_channel.c 410558 
>   /branches/12/include/asterisk/frame.h 410558 
>   /branches/12/include/asterisk/bridge_channel.h 410558 
>   /branches/12/funcs/func_frame_trace.c 410558 
>   /branches/12/bridges/bridge_softmix.c 410558 
> 
> Diff: https://reviewboard.asterisk.org/r/3338/diff/
> 
> 
> Testing
> -------
> 
> Initially, I did some manual playbacks using Swagger-UI to a channel in a bridge to ensure that the specified file was actually being played as promised. I also queued up several files in quick succession to ensure that they played in series and not on top of each other.
> 
> In addition, I have created an automated test that is up for review at /r/3339
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

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