<p>Joshua Colp has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/9316">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">main/cdr.c: Alleviate CDR deadlock<br><br>There is a rare case (do to the infrequent timing involved) where<br>CDR submission threads in batch mode can deadlock with a currently<br>running CDR batch process. This patch should remove the need for<br>holding the lock in the scheduler and should clean a few code<br>paths up that inconsistently submitted new work to the CDR batch<br>processor.<br><br>ASTERISK-27909<br><br>Change-Id: I6333e865db7c593c102c2fd948cecdb96481974d<br>Reported-by: Denis Lebedev<br>---<br>M main/cdr.c<br>1 file changed, 16 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/16/9316/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/cdr.c b/main/cdr.c<br>index 496fb36..c048761 100644<br>--- a/main/cdr.c<br>+++ b/main/cdr.c<br>@@ -3754,6 +3754,7 @@<br> static int submit_scheduled_batch(const void *data)<br> {<br> struct module_config *mod_cfg;<br>+ int nextms;<br> <br> cdr_submit_batch(0);<br> <br>@@ -3762,25 +3763,23 @@<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>+ /* Calculate the next scheduled interval */<br>+ nextms = mod_cfg->general->batch_settings.time * 1000;<br> <br> ao2_cleanup(mod_cfg);<br>- /* returning zero so the scheduler does not automatically reschedule */<br>- return 0;<br>+<br>+ return nextms;<br> }<br> <br> /*! Do not hold the batch lock while calling this function */<br>-static void submit_unscheduled_batch(void)<br>+static void start_batch_mode(void)<br> {<br> /* Prevent two deletes from happening at the same time */<br> ast_mutex_lock(&cdr_sched_lock);<br> /* this is okay since we are not being called from within the scheduler */<br> AST_SCHED_DEL(sched, cdr_sched);<br> /* schedule the submission to occur ASAP (1 ms) */<br>- cdr_sched = ast_sched_add(sched, 1, submit_scheduled_batch, NULL);<br>+ cdr_sched = ast_sched_add_variable(sched, 1, submit_scheduled_batch, NULL, 1);<br> ast_mutex_unlock(&cdr_sched_lock);<br> <br> /* signal the do_cdr thread to wakeup early and do some work (that lazy thread ;) */<br>@@ -3845,9 +3844,9 @@<br> }<br> ast_mutex_unlock(&cdr_batch_lock);<br> <br>- /* Don't call submit_unscheduled_batch with the cdr_batch_lock held */<br>+ /* Don't submit a batch with cdr_batch_lock held */<br> if (submit_batch) {<br>- submit_unscheduled_batch();<br>+ start_batch_mode();<br> }<br> }<br> <br>@@ -3861,7 +3860,7 @@<br> struct timeval now;<br> schedms = ast_sched_wait(sched);<br> /* this shouldn't happen, but provide a 1 second default just in case */<br>- if (schedms <= 0)<br>+ if (schedms < 0)<br> schedms = 1000;<br> now = ast_tvadd(ast_tvnow(), ast_samp2tv(schedms, 1000));<br> timeout.tv_sec = now.tv_sec;<br>@@ -3871,7 +3870,7 @@<br> ast_cond_timedwait(&cdr_pending_cond, &cdr_pending_lock, &timeout);<br> numevents = ast_sched_runq(sched);<br> ast_mutex_unlock(&cdr_pending_lock);<br>- ast_debug(2, "Processed %d scheduled CDR batches from the run queue\n", numevents);<br>+ ast_debug(2, "Processed %d CDR batches from the run queue\n", numevents);<br> }<br> <br> return NULL;<br>@@ -4189,7 +4188,7 @@<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> } else {<br>- submit_unscheduled_batch();<br>+ start_batch_mode();<br> ast_cli(a->fd, "Submitted CDRs to backend engines for processing. This may take a while.\n");<br> }<br> ao2_cleanup(mod_cfg);<br>@@ -4305,7 +4304,7 @@<br> aco_option_register(&cfg_info, "scheduleronly", ACO_EXACT, general_options, DEFAULT_BATCH_SCHEDULER_ONLY, OPT_BOOLFLAG_T, 1, FLDSET(struct ast_cdr_config, batch_settings.settings), BATCH_MODE_SCHEDULER_ONLY);<br> aco_option_register(&cfg_info, "safeshutdown", ACO_EXACT, general_options, DEFAULT_BATCH_SAFE_SHUTDOWN, OPT_BOOLFLAG_T, 1, FLDSET(struct ast_cdr_config, batch_settings.settings), BATCH_MODE_SAFE_SHUTDOWN);<br> aco_option_register(&cfg_info, "size", ACO_EXACT, general_options, DEFAULT_BATCH_SIZE, OPT_UINT_T, PARSE_IN_RANGE, FLDSET(struct ast_cdr_config, batch_settings.size), 0, MAX_BATCH_SIZE);<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>+ 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), 1, MAX_BATCH_TIME);<br> }<br> <br> if (aco_process_config(&cfg_info, reload) == ACO_PROCESS_ERROR) {<br>@@ -4371,8 +4370,6 @@<br> <br> static void cdr_enable_batch_mode(struct ast_cdr_config *config)<br> {<br>- SCOPED_LOCK(batch, &cdr_batch_lock, ast_mutex_lock, ast_mutex_unlock);<br>-<br> /* Only create the thread level portions once */<br> if (cdr_thread == AST_PTHREADT_NULL) {<br> ast_cond_init(&cdr_pending_cond, NULL);<br>@@ -4382,9 +4379,9 @@<br> }<br> }<br> <br>- /* Kill the currently scheduled item */<br>- AST_SCHED_DEL(sched, cdr_sched);<br>- cdr_sched = ast_sched_add(sched, config->batch_settings.time * 1000, submit_scheduled_batch, NULL);<br>+ /* Start the batching process */<br>+ start_batch_mode();<br>+<br> ast_log(LOG_NOTICE, "CDR batch mode logging enabled, first of either size %u or time %u seconds.\n",<br> config->batch_settings.size, config->batch_settings.time);<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/9316">change 9316</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/9316"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I6333e865db7c593c102c2fd948cecdb96481974d </div>
<div style="display:none"> Gerrit-Change-Number: 9316 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>