From 20a4530c0db6d4317d8a9952edd1655e10b83844 Mon Sep 17 00:00:00 2001 From: Pekka Riikonen Date: Thu, 13 Dec 2007 17:04:27 +0000 Subject: [PATCH] Fixed deadlocks and crashes related to thread stopping. --- CHANGES.RUNTIME | 10 +++ lib/silcutil/silcthread.c | 146 +++++++++++++++++++++----------------- 2 files changed, 92 insertions(+), 64 deletions(-) diff --git a/CHANGES.RUNTIME b/CHANGES.RUNTIME index ec67f058..9d7a6ffc 100644 --- a/CHANGES.RUNTIME +++ b/CHANGES.RUNTIME @@ -1,3 +1,13 @@ +Thu Dec 13 17:37:21 EET 2007 Pekka Riikonen + + * Clear the locked flag before unlocking mutex, not after. + Affected files are lib/silcutil/unix/silcunixthread.c and + lib/silcutil/symbian/silcsymbianthread.c. + + * Fixed thread pool locking dealing with thread stopping to + prevent deadlocks and crashes. Affected file is + lib/silcutil/silcthread.c. + Tue Nov 6 17:09:42 EET 2007 Pekka Riikonen * Added '%@' formatting to silc_snprintf and variants. It diff --git a/lib/silcutil/silcthread.c b/lib/silcutil/silcthread.c index ad53a421..794259c7 100644 --- a/lib/silcutil/silcthread.c +++ b/lib/silcutil/silcthread.c @@ -120,38 +120,11 @@ static void *silc_thread_pool_run_thread(void *context) while (!t->run && !t->stop) silc_cond_wait(thread_signal, lock); - if (silc_unlikely(t->stop)) { - /* Stop the thread. Remove from threads list. */ - SILC_LOG_DEBUG(("Stop thread %p", t)); - silc_mutex_lock(tp->lock); - silc_list_del(tp->threads, t); - silc_list_start(tp->threads); - - /* Clear thread's call queue. */ - silc_list_start(t->queue); - silc_list_start(t->free_queue); - while ((q = silc_list_get(t->queue))) - silc_sfree(tp->stack, q); - while ((q = silc_list_get(t->free_queue))) - silc_sfree(tp->stack, q); - - /* Destroy the thread */ - silc_mutex_unlock(lock); - silc_mutex_free(lock); - silc_cond_free(thread_signal); - silc_sfree(tp->stack, t); - - /* If we are last thread, signal the waiting destructor. */ - if (silc_list_count(tp->threads) == 0) - silc_cond_signal(tp->pool_signal); - - /* Release pool reference. Releases lock also. */ - silc_thread_pool_unref(tp); - break; - } - silc_mutex_unlock(lock); + if (t->stop) + goto stop; /* Execute code */ + silc_mutex_unlock(lock); execute: SILC_LOG_DEBUG(("Execute call %p, context %p, thread %p", t->run, t->run_context, t)); @@ -175,9 +148,12 @@ static void *silc_thread_pool_run_thread(void *context) } } + silc_mutex_lock(lock); + if (t->stop) + goto stop; + /* Check if there are calls in queue. Takes the most recently added call since new ones are added at the start of the list. */ - silc_mutex_lock(lock); if (silc_list_count(t->queue) > 0) { execute_queue: silc_list_start(t->queue); @@ -196,56 +172,98 @@ static void *silc_thread_pool_run_thread(void *context) silc_list_add(t->free_queue, q); silc_mutex_unlock(lock); goto execute; - } else { - silc_mutex_unlock(lock); + } - /* Nothing to do. Attempt to steal call from some other thread. */ - silc_mutex_lock(tp->lock); + silc_mutex_unlock(lock); + silc_mutex_lock(tp->lock); + + /* Nothing to do. Attempt to steal call from some other thread. */ + o = silc_list_get(tp->threads); + if (!o) { + /* List wraps around */ + silc_list_start(tp->threads); o = silc_list_get(tp->threads); - if (!o) { - /* List wraps around */ - silc_list_start(tp->threads); - o = silc_list_get(tp->threads); - } + } + + /* Check that the other thread is valid and has something to execute. */ + silc_mutex_lock(o->lock); + if (o == t || o->stop || silc_list_count(o->queue) == 0) { + silc_mutex_unlock(o->lock); + o = NULL; + } + + if (o) { silc_mutex_unlock(tp->lock); + silc_list_start(o->queue); + q = silc_list_get(o->queue); - if (o && o != t) { - silc_mutex_lock(o->lock); - if (silc_list_count(o->queue) > 0) { - silc_list_start(o->queue); - q = silc_list_get(o->queue); - - SILC_LOG_DEBUG(("Execute call from queue from thread %p", o)); - - /* Execute this call now */ - t->run = q->run; - t->run_context = q->run_context; - t->completion = q->completion; - t->completion_context = q->completion_context; - t->schedule = q->schedule; - - silc_list_del(o->queue, q); - silc_list_add(o->free_queue, q); - silc_mutex_unlock(o->lock); - goto execute; - } - silc_mutex_unlock(o->lock); - } + SILC_LOG_DEBUG(("Execute call from queue from thread %p", o)); + + /* Execute this call now */ + t->run = q->run; + t->run_context = q->run_context; + t->completion = q->completion; + t->completion_context = q->completion_context; + t->schedule = q->schedule; + + silc_list_del(o->queue, q); + silc_list_add(o->free_queue, q); + silc_mutex_unlock(o->lock); + goto execute; } silc_mutex_lock(lock); + if (t->stop) { + silc_mutex_unlock(tp->lock); + goto stop; + } /* Now that we have the lock back, check the queue again. */ - if (silc_list_count(t->queue) > 0) + if (silc_list_count(t->queue) > 0) { + silc_mutex_unlock(tp->lock); goto execute_queue; + } /* The thread is now free for use again. */ t->run = NULL; t->completion = NULL; t->schedule = NULL; silc_list_add(tp->free_threads, t); + silc_mutex_unlock(tp->lock); } + stop: + /* Stop the thread. Remove from threads list. */ + SILC_LOG_DEBUG(("Stop thread %p", t)); + + /* We can unlock the thread now. After we get the thread pool lock + no one can retrieve the thread anymore. */ + silc_mutex_unlock(lock); + silc_mutex_lock(tp->lock); + + silc_list_del(tp->threads, t); + silc_list_start(tp->threads); + + /* Clear thread's call queue. */ + silc_list_start(t->queue); + silc_list_start(t->free_queue); + while ((q = silc_list_get(t->queue))) + silc_sfree(tp->stack, q); + while ((q = silc_list_get(t->free_queue))) + silc_sfree(tp->stack, q); + + /* Destroy the thread */ + silc_mutex_free(lock); + silc_cond_free(thread_signal); + silc_sfree(tp->stack, t); + + /* If we are last thread, signal the waiting destructor. */ + if (silc_list_count(tp->threads) == 0) + silc_cond_signal(tp->pool_signal); + + /* Release pool reference. Releases lock also. */ + silc_thread_pool_unref(tp); + return NULL; } @@ -397,7 +415,7 @@ SilcBool silc_thread_pool_run(SilcThreadPool tp, /* Get free thread */ silc_list_start(tp->free_threads); t = silc_list_get(tp->free_threads); - if (!t) { + if (!t || t->stop) { if (silc_list_count(tp->threads) + 1 > tp->max_threads) { /* Maximum threads reached */ if (!queuable) { -- 2.24.0