<p>Mark Murawski has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/17798">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pbx: Ensure we don't operate on dialplan while dialplan is reloading, which causes deadlocks<br><br>Previously it was possible to execute 'dialplan add' during a dialplan<br>reload, causing deadlocks<br><br>Now we prevent 'dialplan add' until the dialplan completes reloading.<br>Any 'dialplan add' executed during a reload will fail gracefully.<br><br>This change also prevents 'continuous dialplan reload'. Previously<br>subsequent 'dialplan reload' would all be queued. For example 10x<br>'dialplan reload' will result in asterisk being busy reloading<br>dialplan 10 times in a row, which is doubtfully desited behavior on<br>any system when there's backend scripts instructing Asterisk to reload<br>upon change.<br><br>Now, subsequent 'dialplan reload' will result in the normal 'module<br>reload in progress' type of behavior that many other modules implement<br>if they recieve commands during a reload.<br><br>ASTERISK-29774 #close<br>Reported-by: Steve Sether<br><br>Change-Id: I435db97fc56f18b6f5c25a3e400603817b07aa49<br>---<br>M include/asterisk/module.h<br>M main/loader.c<br>M main/pbx.c<br>M pbx/pbx_config.c<br>4 files changed, 42 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/98/17798/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/asterisk/module.h b/include/asterisk/module.h</span><br><span>index cce8735..9e089c7 100644</span><br><span>--- a/include/asterisk/module.h</span><br><span>+++ b/include/asterisk/module.h</span><br><span>@@ -178,6 +178,8 @@</span><br><span> */</span><br><span> enum ast_module_reload_result ast_module_reload(const char *name);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+enum ast_module_reload_result ast_module_reload_pending_check(const char *name);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*!</span><br><span> * \brief Notify when usecount has been changed.</span><br><span> *</span><br><span>diff --git a/main/loader.c b/main/loader.c</span><br><span>index aafcd3b..1058721 100644</span><br><span>--- a/main/loader.c</span><br><span>+++ b/main/loader.c</span><br><span>@@ -1559,6 +1559,24 @@</span><br><span> publish_load_message_type("Reload", S_OR(name, "All"), res_buffer);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+enum ast_module_reload_result ast_module_reload_pending_check(const char *name)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If we aren't fully booted, we just pretend we reloaded but we queue this</span><br><span style="color: hsl(120, 100%, 40%);">+ up to run once we are booted up. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!modules_loaded) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return AST_MODULE_RELOAD_IN_PROGRESS;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_mutex_trylock(&reloadlock)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_verb(3, "The previous reload command didn't finish yet\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return AST_MODULE_RELOAD_IN_PROGRESS;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_mutex_unlock(&reloadlock);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return AST_MODULE_RELOAD_SUCCESS;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> enum ast_module_reload_result ast_module_reload(const char *name)</span><br><span> {</span><br><span> struct ast_module *cur;</span><br><span>diff --git a/main/pbx.c b/main/pbx.c</span><br><span>index 07cf8e7..0401b71 100644</span><br><span>--- a/main/pbx.c</span><br><span>+++ b/main/pbx.c</span><br><span>@@ -8508,6 +8508,11 @@</span><br><span> return ast_mutex_lock(&conlock);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+int ast_rdlock_contexts_try(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return ast_mutex_trylock(&conlock);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> int ast_unlock_contexts(void)</span><br><span> {</span><br><span> return ast_mutex_unlock(&conlock);</span><br><span>diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c</span><br><span>index 648d751..1d3a08d 100644</span><br><span>--- a/pbx/pbx_config.c</span><br><span>+++ b/pbx/pbx_config.c</span><br><span>@@ -1100,6 +1100,11 @@</span><br><span> if (strcmp(a->argv[6], "replace"))</span><br><span> return CLI_SHOWUSAGE;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_module_reload_pending_check("pbx_config") == AST_MODULE_RELOAD_IN_PROGRESS) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_cli(a->fd, "Dialplan reload in progress. Cannot add extensions right now.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> whole_exten = ast_strdupa(a->argv[3]);</span><br><span> exten = strsep(&whole_exten,",");</span><br><span> if (strchr(exten, '/')) {</span><br><span>@@ -1575,6 +1580,8 @@</span><br><span> </span><br><span> static char *handle_cli_dialplan_reload(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ static char *ret;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> switch (cmd) {</span><br><span> case CLI_INIT:</span><br><span> e->command = "dialplan reload";</span><br><span>@@ -1591,12 +1598,15 @@</span><br><span> if (a->argc != 2)</span><br><span> return CLI_SHOWUSAGE;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (clearglobalvars_config)</span><br><span style="color: hsl(0, 100%, 40%);">- pbx_builtin_clear_globals();</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Lock-Protected reload with early abort if currently reloading or the last reload has not finished */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* This runs reload() under the hood */</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = ast_module_reload("pbx_config") == AST_MODULE_RELOAD_SUCCESS ? CLI_SUCCESS : CLI_FAILURE;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- pbx_load_module();</span><br><span style="color: hsl(0, 100%, 40%);">- ast_cli(a->fd, "Dialplan reloaded.\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return CLI_SUCCESS;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret == CLI_SUCCESS) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_cli(a->fd, "Dialplan reloaded.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span> }</span><br><span> </span><br><span> /*!</span><br><span>@@ -2153,6 +2163,8 @@</span><br><span> {</span><br><span> if (clearglobalvars_config)</span><br><span> pbx_builtin_clear_globals();</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Lock-Protected reload not needed because we're already being called from ast_module_reload() */</span><br><span> return pbx_load_module();</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/17798">change 17798</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/17798"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I435db97fc56f18b6f5c25a3e400603817b07aa49 </div>
<div style="display:none"> Gerrit-Change-Number: 17798 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>