[asterisk-dev] [Code Review] 4342: CHANNEL(peer), chan_iax2, res_fax, SNMP agent: Fix deadlock from reaching across a bridge.

[Matt Jordan]

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



/branches/13/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/4342/#comment24654>

    This change isn't necessary.
    
    AST_CHAN_TP_CREATESJITTER is only used by ast_jb_do_usecheck, which is dead code. Nothing calls it any longer.
    
    Previously, ast_jb_do_usecheck was used by ast_generic_bridge to see if it needed to empty/reset the jitter buffers on the channels during bridging.
    
    Now, instead of manipulating both channels at the same time, the bridging framework checks to see if a channel wants a jitter buffer using ast_jb_enable_for_channel when the channel goes into the bridge. This is done by looking at ast_channel_jb and adding a framehook, as opposed to the technology itself.
    
    The notion of using channel technology to determine if channels should have jitter buffers only made sense when the de-jittering of a jittered channel occurred on the other side of the bridge. This was necessary when a channel technology such as DAHDI was bridged with a channel technology that provided jitter. This is no longer the case. Now, each channel can have a jitter buffer placed on the read side of the channel. A DAHDI channel can be safely bridged with a jittery channel, so long as that channel has a jitter buffer on it - the DAHDI channel doesn't care one way or the other, as reading from the jittered channel still provides a constant stream of frames.



/branches/13/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/4342/#comment24655>

    If we want to force a jitter buffer, we should simply set up a read-side jitter buffer on the channel and be done. The notion of reaching across the bridge to determine if they are "okay" with us setting up a jitter buffer is no longer needed.


- Matt Jordan


On Jan. 14, 2015, 4:25 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4342/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 4:25 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24600
>     https://issues.asterisk.org/jira/browse/ASTERISK-24600
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Calling ast_channel_bridge_peer() cannot be done while holding any channel
> locks.  The reported issue hit the deadlock in chan_iax2, but an audit of
> the ast_channel_bridge_peer() calls found three more locations where the
> same deadlock can occur.
> 
> * Made CHANNEL(peer), chan_iax2, res_fax, and the SNMP agent not call
> ast_channel_bridge_peer() with any channel locked.  For CHANNEL(peer) and
> chan_iax2 I had to rework the logic to not hold the channel lock.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/snmp/agent.c 430625 
>   /branches/13/res/res_fax.c 430625 
>   /branches/13/funcs/func_channel.c 430625 
>   /branches/13/channels/chan_iax2.c 430625 
> 
> Diff: https://reviewboard.asterisk.org/r/4342/diff/
> 
> 
> Testing
> -------
> 
> Testsuite tests still pass for FAX.  The full testsuite didn't hit any deadlock.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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