<p>Michael Bradeen has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18020">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">taskprocessor.c: Prevent crash on graceful shutdown<br><br>When tps_shutdown is called as part of the cleanup process there is a<br>chance that one of the taskprocessors that references the<br>tps_singletons object is still running. The change is to allow for<br>tps_shutdown to check tps_singleton's container count and give the<br>running taskprocessors a chance to finish. If after<br>AST_TASKPROCESSOR_SHUTDOWN_MAX_WAIT (10) seconds there are still<br>container references we shutdown anyway as this is most likely a bug<br>due to a taskprocessor not being unreferenced.<br><br>ASTERISK-29365<br><br>Change-Id: Ia932fc003d316389b9c4fd15ad6594458c9727f1<br>---<br>M main/taskprocessor.c<br>1 file changed, 67 insertions(+), 0 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/20/18020/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/taskprocessor.c b/main/taskprocessor.c</span><br><span>index 0209852..c5f3558 100644</span><br><span>--- a/main/taskprocessor.c</span><br><span>+++ b/main/taskprocessor.c</span><br><span>@@ -156,6 +156,9 @@</span><br><span> static char *cli_tps_reset_stats(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a);</span><br><span> static char *cli_tps_reset_stats_all(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 int tps_sort_cb(const void *obj_left, const void *obj_right, int flags);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static struct ast_cli_entry taskprocessor_clis[] = {</span><br><span> AST_CLI_DEFINE(cli_tps_ping, "Ping a named task processor"),</span><br><span> AST_CLI_DEFINE(cli_tps_report, "List instantiated task processors and statistics"),</span><br><span>@@ -284,12 +287,76 @@</span><br><span> .dtor = default_listener_pvt_dtor,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! \brief How many seconds to wait for running taskprocessors to finish on shutdown. */</span><br><span style="color: hsl(120, 100%, 40%);">+#define AST_TASKPROCESSOR_SHUTDOWN_MAX_WAIT 10</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*!</span><br><span> * \internal</span><br><span> * \brief Clean up resources on Asterisk shutdown</span><br><span> */</span><br><span> static void tps_shutdown(void)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ int objcount;</span><br><span style="color: hsl(120, 100%, 40%);">+ int tries;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ao2_container *sorted_tps;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_taskprocessor *tps;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ao2_iterator iter;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct timespec delay = {1, 0};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* During shutdown there may still be taskprocessor threads running and those</span><br><span style="color: hsl(120, 100%, 40%);">+ * tasprocessors reference tps_singletons. When those taskprocessors finish</span><br><span style="color: hsl(120, 100%, 40%);">+ * they will call ast_taskprocessor_unreference, creating a race condition which</span><br><span style="color: hsl(120, 100%, 40%);">+ * can result in tps_singletons being referenced after being deleted. To try and</span><br><span style="color: hsl(120, 100%, 40%);">+ * avoid this we check the container count and if greater than zero, give the</span><br><span style="color: hsl(120, 100%, 40%);">+ * running taskprocessors a chance to finish */</span><br><span style="color: hsl(120, 100%, 40%);">+ objcount = ao2_container_count(tps_singletons);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (objcount > 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "waiting for taskprocessor shutdown, %d tps object(s) still allocated.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ objcount);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* give the running taskprocessors a chance to finish, up to</span><br><span style="color: hsl(120, 100%, 40%);">+ * AST_TASKPROCESSOR_SHUTDOWN_MAX_WAIT seconds */</span><br><span style="color: hsl(120, 100%, 40%);">+ for (tries = 0; tries < AST_TASKPROCESSOR_SHUTDOWN_MAX_WAIT; tries++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ while (nanosleep(&delay, &delay));</span><br><span style="color: hsl(120, 100%, 40%);">+ objcount = ao2_container_count(tps_singletons);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* if count is 0, we are done waiting */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (objcount == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ delay.tv_sec = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ delay.tv_nsec = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "waiting for taskprocessor shutdown, %d tps object(s) still allocated.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ objcount);</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%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* rather than try forever, risk an assertion on shutdown. This probably indicates</span><br><span style="color: hsl(120, 100%, 40%);">+ * a taskprocessor was not cleaned up somewhere */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (objcount > 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Asertion may occur, the following taskprocessors are still runing:\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ sorted_tps = ao2_container_alloc_rbtree(AO2_ALLOC_OPT_LOCK_NOLOCK, 0, tps_sort_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+ NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!sorted_tps || ao2_container_dup(sorted_tps, tps_singletons, 0)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_ERROR, "unable to get sorted list of taskprocessors");</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ else {</span><br><span style="color: hsl(120, 100%, 40%);">+ iter = ao2_iterator_init(sorted_tps, AO2_ITERATOR_UNLINK);</span><br><span style="color: hsl(120, 100%, 40%);">+ while ((tps = ao2_iterator_next(&iter))) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_ERROR, "taskprocessor '%s'\n", tps->name);</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%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(sorted_tps);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ else {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "All waiting taskprocessors cleared!\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> ast_cli_unregister_multiple(taskprocessor_clis, ARRAY_LEN(taskprocessor_clis));</span><br><span> AST_VECTOR_CALLBACK_VOID(&overloaded_subsystems, ast_free);</span><br><span> AST_VECTOR_RW_FREE(&overloaded_subsystems);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18020">change 18020</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/+/18020"/><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-Change-Id: Ia932fc003d316389b9c4fd15ad6594458c9727f1 </div>
<div style="display:none"> Gerrit-Change-Number: 18020 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>