<p>Matthew Fredrickson has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/9270">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">main/cdr.c: Alleviates 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.  This is for ASTERISK-27909.<br><br>Change-Id: I6333e865db7c593c102c2fd948cecdb96481974d<br>Reported-by: Denis Lebedev<br>---<br>M main/cdr.c<br>1 file changed, 20 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/70/9270/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/cdr.c b/main/cdr.c<br>index b0a48e1..9e44f62 100644<br>--- a/main/cdr.c<br>+++ b/main/cdr.c<br>@@ -3784,6 +3784,7 @@<br> static int submit_scheduled_batch(const void *data)<br> {<br>  struct module_config *mod_cfg;<br>+       int nextms = 0;<br> <br>    cdr_submit_batch(0);<br> <br>@@ -3793,24 +3794,29 @@<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>+       nextms = mod_cfg->general->batch_settings.time * 1000;<br>+<br>+      if (!nextms) {<br>+               /* I hate doing this, but previously if you set the batch time to 0, it would continuously<br>+              resubmit batches to the scheduler.  IMHO that seems buggy, but I'd rather not break it<br>+                   if someone was relying on that behavior */<br>+                nextms = 1;<br>+  }<br> <br>  ao2_cleanup(mod_cfg);<br>         /* returning zero so the scheduler does not automatically reschedule */<br>-      return 0;<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>@@ -3875,9 +3881,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 call start_batch with cdr_batch_lock held */<br>     if (submit_batch) {<br>-          submit_unscheduled_batch();<br>+          start_batch_mode();<br>   }<br> }<br> <br>@@ -3890,8 +3896,8 @@<br>       for (;;) {<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>+         /* this only can happen if someone sets there batch time to 0 or does 'cdr submit' from the CLI back to back quickly */<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>@@ -4219,7 +4225,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>@@ -4412,9 +4418,10 @@<br>                 }<br>     }<br> <br>+ /* Start the batching process */<br>+     start_batch_mode();<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>        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/9270">change 9270</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/9270"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </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: 9270 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Matthew Fredrickson <creslin@digium.com> </div>