[asterisk-dev] [Code Review] 3754: media formats: fix ref leak on peer for mwi sub

[Matt Jordan]

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



/team/group/media_formats-reviewed-trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/3754/#comment22917>

    You should assert on the peer here:
    
    if (stasis_subscription_final_message(sub, msg)) {
        ast_assert(peer == NULL);
        ast_free(peer_name);
        return;
    }
    
    The peer has to be destroyed OR removed from the peers container when the final subscription message is received. Otherwise, we somehow cancelled a subscription and had the peer active at the same time.
    
    Note that during a reload operation, the subscription to the mailbox is cleared via set_peer_defaults - the peer object, however, is still very much alive. It is the same object: chan_sip does not reload atomically in the same fashion as more recent modules. The peer object is, however, removed from the peers container during this period. If this logic was ever broken, we would want an assert to fire here noting that we somehow left the peer accessible even when we cancelled the subscription.



/team/group/media_formats-reviewed-trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/3754/#comment22918>

    Duping the string can fail. If the memory allocation fails, bail!


- Matt Jordan


On July 14, 2014, 9:55 a.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3754/
> -----------------------------------------------------------
> 
> (Updated July 14, 2014, 9:55 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23959
>     https://issues.asterisk.org/jira/browse/ASTERISK-23959
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> To resolve circular reference, pass peer name to mwi_event_cb instead of link to peer with ref held.
> 
> 
> Diffs
> -----
> 
>   /team/group/media_formats-reviewed-trunk/channels/chan_sip.c 418503 
> 
> Diff: https://reviewboard.asterisk.org/r/3754/diff/
> 
> 
> Testing
> -------
> 
> Resolved ref leak according to REF_DEBUG, and tests clean under valgrind as well.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

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