[asterisk-dev] [Code Review] 4257: chan_pjsip: Race condition between channel answer and bridge setup when using direct media

[Mark Michelson]

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



branches/13/channels/chan_pjsip.c
<https://reviewboard.asterisk.org/r/4257/#comment24474>

    I have a couple of recommendations here:
    
    1) You bump the refcount of channel->session up here befor unlocking ast as a means of ensuring the session will not get torn down if ast is somehow hung up/masqueraded/whatever after unlocking. The problem is, when you push the task, you're referring to the session via the channel, which may just as well be affected by hangup/masquerade/etc. operation that affects ast. So my suggestion here is to save the reference to channel->session to a local variable and pass that to the synchronous task.
    
    2) Since the task we're pushing is synchronous now, it's a bit weird that we bump the refcount of the session here and decrement the refcount in the pushed task. Since the pushed task is now synchronous, we may as well do all refcount manipulations here so the code is more clear.


- Mark Michelson


On Dec. 11, 2014, 7:31 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4257/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2014, 7:31 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24563
>     https://issues.asterisk.org/jira/browse/ASTERISK-24563
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When direct media is enabled and a pjsip channel is answered a race would occur between the handling of the answer and bridge setup. Sometimes the media negotiation would take place after the native bridge was setup.  This resulted in a NULL media address, which in turn resulted in Asterisk using its address as the remote media address when sending a reinvite.  This patch makes the chan_pjsip answer handler synchronous thus alleviating the race condition (the bridge won't start setting things up until after it returns).
> 
> 
> Diffs
> -----
> 
>   branches/13/channels/chan_pjsip.c 429386 
> 
> Diff: https://reviewboard.asterisk.org/r/4257/diff/
> 
> 
> Testing
> -------
> 
> Made around 100 phones calls with and did not issue occur again.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

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