Resolve some errors produced during module unload of chan_iax2.
authorRussell Bryant <russell@russellbryant.com>
Wed, 23 Jun 2010 23:09:28 +0000 (23:09 +0000)
committerRussell Bryant <russell@russellbryant.com>
Wed, 23 Jun 2010 23:09:28 +0000 (23:09 +0000)
The external test suite stops Asterisk using the "core stop gracefully" command.
The logs from the tests show that there are a number of problems with Asterisk
trying to cleanly shut down.  This patch addresses the following type of error
that comes from chan_iax2:

[Jun 22 16:58:11] ERROR[29884]: lock.c:129 __ast_pthread_mutex_destroy:
                chan_iax2.c line 11371 (iax2_process_thread_cleanup):
                Error destroying mutex &thread->lock: Device or resource busy

For an example in the context of a build, see:

http://bamboo.asterisk.org/browse/AST-TRUNK-739/log

The primary purpose of this patch is to change the thread pool shutdown
procedure to be more explicit to ensure that the thread exits from a point
where it is not holding a lock.  While testing that, I encountered various
crashes due to the order of operations in unload_module() being problematic.
I reordered some things there, as well.

Review: https://reviewboard.asterisk.org/r/736/

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@272370 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/chan_iax2.c

index f5cc5f8..b416779 100644 (file)
@@ -1026,6 +1026,7 @@ struct iax2_thread {
         *  a call which this thread is already processing a full frame for, they
         *  are queued up here. */
        AST_LIST_HEAD_NOLOCK(, iax2_pkt_buf) full_frames;
+       unsigned char stop;
 };
 
 /* Thread lists */
@@ -1351,7 +1352,7 @@ static struct iax2_thread *find_idle_thread(void)
        ast_mutex_lock(&thread->init_lock);
 
        /* Create thread and send it on it's way */
