You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by tr...@apache.org on 2010/09/28 12:37:29 UTC
svn commit: r1002105 - /apr/apr/trunk/util-misc/apr_thread_pool.c
Author: trawick
Date: Tue Sep 28 10:37:28 2010
New Revision: 1002105
URL: http://svn.apache.org/viewvc?rev=1002105&view=rev
Log:
apr_thread_pool: Fix some potential deadlock situations.
The use of two mutexes allowed race conditions between
releasing one and acquiring another.
PR: 49709
Submitted by: Joe Mudd <Joe.Mudd sas.com>
Reviewed by: henryjen, trawick
Modified:
apr/apr/trunk/util-misc/apr_thread_pool.c
Modified: apr/apr/trunk/util-misc/apr_thread_pool.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apr_thread_pool.c?rev=1002105&r1=1002104&r2=1002105&view=diff
==============================================================================
--- apr/apr/trunk/util-misc/apr_thread_pool.c (original)
+++ apr/apr/trunk/util-misc/apr_thread_pool.c Tue Sep 28 10:37:28 2010
@@ -72,7 +72,6 @@ struct apr_thread_pool
struct apr_thread_list *busy_thds;
struct apr_thread_list *idle_thds;
apr_thread_mutex_t *lock;
- apr_thread_mutex_t *cond_lock;
apr_thread_cond_t *cond;
volatile int terminated;
struct apr_thread_pool_tasks *recycled_tasks;
@@ -95,16 +94,9 @@ static apr_status_t thread_pool_construc
if (APR_SUCCESS != rv) {
return rv;
}
- rv = apr_thread_mutex_create(&me->cond_lock, APR_THREAD_MUTEX_UNNESTED,
- me->pool);
- if (APR_SUCCESS != rv) {
- apr_thread_mutex_destroy(me->lock);
- return rv;
- }
rv = apr_thread_cond_create(&me->cond, me->pool);
if (APR_SUCCESS != rv) {
apr_thread_mutex_destroy(me->lock);
- apr_thread_mutex_destroy(me->cond_lock);
return rv;
}
me->tasks = apr_palloc(me->pool, sizeof(*me->tasks));
@@ -148,7 +140,6 @@ static apr_status_t thread_pool_construc
CATCH_ENOMEM:
rv = APR_ENOMEM;
apr_thread_mutex_destroy(me->lock);
- apr_thread_mutex_destroy(me->cond_lock);
apr_thread_cond_destroy(me->cond);
FINAL_EXIT:
return rv;
@@ -321,16 +312,12 @@ static void *APR_THREAD_FUNC thread_pool
else
wait = -1;
- apr_thread_mutex_unlock(me->lock);
- apr_thread_mutex_lock(me->cond_lock);
if (wait >= 0) {
- rv = apr_thread_cond_timedwait(me->cond, me->cond_lock, wait);
+ rv = apr_thread_cond_timedwait(me->cond, me->lock, wait);
}
else {
- rv = apr_thread_cond_wait(me->cond, me->cond_lock);
+ rv = apr_thread_cond_wait(me->cond, me->lock);
}
- apr_thread_mutex_unlock(me->cond_lock);
- apr_thread_mutex_lock(me->lock);
}
/* idle thread been asked to stop, will be joined */
@@ -350,7 +337,6 @@ static apr_status_t thread_pool_cleanup(
apr_sleep(20 * 1000); /* spin lock with 20 ms */
}
apr_thread_mutex_destroy(_myself->lock);
- apr_thread_mutex_destroy(_myself->cond_lock);
apr_thread_cond_destroy(_myself->cond);
return APR_SUCCESS;
}
@@ -530,10 +516,8 @@ static apr_status_t schedule_task(apr_th
me->thd_high = me->thd_cnt;
}
}
- apr_thread_mutex_unlock(me->lock);
- apr_thread_mutex_lock(me->cond_lock);
apr_thread_cond_signal(me->cond);
- apr_thread_mutex_unlock(me->cond_lock);
+ apr_thread_mutex_unlock(me->lock);
return rv;
}
@@ -585,11 +569,9 @@ static apr_status_t add_task(apr_thread_
me->thd_high = me->thd_cnt;
}
}
- apr_thread_mutex_unlock(me->lock);
- apr_thread_mutex_lock(me->cond_lock);
apr_thread_cond_signal(me->cond);
- apr_thread_mutex_unlock(me->cond_lock);
+ apr_thread_mutex_unlock(me->lock);
return rv;
}
@@ -847,9 +829,9 @@ static apr_size_t trim_idle_threads(apr_
elt = trim_threads(me, &cnt, 1);
- apr_thread_mutex_lock(me->cond_lock);
+ apr_thread_mutex_lock(me->lock);
apr_thread_cond_broadcast(me->cond);
- apr_thread_mutex_unlock(me->cond_lock);
+ apr_thread_mutex_unlock(me->lock);
n_dbg = 0;
if (NULL != (head = elt)) {