Author: kmoore
Date: Tue Aug  6 07:45:39 2013
New Revision: 396320

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=396320
Log:
Fix memory leaks in the CDR engine

Fix refcount bugs and a possible locking problem in the CDR engine
relating to use of ao2_iterators.

Review: https://reviewboard.asterisk.org/r/2724/
(closes issue ASTERISK-22126)

Modified:
    trunk/main/cdr.c

Modified: trunk/main/cdr.c
URL: 
http://svnview.digium.com/svn/asterisk/trunk/main/cdr.c?view=diff&rev=396320&r1=396319&r2=396320
==============================================================================
--- trunk/main/cdr.c (original)
+++ trunk/main/cdr.c Tue Aug  6 07:45:39 2013
@@ -1481,8 +1481,9 @@
 
        while ((cand_cdr_master = ao2_iterator_next(it_cdrs))) {
                struct cdr_object *cand_cdr;
-
-               ao2_lock(cand_cdr_master);
+               RAII_VAR(struct cdr_object *, cdr_cleanup, cand_cdr_master, 
ao2_cleanup);
+               SCOPED_AO2LOCK(lock, cand_cdr_master);
+
                for (cand_cdr = cand_cdr_master; cand_cdr; cand_cdr = 
cand_cdr->next) {
                        /* Skip any records that are not in a bridge or in this 
bridge.
                         * I'm not sure how that would happen, but it pays to 
be careful. */
@@ -1498,8 +1499,6 @@
                        success = 1;
                        break;
                }
-               ao2_unlock(cand_cdr_master);
-               ao2_t_ref(cand_cdr_master, -1, "Drop iterator reference");
        }
        ao2_iterator_destroy(it_cdrs);
 
@@ -1624,8 +1623,9 @@
 
        while ((cand_cdr_master = ao2_iterator_next(it_cdrs))) {
                struct cdr_object *cand_cdr;
-
-               ao2_lock(cand_cdr_master);
+               RAII_VAR(struct cdr_object *, cdr_cleanup, cand_cdr_master, 
ao2_cleanup);
+               SCOPED_AO2LOCK(lock, cand_cdr_master);
+
                for (cand_cdr = cand_cdr_master; cand_cdr; cand_cdr = 
cand_cdr->next) {
                        /* Skip any records that are not in a bridge or in this 
bridge.
                         * I'm not sure how that would happen, but it pays to 
be careful. */
@@ -1654,8 +1654,6 @@
                        success = 1;
                        break;
                }
-               ao2_unlock(cand_cdr_master);
-               ao2_t_ref(cand_cdr_master, -1, "Drop iterator reference");
        }
        ao2_iterator_destroy(it_cdrs);
 
@@ -2276,7 +2274,7 @@
                ao2_cleanup(candidates);
                return NULL;
        }
-       while ((cand_cdr_master = ao2_iterator_next(it_cdrs))) {
+       for (; (cand_cdr_master = ao2_iterator_next(it_cdrs)); 
ao2_cleanup(cand_cdr_master)) {
                SCOPED_AO2LOCK(lock, cand_cdr_master);
                add_candidate_for_bridge(bridge->uniqueid, candidates, 
cand_cdr_master, 1);
        }
@@ -2290,7 +2288,7 @@
                ao2_cleanup(candidates);
                return NULL;
        }
-       while ((cand_cdr_master = ao2_iterator_next(it_cdrs))) {
+       for (; (cand_cdr_master = ao2_iterator_next(it_cdrs)); 
ao2_cleanup(cand_cdr_master)) {
                SCOPED_AO2LOCK(lock, cand_cdr_master);
                add_candidate_for_bridge(bridge->uniqueid, candidates, 
cand_cdr_master, 0);
        }
@@ -2860,7 +2858,7 @@
                return -1;
        }
 
-       while ((cdr = ao2_iterator_next(it_cdrs))) {
+       for (; (cdr = ao2_iterator_next(it_cdrs)); ao2_unlock(cdr), 
ao2_cleanup(cdr)) {
                ao2_lock(cdr);
                for (it_cdr = cdr; it_cdr; it_cdr = it_cdr->next) {
                        struct varshead *headp = NULL;
@@ -2876,8 +2874,6 @@
                                set_variable(headp, name, value);
                        }
                }
-               ao2_unlock(cdr);
-               ao2_ref(cdr, -1);
        }
        ao2_iterator_destroy(it_cdrs);
 
@@ -3597,7 +3593,7 @@
        ast_cli(a->fd, TITLE_STRING, "Channel", "Dst. Channel", "LastApp", 
"Start", "Answer", "End", "Billsec", "Duration");
 
        it_cdrs = ao2_iterator_init(active_cdrs_by_channel, 0);
-       while ((cdr = ao2_iterator_next(&it_cdrs))) {
+       for (; (cdr = ao2_iterator_next(&it_cdrs)); ao2_cleanup(cdr)) {
                struct cdr_object *it_cdr;
                struct timeval start_time = { 0, };
                struct timeval answer_time = { 0, };
@@ -3622,7 +3618,6 @@
 
                /* If there was no start time, then all CDRs were for a dialed 
channel; skip */
                if (ast_tvzero(start_time)) {
-                       ao2_ref(cdr, -1);
                        continue;
                }
                it_cdr = cdr->last;
@@ -3639,7 +3634,6 @@
                                end_time_buffer,
                                ast_tvzero(answer_time) ? 0 : 
(long)ast_tvdiff_ms(end_time, answer_time) / 1000,
                                (long)ast_tvdiff_ms(end_time, start_time) / 
1000);
-               ao2_ref(cdr, -1);
        }
        ao2_iterator_destroy(&it_cdrs);
 #undef FORMAT_STRING


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

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

Reply via email to