-       if (ast_pthread_create_detached_background(&thread->threadid, NULL, iax2_process_thread, thread)) {
+       if (ast_pthread_create_background(&thread->threadid, NULL, iax2_process_thread, thread)) {
                ast_cond_destroy(&thread->cond);
                ast_mutex_destroy(&thread->lock);
                ast_free(thread);
@@ -11383,13 +11384,22 @@ static void *iax2_process_thread(void *data)
        struct timespec ts;
        int put_into_idle = 0;
        int first_time = 1;
+       int old_state;
 
-       ast_atomic_fetchadd_int(&iaxactivethreadcount,1);
+       ast_atomic_fetchadd_int(&iaxactivethreadcount, 1);
+
+       pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_state);
        pthread_cleanup_push(iax2_process_thread_cleanup, data);
-       for(;;) {
+
+       for (;;) {
                /* Wait for something to signal us to be awake */
                ast_mutex_lock(&thread->lock);
 
+               if (thread->stop) {
+                       ast_mutex_unlock(&thread->lock);
+                       break;
+               }
+
                /* Flag that we're ready to accept signals */
                if (first_time) {
                        signal_condition(&thread->init_lock, &thread->init_cond);
@@ -11397,8 +11407,9 @@ static void *iax2_process_thread(void *data)
                }
 
                /* Put into idle list if applicable */
-               if (put_into_idle)
+               if (put_into_idle) {
                        insert_idle_thread(thread);
+               }
 
                if (thread->type == IAX_THREAD_TYPE_DYNAMIC) {
                        struct iax2_thread *t = NULL;
@@ -11409,7 +11420,7 @@ static void *iax2_process_thread(void *data)
                        if (ast_cond_timedwait(&thread->cond, &thread->lock, &ts) == ETIMEDOUT) {
                                /* This thread was never put back into the available dynamic
                                 * thread list, so just go away. */
-                               if (!put_into_idle) {
+                               if (!put_into_idle || thread->stop) {
                                        ast_mutex_unlock(&thread->lock);
                                        break;
                                }
@@ -11431,8 +11442,7 @@ static void *iax2_process_thread(void *data)
                                wait = ast_tvadd(ast_tvnow(), ast_samp2tv(30000, 1000));
                                ts.tv_sec = wait.tv_sec;
                                ts.tv_nsec = wait.tv_usec * 1000;
-                               if (ast_cond_timedwait(&thread->cond, &thread->lock, &ts) == ETIMEDOUT)
-                               {
+                               if (ast_cond_timedwait(&thread->cond, &thread->lock, &ts) == ETIMEDOUT) {
                                        ast_mutex_unlock(&thread->lock);
                                        break;
                                }
@@ -11446,11 +11456,15 @@ static void *iax2_process_thread(void *data)
 
                ast_mutex_unlock(&thread->lock);
 
+               if (thread->stop) {
+                       break;
+               }
+
                if (thread->iostate == IAX_IOSTATE_IDLE)
                        continue;
 
                /* See what we need to do */
-               switch(thread->iostate) {
+               switch (thread->iostate) {
                case IAX_IOSTATE_READY:
                        thread->actions++;
                        thread->iostate = IAX_IOSTATE_PROCESSING;
@@ -11906,7 +11920,7 @@ static int start_network_thread(void)
                        thread->threadnum = ++threadcount;
                        ast_mutex_init(&thread->lock);
                        ast_cond_init(&thread->cond, NULL);
-                       if (ast_pthread_create_detached(&thread->threadid, NULL, iax2_process_thread, thread)) {
+                       if (ast_pthread_create_background(&thread->threadid, NULL, iax2_process_thread, thread)) {
                                ast_log(LOG_WARNING, "Failed to create new thread!\n");
                                ast_free(thread);
                                thread = NULL;
@@ -13838,55 +13852,62 @@ static struct ast_cli_entry cli_iax2[] = {
 #endif /* IAXTESTS */
 };
 
+static void cleanup_thread_list(void *head)
+{
+       AST_LIST_HEAD(iax2_thread_list, iax2_thread);
+       struct iax2_thread_list *list_head = head;
+       struct iax2_thread *thread;
+
+       AST_LIST_LOCK(list_head);
+       while ((thread = AST_LIST_REMOVE_HEAD(&idle_list, list))) {
+               pthread_t thread_id = thread->threadid;
+
+               thread->stop = 1;
+               signal_condition(&thread->lock, &thread->cond);
+
+               AST_LIST_UNLOCK(list_head);
+               pthread_join(thread_id, NULL);
+               AST_LIST_LOCK(list_head);
+       }
+       AST_LIST_UNLOCK(list_head);
+}
+
 static int __unload_module(void)
 {
-       struct iax2_thread *thread = NULL;
        struct ast_context *con;
        int x;
 
+       ast_manager_unregister("IAXpeers");
+       ast_manager_unregister("IAXpeerlist");
+       ast_manager_unregister("IAXnetstats");
+       ast_manager_unregister("IAXregistry");
+       ast_unregister_application(papp);
+       ast_cli_unregister_multiple(cli_iax2, ARRAY_LEN(cli_iax2));
+       ast_unregister_switch(&iax2_switch);
+       ast_channel_unregister(&iax2_tech);
+
        if (netthreadid != AST_PTHREADT_NULL) {
                pthread_cancel(netthreadid);
                pthread_kill(netthreadid, SIGURG);
                pthread_join(netthreadid, NULL);
        }
 
-       sched = ast_sched_thread_destroy(sched);
+       for (x = 0; x < ARRAY_LEN(iaxs); x++) {
+               if (iaxs[x]) {
+                       iax2_destroy(x);
+               }
+       }
 
        /* Call for all threads to halt */
-       AST_LIST_LOCK(&idle_list);
-       while ((thread = AST_LIST_REMOVE_HEAD(&idle_list, list)))
-               pthread_cancel(thread->threadid);
-       AST_LIST_UNLOCK(&idle_list);
+       cleanup_thread_list(&idle_list);
+       cleanup_thread_list(&active_list);
+       cleanup_thread_list(&dynamic_list);
 
-       AST_LIST_LOCK(&active_list);
-       while ((thread = AST_LIST_REMOVE_HEAD(&active_list, list)))
-               pthread_cancel(thread->threadid);
-       AST_LIST_UNLOCK(&active_list);
+       sched = ast_sched_thread_destroy(sched);
 
-       AST_LIST_LOCK(&dynamic_list);
-       while ((thread = AST_LIST_REMOVE_HEAD(&dynamic_list, list)))
-               pthread_cancel(thread->threadid);
-       AST_LIST_UNLOCK(&dynamic_list);
-       
-       /* Wait for threads to exit */
-       while(0 < iaxactivethreadcount)
-               usleep(10000);
-       
        ast_netsock_release(netsock);
        ast_netsock_release(outsock);
-       for (x = 0; x < ARRAY_LEN(iaxs); x++) {
-               if (iaxs[x]) {
-                       iax2_destroy(x);
-               }
-       }
-       ast_manager_unregister( "IAXpeers" );
-       ast_manager_unregister( "IAXpeerlist" );
-       ast_manager_unregister( "IAXnetstats" );
-       ast_manager_unregister( "IAXregistry" );
-       ast_unregister_application(papp);
-       ast_cli_unregister_multiple(cli_iax2, ARRAY_LEN(cli_iax2));
-       ast_unregister_switch(&iax2_switch);
-       ast_channel_unregister(&iax2_tech);
+
        delete_users();
        iax_provision_unload();
        reload_firmware(1);