[asterisk-dev] [Code Review] 4178: res_pjsip_outbound_publish: stack overflow when using non-default sorcery wizard

[Joshua Colp]

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



branches/13/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/4178/#comment24369>

    Per a comment further down if the container can be used like it should be this can become lock free.



branches/13/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/4178/#comment24370>

    Is it actually possible for this to happen?



branches/13/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/4178/#comment24368>

    This is counter to how an ao2 global container is supposed to work/be used. As you are using it it has no real advantage.
    
    What you are supposed to do is construct a container and once done atomically swap. Both containers are lock free and remain unmodified. The objects inside *may* have locks and be modified but the container itself doesn't.
    
    Is it possible to do that with this implementation? If not then I think it can just become a regular container.



branches/13/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/4178/#comment24367>

    This is duplicating code from above. Would it not be better to assume that states may be NULL when setting it up above?
    
    Of course I made the assumption of the global container being used like elsewhere.
    
    If you can't do that then this can become a non-swappable regular container I'd say.



branches/13/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/4178/#comment24366>

    You need to be in a loop here. It's possible for this to get spuriously triggered.
    
    As for the time limit - can things be in a state where that is safe to do?
    
    Some logging would also be useful so people know what is going on if it waits a bit.


- Joshua Colp


On Nov. 20, 2014, 9:43 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4178/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2014, 9:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24514
>     https://issues.asterisk.org/jira/browse/ASTERISK-24514
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When using a non-default sorcery wizard (in this instance realtime) for outbound publishes Asterisk will crash after a stack overflow occurs due to the code infinitely recursing.  The fix entails removing the outbound publish state dependency from the outbound publish sorcery object and instead keeping an in memory container that can be used to lookup the state when needed.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip_outbound_publish.c 428498 
> 
> Diff: https://reviewboard.asterisk.org/r/4178/diff/
> 
> 
> Testing
> -------
> 
> On top of running the current testsuite tests I also manually tested various configurations and scenarios using a static configuration file as well as dynamic realtime.  Verified that the crash no longer occurs and the potentially affected functionality works as expected (for instance, module [re]loading).
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

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