[asterisk-dev] [Code Review] 3752: astobj2: Fix race condition in ref_debug log

[Corey Farrell]

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


I think the way this has to be fixes is to simply get rid of the 6th column (the current ref count).  refcounter.py would need to be modified to keep total based on column 2 (delta).


/team/group/media_formats-reviewed-trunk/main/astobj2.c
<https://reviewboard.asterisk.org/r/3752/#comment22869>

    This reorder will prevent ref_log from recording a double-free.
    
    Example:
    obj = ao2_alloc(..);
    ao2_ref(obj, -1);
    ao2_ref(obj, -1);
    
    Currently this block of code would produce an error in the ref_log.
    
    Also I know it sounds weird but I've seen:
    
    Thread 1: ao2_ref(obj, -1) == 2
    Thread 2: ao2_ref(obj, -1) == 1
    
    And for thread 2 (which runs the destructor) actually finishing first (and the last 2 entries to ref_log are reversed.


- Corey Farrell


On July 11, 2014, 10:45 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3752/
> -----------------------------------------------------------
> 
> (Updated July 11, 2014, 10:45 p.m.)
> 
> 
> Review request for Asterisk Developers and Corey Farrell.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> As Corey noticed, when an object's reference is changed rapidly the ref debug log will report the wrong reference. This is due to accessing the reference (which is not thread safe) prior to the reference count being changed. Since the change in reference count is returned by internal_ao2_ref, it can be used instead to determine what the current state of the object is.
> 
> 
> Diffs
> -----
> 
>   /team/group/media_formats-reviewed-trunk/main/astobj2.c 418435 
> 
> Diff: https://reviewboard.asterisk.org/r/3752/diff/
> 
> 
> Testing
> -------
> 
> Objects that were incorrectly reported as leaked no longer are.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

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