<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6785">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">cdr.c: Eliminated many calls to ao2_global_obj_ref().<br><br>The CDR performance gets worse the further it gets behind in processing<br>stasis messages. One of the reasons is we were getting the global config<br>to determine if we needed to log a debugging message.<br><br>* Many calls to ao2_global_obj_ref() were just so we could determine if<br>debug mode is enabled. Made a global flag to check instead.<br><br>* Eliminated many RAII_VAR() usages associated with the remaining<br>ao2_global_obj_ref() calls.<br><br>* Added missing NULL checks for the returned ao2_global_obj_ref() value.<br><br>ASTERISK-27335<br><br>Change-Id: Iceaad93172862f610cad0188956634187bfcc7cd<br>---<br>M main/cdr.c<br>1 file changed, 185 insertions(+), 124 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/cdr.c b/main/cdr.c<br>index dc5c947..def3871 100644<br>--- a/main/cdr.c<br>+++ b/main/cdr.c<br>@@ -207,9 +207,16 @@<br> #define DEFAULT_BATCH_SCHEDULER_ONLY "0"<br> #define DEFAULT_BATCH_SAFE_SHUTDOWN "1"<br> <br>-#define CDR_DEBUG(mod_cfg, fmt, ...) \<br>+#define cdr_set_debug_mode(mod_cfg) \<br> do { \<br>- if (ast_test_flag(&(mod_cfg)->general->settings, CDR_DEBUG)) { \<br>+ cdr_debug_enabled = ast_test_flag(&(mod_cfg)->general->settings, CDR_DEBUG); \<br>+ } while (0)<br>+<br>+static int cdr_debug_enabled;<br>+<br>+#define CDR_DEBUG(fmt, ...) \<br>+ do { \<br>+ if (cdr_debug_enabled) { \<br> ast_verbose((fmt), ##__VA_ARGS__); \<br> } \<br> } while (0)<br>@@ -237,6 +244,7 @@<br> <br> static void *module_config_alloc(void);<br> static void module_config_destructor(void *obj);<br>+static void module_config_post_apply(void);<br> <br> /*! \brief The file definition */<br> static struct aco_file module_file_conf = {<br>@@ -247,9 +255,22 @@<br> <br> CONFIG_INFO_CORE("cdr", cfg_info, module_configs, module_config_alloc,<br> .files = ACO_FILES(&module_file_conf),<br>+ .post_apply_config = module_config_post_apply,<br> );<br> <br> static struct aco_type *general_options[] = ACO_TYPES(&general_option);<br>+<br>+static void module_config_post_apply(void)<br>+{<br>+ struct module_config *mod_cfg;<br>+<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return;<br>+ }<br>+ cdr_set_debug_mode(mod_cfg);<br>+ ao2_cleanup(mod_cfg);<br>+}<br> <br> /*! \brief Dispose of a module config object */<br> static void module_config_destructor(void *obj)<br>@@ -764,9 +785,7 @@<br> */<br> static void cdr_object_transition_state(struct cdr_object *cdr, struct cdr_object_fn_table *fn_table)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>-<br>- CDR_DEBUG(mod_cfg, "%p - Transitioning CDR for %s from state %s to %s\n",<br>+ CDR_DEBUG("%p - Transitioning CDR for %s from state %s to %s\n",<br> cdr, cdr->party_a.snapshot->name,<br> cdr->fn_table ? cdr->fn_table->name : "NONE", fn_table->name);<br> cdr->fn_table = fn_table;<br>@@ -874,7 +893,6 @@<br> */<br> static struct cdr_object *cdr_object_alloc(struct ast_channel_snapshot *chan)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object *cdr;<br> <br> ast_assert(chan != NULL);<br>@@ -897,7 +915,7 @@<br> cdr->party_a.snapshot = chan;<br> ao2_t_ref(cdr->party_a.snapshot, +1, "bump snapshot during CDR creation");<br> <br>- CDR_DEBUG(mod_cfg, "%p - Created CDR for channel %s\n", cdr, chan->name);<br>+ CDR_DEBUG("%p - Created CDR for channel %s\n", cdr, chan->name);<br> <br> cdr_object_transition_state(cdr, &single_state_fn_table);<br> <br>@@ -1221,13 +1239,11 @@<br> */<br> static void cdr_object_dispatch(struct cdr_object *cdr)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct ast_cdr *pub_cdr;<br> <br>- CDR_DEBUG(mod_cfg, "%p - Dispatching CDR for Party A %s, Party B %s\n", cdr,<br>- cdr->party_a.snapshot->name,<br>- cdr->party_b.snapshot ? cdr->party_b.snapshot->name : "<none>");<br>+ CDR_DEBUG("%p - Dispatching CDR for Party A %s, Party B %s\n", cdr,<br>+ cdr->party_a.snapshot->name,<br>+ cdr->party_b.snapshot ? cdr->party_b.snapshot->name : "<none>");<br> pub_cdr = cdr_object_create_public_records(cdr);<br> cdr_detach(pub_cdr);<br> }<br>@@ -1327,13 +1343,12 @@<br> * \brief Check to see if a CDR needs to be answered based on its Party A.<br> * Note that this is safe to call as much as you want - we won't answer twice<br> */<br>-static void cdr_object_check_party_a_answer(struct cdr_object *cdr) {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>-<br>+static void cdr_object_check_party_a_answer(struct cdr_object *cdr)<br>+{<br> if (cdr->party_a.snapshot->state == AST_STATE_UP && ast_tvzero(cdr->answer)) {<br> cdr->answer = ast_tvnow();<br> /* tv_usec is suseconds_t, which could be int or long */<br>- CDR_DEBUG(mod_cfg, "%p - Set answered time to %ld.%06ld\n", cdr,<br>+ CDR_DEBUG("%p - Set answered time to %ld.%06ld\n", cdr,<br> (long)cdr->answer.tv_sec,<br> (long)cdr->answer.tv_usec);<br> }<br>@@ -1489,15 +1504,13 @@<br> <br> static int single_state_process_dial_begin(struct cdr_object *cdr, struct ast_channel_snapshot *caller, struct ast_channel_snapshot *peer)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>-<br> if (caller && !strcasecmp(cdr->party_a.snapshot->name, caller->name)) {<br> base_process_party_a(cdr, caller);<br>- CDR_DEBUG(mod_cfg, "%p - Updated Party A %s snapshot\n", cdr,<br>- cdr->party_a.snapshot->name);<br>+ CDR_DEBUG("%p - Updated Party A %s snapshot\n", cdr,<br>+ cdr->party_a.snapshot->name);<br> cdr_object_swap_snapshot(&cdr->party_b, peer);<br>- CDR_DEBUG(mod_cfg, "%p - Updated Party B %s snapshot\n", cdr,<br>- cdr->party_b.snapshot->name);<br>+ CDR_DEBUG("%p - Updated Party B %s snapshot\n", cdr,<br>+ cdr->party_b.snapshot->name);<br> <br> /* If we have two parties, lock the application that caused the<br> * two parties to be associated. This prevents mid-call event<br>@@ -1507,8 +1520,8 @@<br> } else if (!strcasecmp(cdr->party_a.snapshot->name, peer->name)) {<br> /* We're the entity being dialed, i.e., outbound origination */<br> base_process_party_a(cdr, peer);<br>- CDR_DEBUG(mod_cfg, "%p - Updated Party A %s snapshot\n", cdr,<br>- cdr->party_a.snapshot->name);<br>+ CDR_DEBUG("%p - Updated Party A %s snapshot\n", cdr,<br>+ cdr->party_a.snapshot->name);<br> }<br> <br> cdr_object_transition_state(cdr, &dial_state_fn_table);<br>@@ -1530,7 +1543,6 @@<br> static int single_state_bridge_enter_comparison(struct cdr_object *cdr,<br> struct cdr_object *cand_cdr)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object_snapshot *party_a;<br> <br> /* Don't match on ourselves */<br>@@ -1541,7 +1553,7 @@<br> /* Try the candidate CDR's Party A first */<br> party_a = cdr_object_pick_party_a(&cdr->party_a, &cand_cdr->party_a);<br> if (!strcasecmp(party_a->snapshot->name, cdr->party_a.snapshot->name)) {<br>- CDR_DEBUG(mod_cfg, "%p - Party A %s has new Party B %s\n",<br>+ CDR_DEBUG("%p - Party A %s has new Party B %s\n",<br> cdr, cdr->party_a.snapshot->name, cand_cdr->party_a.snapshot->name);<br> cdr_object_snapshot_copy(&cdr->party_b, &cand_cdr->party_a);<br> if (!cand_cdr->party_b.snapshot) {<br>@@ -1561,7 +1573,7 @@<br> }<br> party_a = cdr_object_pick_party_a(&cdr->party_a, &cand_cdr->party_b);<br> if (!strcasecmp(party_a->snapshot->name, cdr->party_a.snapshot->name)) {<br>- CDR_DEBUG(mod_cfg, "%p - Party A %s has new Party B %s\n",<br>+ CDR_DEBUG("%p - Party A %s has new Party B %s\n",<br> cdr, cdr->party_a.snapshot->name, cand_cdr->party_b.snapshot->name);<br> cdr_object_snapshot_copy(&cdr->party_b, &cand_cdr->party_b);<br> return 0;<br>@@ -1937,7 +1949,6 @@<br> */<br> static void handle_dial_message(void *data, struct stasis_subscription *sub, struct stasis_message *message)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object *cdr;<br> struct ast_multi_channel_blob *payload = stasis_message_data(message);<br> struct ast_channel_snapshot *caller;<br>@@ -1961,12 +1972,12 @@<br> dial_status = ast_json_string_get(dial_status_blob);<br> }<br> <br>- CDR_DEBUG(mod_cfg, "Dial %s message for %s, %s: %u.%08u\n",<br>- ast_strlen_zero(dial_status) ? "Begin" : "End",<br>- caller ? caller->name : "(none)",<br>- peer ? peer->name : "(none)",<br>- (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>- (unsigned int)stasis_message_timestamp(message)->tv_usec);<br>+ CDR_DEBUG("Dial %s message for %s, %s: %u.%08u\n",<br>+ ast_strlen_zero(dial_status) ? "Begin" : "End",<br>+ caller ? caller->name : "(none)",<br>+ peer ? peer->name : "(none)",<br>+ (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_usec);<br> <br> /* Figure out who is running this show */<br> if (caller) {<br>@@ -1986,10 +1997,10 @@<br> if (!it_cdr->fn_table->process_dial_begin) {<br> continue;<br> }<br>- CDR_DEBUG(mod_cfg, "%p - Processing Dial Begin message for channel %s, peer %s\n",<br>- it_cdr,<br>- caller ? caller->name : "(none)",<br>- peer ? peer->name : "(none)");<br>+ CDR_DEBUG("%p - Processing Dial Begin message for channel %s, peer %s\n",<br>+ it_cdr,<br>+ caller ? caller->name : "(none)",<br>+ peer ? peer->name : "(none)");<br> res &= it_cdr->fn_table->process_dial_begin(it_cdr,<br> caller,<br> peer);<br>@@ -1997,10 +2008,10 @@<br> if (!it_cdr->fn_table->process_dial_end) {<br> continue;<br> }<br>- CDR_DEBUG(mod_cfg, "%p - Processing Dial End message for channel %s, peer %s\n",<br>- it_cdr,<br>- caller ? caller->name : "(none)",<br>- peer ? peer->name : "(none)");<br>+ CDR_DEBUG("%p - Processing Dial End message for channel %s, peer %s\n",<br>+ it_cdr,<br>+ caller ? caller->name : "(none)",<br>+ peer ? peer->name : "(none)");<br> it_cdr->fn_table->process_dial_end(it_cdr,<br> caller,<br> peer,<br>@@ -2090,7 +2101,6 @@<br> static void handle_channel_cache_message(void *data, struct stasis_subscription *sub, struct stasis_message *message)<br> {<br> struct cdr_object *cdr;<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct stasis_cache_update *update = stasis_message_data(message);<br> struct ast_channel_snapshot *old_snapshot;<br> struct ast_channel_snapshot *new_snapshot;<br>@@ -2148,7 +2158,7 @@<br> }<br> }<br> } else {<br>- CDR_DEBUG(mod_cfg, "%p - Beginning finalize/dispatch for %s\n", cdr, old_snapshot->name);<br>+ CDR_DEBUG("%p - Beginning finalize/dispatch for %s\n", cdr, old_snapshot->name);<br> for (it_cdr = cdr; it_cdr; it_cdr = it_cdr->next) {<br> cdr_object_finalize(it_cdr);<br> }<br>@@ -2228,8 +2238,6 @@<br> struct ast_bridge_blob *update = stasis_message_data(message);<br> struct ast_bridge_snapshot *bridge = update->bridge;<br> struct ast_channel_snapshot *channel = update->channel;<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object *cdr;<br> struct cdr_object *it_cdr;<br> struct bridge_leave_data leave_data = {<br>@@ -2246,10 +2254,10 @@<br> return;<br> }<br> <br>- CDR_DEBUG(mod_cfg, "Bridge Leave message for %s: %u.%08u\n",<br>- channel->name,<br>- (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>- (unsigned int)stasis_message_timestamp(message)->tv_usec);<br>+ CDR_DEBUG("Bridge Leave message for %s: %u.%08u\n",<br>+ channel->name,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_usec);<br> <br> cdr = ao2_find(active_cdrs_by_channel, channel->uniqueid, OBJ_SEARCH_KEY);<br> if (!cdr) {<br>@@ -2264,8 +2272,8 @@<br> if (!it_cdr->fn_table->process_bridge_leave) {<br> continue;<br> }<br>- CDR_DEBUG(mod_cfg, "%p - Processing Bridge Leave for %s\n",<br>- it_cdr, channel->name);<br>+ CDR_DEBUG("%p - Processing Bridge Leave for %s\n",<br>+ it_cdr, channel->name);<br> if (!it_cdr->fn_table->process_bridge_leave(it_cdr, bridge, channel)) {<br> ast_string_field_set(it_cdr, bridge, "");<br> left_bridge = 1;<br>@@ -2293,8 +2301,6 @@<br> static void bridge_candidate_add_to_cdr(struct cdr_object *cdr,<br> struct cdr_object_snapshot *party_b)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object *new_cdr;<br> <br> new_cdr = cdr_object_create_and_append(cdr);<br>@@ -2305,7 +2311,7 @@<br> cdr_object_check_party_a_answer(new_cdr);<br> ast_string_field_set(new_cdr, bridge, cdr->bridge);<br> cdr_object_transition_state(new_cdr, &bridge_state_fn_table);<br>- CDR_DEBUG(mod_cfg, "%p - Party A %s has new Party B %s\n",<br>+ CDR_DEBUG("%p - Party A %s has new Party B %s\n",<br> new_cdr, new_cdr->party_a.snapshot->name,<br> party_b->snapshot->name);<br> }<br>@@ -2324,8 +2330,6 @@<br> */<br> static int bridge_candidate_process(struct cdr_object *cdr, struct cdr_object *base_cand_cdr)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> struct cdr_object_snapshot *party_a;<br> struct cdr_object *cand_cdr;<br> <br>@@ -2358,7 +2362,7 @@<br> && strcasecmp(cand_cdr->party_b.snapshot->name, cdr->party_a.snapshot->name)) {<br> bridge_candidate_add_to_cdr(cand_cdr, &cdr->party_a);<br> } else {<br>- CDR_DEBUG(mod_cfg, "%p - Party A %s has new Party B %s\n",<br>+ CDR_DEBUG("%p - Party A %s has new Party B %s\n",<br> cand_cdr, cand_cdr->party_a.snapshot->name,<br> cdr->party_a.snapshot->name);<br> cdr_object_snapshot_copy(&cand_cdr->party_b, &cdr->party_a);<br>@@ -2408,8 +2412,6 @@<br> struct ast_bridge_snapshot *bridge,<br> struct ast_channel_snapshot *channel)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> int res = 1;<br> struct cdr_object *it_cdr;<br> struct cdr_object *new_cdr;<br>@@ -2421,8 +2423,8 @@<br> res &= it_cdr->fn_table->process_parking_bridge_enter(it_cdr, bridge, channel);<br> }<br> if (it_cdr->fn_table->process_party_a) {<br>- CDR_DEBUG(mod_cfg, "%p - Updating Party A %s snapshot\n", it_cdr,<br>- channel->name);<br>+ CDR_DEBUG("%p - Updating Party A %s snapshot\n", it_cdr,<br>+ channel->name);<br> it_cdr->fn_table->process_party_a(it_cdr, channel);<br> }<br> }<br>@@ -2448,8 +2450,6 @@<br> struct ast_bridge_snapshot *bridge,<br> struct ast_channel_snapshot *channel)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> enum process_bridge_enter_results result;<br> struct cdr_object *it_cdr;<br> struct cdr_object *new_cdr;<br>@@ -2459,15 +2459,15 @@<br> <br> for (it_cdr = cdr; it_cdr; it_cdr = it_cdr->next) {<br> if (it_cdr->fn_table->process_party_a) {<br>- CDR_DEBUG(mod_cfg, "%p - Updating Party A %s snapshot\n", it_cdr,<br>- channel->name);<br>+ CDR_DEBUG("%p - Updating Party A %s snapshot\n", it_cdr,<br>+ channel->name);<br> it_cdr->fn_table->process_party_a(it_cdr, channel);<br> }<br> <br> /* Notify all states that they have entered a bridge */<br> if (it_cdr->fn_table->process_bridge_enter) {<br>- CDR_DEBUG(mod_cfg, "%p - Processing bridge enter for %s\n", it_cdr,<br>- channel->name);<br>+ CDR_DEBUG("%p - Processing bridge enter for %s\n", it_cdr,<br>+ channel->name);<br> result = it_cdr->fn_table->process_bridge_enter(it_cdr, bridge, channel);<br> switch (result) {<br> case BRIDGE_ENTER_ONLY_PARTY:<br>@@ -2530,8 +2530,6 @@<br> struct ast_bridge_snapshot *bridge = update->bridge;<br> struct ast_channel_snapshot *channel = update->channel;<br> struct cdr_object *cdr;<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> <br> if (filter_bridge_messages(bridge)) {<br> return;<br>@@ -2541,10 +2539,10 @@<br> return;<br> }<br> <br>- CDR_DEBUG(mod_cfg, "Bridge Enter message for channel %s: %u.%08u\n",<br>- channel->name,<br>- (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>- (unsigned int)stasis_message_timestamp(message)->tv_usec);<br>+ CDR_DEBUG("Bridge Enter message for channel %s: %u.%08u\n",<br>+ channel->name,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_usec);<br> <br> cdr = ao2_find(active_cdrs_by_channel, channel->uniqueid, OBJ_SEARCH_KEY);<br> if (!cdr) {<br>@@ -2574,8 +2572,6 @@<br> struct ast_parked_call_payload *payload = stasis_message_data(message);<br> struct ast_channel_snapshot *channel = payload->parkee;<br> struct cdr_object *cdr;<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br> int unhandled = 1;<br> struct cdr_object *it_cdr;<br> <br>@@ -2593,10 +2589,10 @@<br> return;<br> }<br> <br>- CDR_DEBUG(mod_cfg, "Parked Call message for channel %s: %u.%08u\n",<br>- channel->name,<br>- (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>- (unsigned int)stasis_message_timestamp(message)->tv_usec);<br>+ CDR_DEBUG("Parked Call message for channel %s: %u.%08u\n",<br>+ channel->name,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_sec,<br>+ (unsigned int)stasis_message_timestamp(message)->tv_usec);<br> <br> cdr = ao2_find(active_cdrs_by_channel, channel->uniqueid, OBJ_SEARCH_KEY);<br> if (!cdr) {<br>@@ -2646,20 +2642,37 @@<br> <br> struct ast_cdr_config *ast_cdr_get_config(void)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>- ao2_ref(mod_cfg->general, +1);<br>- return mod_cfg->general;<br>+ struct ast_cdr_config *general;<br>+ struct module_config *mod_cfg;<br>+<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return NULL;<br>+ }<br>+ general = ao2_bump(mod_cfg->general);<br>+ ao2_cleanup(mod_cfg);<br>+ return general;<br> }<br> <br> void ast_cdr_set_config(struct ast_cdr_config *config)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> <br>- ao2_cleanup(mod_cfg->general);<br>- mod_cfg->general = config;<br>- ao2_ref(mod_cfg->general, +1);<br>+ if (!config) {<br>+ return;<br>+ }<br> <br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return;<br>+ }<br>+<br>+ ao2_replace(mod_cfg->general, config);<br>+<br>+ cdr_set_debug_mode(mod_cfg);<br> cdr_toggle_runtime_options();<br>+<br>+ ao2_cleanup(mod_cfg);<br> }<br> <br> int ast_cdr_is_enabled(void)<br>@@ -3299,15 +3312,20 @@<br> <br> static void post_cdr(struct ast_cdr *cdr)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> struct cdr_beitem *i;<br>+<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return;<br>+ }<br> <br> for (; cdr ; cdr = cdr->next) {<br> /* For people, who don't want to see unanswered single-channel events */<br> if (!ast_test_flag(&mod_cfg->general->settings, CDR_UNANSWERED) &&<br> cdr->disposition < AST_CDR_ANSWERED &&<br> (ast_strlen_zero(cdr->channel) || ast_strlen_zero(cdr->dstchannel))) {<br>- ast_debug(1, "Skipping CDR for %s since we weren't answered\n", cdr->channel);<br>+ ast_debug(1, "Skipping CDR for %s since we weren't answered\n", cdr->channel);<br> continue;<br> }<br> <br>@@ -3329,6 +3347,7 @@<br> }<br> AST_RWLIST_UNLOCK(&be_list);<br> }<br>+ ao2_cleanup(mod_cfg);<br> }<br> <br> int ast_cdr_set_property(const char *channel_name, enum ast_cdr_options option)<br>@@ -3547,7 +3566,7 @@<br> <br> static void cdr_submit_batch(int do_shutdown)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> struct cdr_batch_item *oldbatchitems = NULL;<br> pthread_t batch_post_thread = AST_PTHREADT_NULL;<br> <br>@@ -3562,9 +3581,13 @@<br> reset_batch();<br> ast_mutex_unlock(&cdr_batch_lock);<br> <br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+<br> /* if configured, spawn a new thread to post these CDRs,<br> also try to save as much as possible if we are shutting down safely */<br>- if (ast_test_flag(&mod_cfg->general->batch_settings.settings, BATCH_MODE_SCHEDULER_ONLY) || do_shutdown) {<br>+ if (!mod_cfg<br>+ || ast_test_flag(&mod_cfg->general->batch_settings.settings, BATCH_MODE_SCHEDULER_ONLY)<br>+ || do_shutdown) {<br> ast_debug(1, "CDR single-threaded batch processing begins now\n");<br> do_batch_backend_process(oldbatchitems);<br> } else {<br>@@ -3575,17 +3598,27 @@<br> ast_debug(1, "CDR multi-threaded batch processing begins now\n");<br> }<br> }<br>+<br>+ ao2_cleanup(mod_cfg);<br> }<br> <br> static int submit_scheduled_batch(const void *data)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>- cdr_submit_batch(0);<br>- /* manually reschedule from this point in time */<br>+ struct module_config *mod_cfg;<br> <br>+ cdr_submit_batch(0);<br>+<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return 0;<br>+ }<br>+<br>+ /* manually reschedule from this point in time */<br> ast_mutex_lock(&cdr_sched_lock);<br> cdr_sched = ast_sched_add(sched, mod_cfg->general->batch_settings.time * 1000, submit_scheduled_batch, NULL);<br> ast_mutex_unlock(&cdr_sched_lock);<br>+<br>+ ao2_cleanup(mod_cfg);<br> /* returning zero so the scheduler does not automatically reschedule */<br> return 0;<br> }<br>@@ -3619,7 +3652,7 @@<br> }<br> <br> /* maybe they disabled CDR stuff completely, so just drop it */<br>- if (!ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)) {<br>+ if (!mod_cfg || !ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)) {<br> ast_debug(1, "Dropping CDR !\n");<br> ast_cdr_free(cdr);<br> return;<br>@@ -3697,7 +3730,7 @@<br> <br> static char *handle_cli_debug(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> <br> switch (cmd) {<br> case CLI_INIT:<br>@@ -3715,6 +3748,11 @@<br> return CLI_SHOWUSAGE;<br> }<br> <br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ ast_cli(a->fd, "Could not set CDR debugging mode\n");<br>+ return CLI_SUCCESS;<br>+ }<br> if (!strcasecmp(a->argv[3], "on")<br> && !ast_test_flag(&mod_cfg->general->settings, CDR_DEBUG)) {<br> ast_set_flag(&mod_cfg->general->settings, CDR_DEBUG);<br>@@ -3724,6 +3762,8 @@<br> ast_clear_flag(&mod_cfg->general->settings, CDR_DEBUG);<br> ast_cli(a->fd, "CDR debugging disabled\n");<br> }<br>+ cdr_set_debug_mode(mod_cfg);<br>+ ao2_cleanup(mod_cfg);<br> <br> return CLI_SUCCESS;<br> }<br>@@ -3910,7 +3950,7 @@<br> static char *handle_cli_status(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br> {<br> struct cdr_beitem *beitem = NULL;<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> int cnt = 0;<br> long nextbatchtime = 0;<br> <br>@@ -3927,6 +3967,11 @@<br> <br> if (a->argc > 3) {<br> return CLI_SHOWUSAGE;<br>+ }<br>+<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return CLI_FAILURE;<br> }<br> <br> ast_cli(a->fd, "\n");<br>@@ -3965,12 +4010,13 @@<br> ast_cli(a->fd, "\n");<br> }<br> <br>+ ao2_cleanup(mod_cfg);<br> return CLI_SUCCESS;<br> }<br> <br> static char *handle_cli_submit(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> <br> switch (cmd) {<br> case CLI_INIT:<br>@@ -3987,18 +4033,20 @@<br> return CLI_SHOWUSAGE;<br> }<br> <br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (!mod_cfg) {<br>+ return CLI_FAILURE;<br>+ }<br>+<br> if (!ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)) {<br> ast_cli(a->fd, "Cannot submit CDR batch: CDR engine disabled.\n");<br>- return CLI_SUCCESS;<br>- }<br>-<br>- if (ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {<br>+ } else if (ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {<br> ast_cli(a->fd, "Cannot submit CDR batch: batch mode not enabled.\n");<br>- return CLI_SUCCESS;<br>+ } else {<br>+ submit_unscheduled_batch();<br>+ ast_cli(a->fd, "Submitted CDRs to backend engines for processing. This may take a while.\n");<br> }<br>-<br>- submit_unscheduled_batch();<br>- ast_cli(a->fd, "Submitted CDRs to backend engines for processing. This may take a while.\n");<br>+ ao2_cleanup(mod_cfg);<br> <br> return CLI_SUCCESS;<br> }<br>@@ -4094,8 +4142,6 @@<br> <br> static int process_config(int reload)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg, module_config_alloc(), ao2_cleanup);<br>-<br> if (!reload) {<br> if (aco_info_init(&cfg_info)) {<br> return 1;<br>@@ -4114,19 +4160,26 @@<br> aco_option_register(&cfg_info, "time", ACO_EXACT, general_options, DEFAULT_BATCH_TIME, OPT_UINT_T, PARSE_IN_RANGE, FLDSET(struct ast_cdr_config, batch_settings.time), 0, MAX_BATCH_TIME);<br> }<br> <br>- if (aco_process_config(&cfg_info, reload)) {<br>- if (!mod_cfg) {<br>+ if (aco_process_config(&cfg_info, reload) == ACO_PROCESS_ERROR) {<br>+ struct module_config *mod_cfg;<br>+<br>+ if (reload) {<br> return 1;<br> }<br>+<br> /* If we couldn't process the configuration and this wasn't a reload,<br> * create a default config<br> */<br>- if (!reload && !(aco_set_defaults(&general_option, "general", mod_cfg->general))) {<br>- ast_log(LOG_NOTICE, "Failed to process CDR configuration; using defaults\n");<br>- ao2_global_obj_replace_unref(module_configs, mod_cfg);<br>- return 0;<br>+ mod_cfg = module_config_alloc();<br>+ if (!mod_cfg<br>+ || aco_set_defaults(&general_option, "general", mod_cfg->general)) {<br>+ ao2_cleanup(mod_cfg);<br>+ return 1;<br> }<br>- return 1;<br>+ ast_log(LOG_NOTICE, "Failed to process CDR configuration; using defaults\n");<br>+ ao2_global_obj_replace_unref(module_configs, mod_cfg);<br>+ cdr_set_debug_mode(mod_cfg);<br>+ ao2_cleanup(mod_cfg);<br> }<br> <br> return 0;<br>@@ -4213,13 +4266,15 @@<br> */<br> static int cdr_toggle_runtime_options(void)<br> {<br>- RAII_VAR(struct module_config *, mod_cfg,<br>- ao2_global_obj_ref(module_configs), ao2_cleanup);<br>+ struct module_config *mod_cfg;<br> <br>- if (ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)) {<br>+ mod_cfg = ao2_global_obj_ref(module_configs);<br>+ if (mod_cfg<br>+ && ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)) {<br> if (create_subscriptions()) {<br> destroy_subscriptions();<br> ast_log(AST_LOG_ERROR, "Failed to create Stasis subscriptions\n");<br>+ ao2_cleanup(mod_cfg);<br> return -1;<br> }<br> if (ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {<br>@@ -4231,8 +4286,9 @@<br> destroy_subscriptions();<br> ast_log(LOG_NOTICE, "CDR logging disabled.\n");<br> }<br>+ ao2_cleanup(mod_cfg);<br> <br>- return 0;<br>+ return mod_cfg ? 0 : -1;<br> }<br> <br> int ast_cdr_engine_init(void)<br>@@ -4326,22 +4382,27 @@<br> <br> int ast_cdr_engine_reload(void)<br> {<br>- RAII_VAR(struct module_config *, old_mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);<br>- RAII_VAR(struct module_config *, mod_cfg, NULL, ao2_cleanup);<br>+ struct module_config *old_mod_cfg;<br>+ struct module_config *mod_cfg;<br>+<br>+ old_mod_cfg = ao2_global_obj_ref(module_configs);<br> <br> if (!old_mod_cfg || process_config(1)) {<br>+ ao2_cleanup(old_mod_cfg);<br> return -1;<br> }<br> <br> mod_cfg = ao2_global_obj_ref(module_configs);<br>-<br>- if (!ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED) ||<br>- !(ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE))) {<br>+ if (!mod_cfg<br>+ || !ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED)<br>+ || !ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {<br> /* If batch mode used to be enabled, finalize the batch */<br> if (ast_test_flag(&old_mod_cfg->general->settings, CDR_BATCHMODE)) {<br> finalize_batch_mode();<br> }<br> }<br>+ ao2_cleanup(mod_cfg);<br> <br>+ ao2_cleanup(old_mod_cfg);<br> return cdr_toggle_runtime_options();<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6785">change 6785</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6785"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Iceaad93172862f610cad0188956634187bfcc7cd </div>
<div style="display:none"> Gerrit-Change-Number: 6785 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>