[asterisk-dev] [Code Review] 4585: res_pjsip: Refactor endpt_send_request to include transaction timeout

[Joshua Colp]

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


I've also sent an email to Teluu for their thoughts on this. Specifically whether the timeout parameter is deprecated (or whether it should work but does not), and any suggestions on how we can accomplish this.


branches/13/res/res_pjsip.c
<https://reviewboard.asterisk.org/r/4585/#comment25804>

    There's a race condition here. The ast_sched_find_data may return data but when AST_SCHED_DEL is invoked the scheduled item could wrong. Therefore ast_sched_find_data isn't really needed.
    
    As well it's possible for the scheduled item to be running.



branches/13/res/res_pjsip.c
<https://reviewboard.asterisk.org/r/4585/#comment25805>

    So!
    
    If both the transaction timer and the timer here execute at the same time it's possible for the transaction_timeout_cb to operate on a transaction that may be destroyed.
    
    The transaction layer uses group locks to overcome this, which is only available in recent PJSIP releases.
    
    I think instead of using our own scheduler the PJSIP one should be used instead. There's a guarantee that only one scheduled item will occur at a time, so you won't clash with the transaction timers.


- Joshua Colp


On April 3, 2015, 8:12 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4585/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 8:12 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This is the first follow-on to https://reviewboard.asterisk.org/r/4572/ and the discussion at http://lists.digium.com/pipermail/asterisk-dev/2015-March/073921.html 
> 
> This patch pulls the pjsip_endpt_send_request function out of pjproject and into res_pjsip in order to implement transaction timeout capability.  Now when the transaction is initiated, an asterisk sched timer is started.  If the transaction completes (or pjsip itself times it out) before the timer expires, the timer is cancelled.  If the timer expires before the transaction is completed, the transaction is cancelled.  Either way, the callback is called with the TIMER event code.
> 
> The timeout is supplied in the call to ast_sip_send_out_of_dialog_request.  If '-1', no timer is started and the transaction will continue until successful completion or pjsip itself cancels it.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip.c 433967 
>   branches/13/include/asterisk/res_pjsip.h 433967 
> 
> Diff: https://reviewboard.asterisk.org/r/4585/diff/
> 
> 
> Testing
> -------
> 
> Tested that both of the pjsip timeout and asterisk timeout scenarios work and clean up properly.
> 
> All pjsip testsuite tests that worked before the change still work after the change.  A new testsuite test will be written when the companion pjsip_options work is done.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

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