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



/branches/11/main/sched.c
<https://reviewboard.asterisk.org/r/4425/#comment24999>

    You definitely will need to lock the scheduler context here:
    
    ast_mutex_lock(&con->lock);
    
    while(...) {
    ...
    }
    
    ast_mutex_unlock(&con->lock);
    
    Otherwise, a scheduled item could fire while you're removing it.



/branches/11/main/sched.c
<https://reviewboard.asterisk.org/r/4425/#comment24998>

    I'd write this so that the assignment is in the while loop:
    
    int i = 1;
    struct sched *current;
    
    while ((current = ast_heap_peek(con->sched_hep, i)) {
    
    }
    
    Alternatively, since you also have to increment a loop counter, you may 
want to consider a for loop:
    
    for (current = ast_heap_peak(con->sched_heap, i); current; ++i, current = 
ast_heap_peak(con->sched_heap, i)) {
        ...
    }
    
    While the latter option does require calling ast_heap_peak both during the 
initial assignment and during the loop increment, it does let you pull the 
incrementing of i out of the loop body, which would let you reduce indentation:
    
    for (...) {
        if (current->callback != match) {
            continue;
         }
    }


- Matt Jordan


On Feb. 15, 2015, 2:06 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4425/
> -----------------------------------------------------------
> 
> (Updated Feb. 15, 2015, 2:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24451
>     https://issues.asterisk.org/jira/browse/ASTERISK-24451
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When an event is scheduled, it often includes data with a reference bump for 
> the scheduler.  If the scheduler context needs to be destroyed before all 
> events have run, references are leaked.
> 
> This change adds a procedure to run all events that were scheduled for a 
> specific callback, but haven't run yet.  chan_iax2 is modified to use this 
> new procedure to run all pending peercnt_remove_cb and replace_callno events.
> 
> In the long run I think the scheduler will need to be ao2 aware, but that's 
> not feasible for existing releases.
> 
> 
> Diffs
> -----
> 
>   /branches/11/main/sched.c 431733 
>   /branches/11/include/asterisk/sched.h 431733 
>   /branches/11/channels/chan_iax2.c 431733 
> 
> Diff: https://reviewboard.asterisk.org/r/4425/diff/
> 
> 
> Testing
> -------
> 
> Ran a bunch of tests that were leaking references: 
> tests/apps/directed_pickup/pickup_chan, tests/callparking, 
> tests/channels/iax2/acl_call, tests/channels/iax2/basic-call, 
> tests/feature_attended_transfer, tests/feature_blonde_transfer
> 
> Only tests/callparking still has 1 leaked reference (it was more).
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

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