You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2021/04/26 21:14:17 UTC
svn commit: r1889217 - /apr/apr/trunk/util-misc/apr_thread_pool.c
Author: ylavic
Date: Mon Apr 26 21:14:17 2021
New Revision: 1889217
URL: http://svn.apache.org/viewvc?rev=1889217&view=rev
Log:
Follow up to r1884109: Fix synchronization in thread_pool_cleanup().
Ask each thread to signal its task completion individually when being waited in
wait_on_busy_threads(), and ignore threads that have already finished their
task (elt->current_owner == NULL).
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=1889217&r1=1889216&r2=1889217&view=diff
==============================================================================
--- apr/apr/trunk/util-misc/apr_thread_pool.c (original)
+++ apr/apr/trunk/util-misc/apr_thread_pool.c Mon Apr 26 21:14:17 2021
@@ -46,8 +46,9 @@ struct apr_thread_list_elt
{
APR_RING_ENTRY(apr_thread_list_elt) link;
apr_thread_t *thd;
- volatile void *current_owner;
- volatile enum { TH_RUN, TH_STOP, TH_PROBATION } state;
+ void *current_owner;
+ enum { TH_RUN, TH_STOP, TH_PROBATION } state;
+ int signal_work_done;
};
APR_RING_HEAD(apr_thread_list, apr_thread_list_elt);
@@ -78,7 +79,6 @@ struct apr_thread_pool
apr_thread_cond_t *all_done;
apr_thread_mutex_t *lock;
volatile int terminated;
- int waiting_work_done;
struct apr_thread_pool_tasks *recycled_tasks;
struct apr_thread_list *recycled_thds;
apr_thread_pool_task_t *task_idx[TASK_PRIORITY_SEGS];
@@ -254,6 +254,7 @@ static struct apr_thread_list_elt *elt_n
APR_RING_ELEM_INIT(elt, link);
elt->thd = t;
elt->current_owner = NULL;
+ elt->signal_work_done = 0;
elt->state = TH_RUN;
return elt;
}
@@ -313,11 +314,9 @@ static void *APR_THREAD_FUNC thread_pool
APR_RING_INSERT_TAIL(me->recycled_tasks, task,
apr_thread_pool_task, link);
elt->current_owner = NULL;
- if (me->waiting_work_done) {
+ if (elt->signal_work_done) {
+ elt->signal_work_done = 0;
apr_thread_cond_signal(me->work_done);
- apr_thread_mutex_unlock(me->lock);
- apr_thread_mutex_lock(me->lock);
- apr_pool_owner_set(me->pool, 0);
}
} while (elt->state != TH_STOP);
APR_RING_REMOVE(elt, link);
@@ -433,25 +432,23 @@ APR_DECLARE(apr_status_t) apr_thread_poo
return rv;
apr_pool_pre_cleanup_register(tp->pool, tp, thread_pool_cleanup);
- while (init_threads) {
- /* Grab the mutex as apr_thread_create() and thread_pool_func() will
- * allocate from (*me)->pool. This is dangerous if there are multiple
- * initial threads to create.
- */
- apr_thread_mutex_lock(tp->lock);
- apr_pool_owner_set(tp->pool, 0);
+ /* Grab the mutex as apr_thread_create() and thread_pool_func() will
+ * allocate from (*me)->pool. This is dangerous if there are multiple
+ * initial threads to create.
+ */
+ apr_thread_mutex_lock(tp->lock);
+ apr_pool_owner_set(tp->pool, 0);
+ while (init_threads--) {
rv = apr_thread_create(&t, NULL, thread_pool_func, tp, tp->pool);
if (APR_SUCCESS != rv) {
- apr_thread_mutex_unlock(tp->lock);
break;
}
tp->thd_cnt++;
if (tp->thd_cnt > tp->thd_high) {
tp->thd_high = tp->thd_cnt;
}
- apr_thread_mutex_unlock(tp->lock);
- --init_threads;
}
+ apr_thread_mutex_unlock(tp->lock);
if (rv == APR_SUCCESS) {
*me = tp;
@@ -758,7 +755,7 @@ static void wait_on_busy_threads(apr_thr
elt = APR_RING_FIRST(me->busy_thds);
while (elt != APR_RING_SENTINEL(me->busy_thds, apr_thread_list_elt, link)) {
- if (owner && elt->current_owner != owner) {
+ if (owner ? owner != elt->current_owner : !elt->current_owner) {
elt = APR_RING_NEXT(elt, link);
continue;
}
@@ -774,10 +771,9 @@ static void wait_on_busy_threads(apr_thr
#endif
#endif
- me->waiting_work_done = 1;
+ elt->signal_work_done = 1;
apr_thread_cond_wait(me->work_done, me->lock);
apr_pool_owner_set(me->pool, 0);
- me->waiting_work_done = 0;
/* Restart */
elt = APR_RING_FIRST(me->busy_thds);