-----------------------------------------------------------
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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to