You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by aa...@apache.org on 2002/04/28 23:35:13 UTC
cvs commit: httpd-2.0/server/mpm/experimental/threadpool threadpool.c
aaron 02/04/28 14:35:13
Modified: server/mpm/experimental/threadpool threadpool.c
Log:
When we signal a condition variable, we need to own the lock that
is associated with that condition variable. This isn't necessary
for Solaris, but for Posix it is.
Revision Changes Path
1.2 +10 -3 httpd-2.0/server/mpm/experimental/threadpool/threadpool.c
Index: threadpool.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/experimental/threadpool/threadpool.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- threadpool.c 16 Apr 2002 23:37:06 -0000 1.1
+++ threadpool.c 28 Apr 2002 21:35:12 -0000 1.2
@@ -939,10 +939,13 @@
signal_threads(ST_GRACEFUL);
}
if (csd != NULL) {
- worker->csd = (apr_socket_t *)csd;
+ /* Wake up the sleeping worker. */
apr_thread_mutex_lock(worker->mutex);
- apr_thread_mutex_unlock(worker->mutex);
+ worker->csd = (apr_socket_t *)csd;
+ /* We must own the lock associated with the condition
+ * variable that we are signaling. */
apr_thread_cond_signal(worker->cond);
+ apr_thread_mutex_unlock(worker->mutex);
worker = NULL;
}
}
@@ -962,8 +965,10 @@
worker_stack_interrupt_all(idle_worker_stack);
if (worker) {
apr_thread_mutex_lock(worker->mutex);
- apr_thread_mutex_unlock(worker->mutex);
+ /* We must own the lock associated with the condition
+ * variable that we are signaling. */
apr_thread_cond_signal(worker->cond);
+ apr_thread_mutex_unlock(worker->mutex);
}
dying = 1;
ap_scoreboard_image->parent[process_slot].quiescing = 1;
@@ -1003,6 +1008,8 @@
apr_pool_create_ex(&ptrans, NULL, NULL, allocator);
apr_allocator_set_owner(allocator, ptrans);
+ /* XXX: What happens if this is allocated from the
+ * single-thread-optimized ptrans pool? -aaron */
bucket_alloc = apr_bucket_alloc_create(tpool);
wakeup = (worker_wakeup_info *)apr_palloc(tpool, sizeof(*wakeup));
Re: cvs commit: httpd-2.0/server/mpm/experimental/threadpool
threadpool.c
Posted by Brian Pane <bp...@pacbell.net>.
Aaron Bannert wrote:
>On Sun, Apr 28, 2002 at 02:52:39PM -0700, Brian Pane wrote:
>
>>aaron@apache.org wrote:
>>
>>>aaron 02/04/28 14:35:13
>>>
>>>Modified: server/mpm/experimental/threadpool threadpool.c
>>>Log:
>>>When we signal a condition variable, we need to own the lock that
>>>is associated with that condition variable. This isn't necessary
>>>for Solaris, but for Posix it is.
>>>
>>Is it really required for Posix compliance, or just recommended
>>as a standard idiom for guaranteeing that the target thread is
>>actually waiting on the condition variable? I thought it was
>>the latter; if so, in threadpool and leader/follower the
>>lock/unlock/signal idiom should be safe.
>>
>
>Actually, it isn't required for Posix, but that's not the whole story.
>Here's what Stevens says on p. 170-171 of UNIX Network Programming:
>Interprocess Communication, Vol 2, 2nd Ed:
>
> Here we do not signal the condition variable until we release the
> mutex. This is explicitly allowed by Posix: the thread calling
> pthread_cond_signal need not be the current owner of the mutex
> associated with the condition variable. But Posix goes on to say
> that if predictable scheduling behavior is required, then the mutex
> must be locked by the thread calling pthread_cond_signal.
>
>So you are correct, but I think we should be safe here, especially
>considering that we required the lock followed by an immediate unlock
>to get proper behavior, right? I'll alter the comment to be correct,
>and include a reference to Stevens in there too.
>
Sounds good to me.
Thanks,
--Brian
Re: cvs commit: httpd-2.0/server/mpm/experimental/threadpool threadpool.c
Posted by Aaron Bannert <aa...@clove.org>.
On Sun, Apr 28, 2002 at 02:52:39PM -0700, Brian Pane wrote:
> aaron@apache.org wrote:
>
> >aaron 02/04/28 14:35:13
> >
> > Modified: server/mpm/experimental/threadpool threadpool.c
> > Log:
> > When we signal a condition variable, we need to own the lock that
> > is associated with that condition variable. This isn't necessary
> > for Solaris, but for Posix it is.
> >
>
> Is it really required for Posix compliance, or just recommended
> as a standard idiom for guaranteeing that the target thread is
> actually waiting on the condition variable? I thought it was
> the latter; if so, in threadpool and leader/follower the
> lock/unlock/signal idiom should be safe.
Actually, it isn't required for Posix, but that's not the whole story.
Here's what Stevens says on p. 170-171 of UNIX Network Programming:
Interprocess Communication, Vol 2, 2nd Ed:
Here we do not signal the condition variable until we release the
mutex. This is explicitly allowed by Posix: the thread calling
pthread_cond_signal need not be the current owner of the mutex
associated with the condition variable. But Posix goes on to say
that if predictable scheduling behavior is required, then the mutex
must be locked by the thread calling pthread_cond_signal.
So you are correct, but I think we should be safe here, especially
considering that we required the lock followed by an immediate unlock
to get proper behavior, right? I'll alter the comment to be correct,
and include a reference to Stevens in there too.
-aaron
Re: cvs commit: httpd-2.0/server/mpm/experimental/threadpool
threadpool.c
Posted by Brian Pane <bp...@pacbell.net>.
aaron@apache.org wrote:
>aaron 02/04/28 14:35:13
>
> Modified: server/mpm/experimental/threadpool threadpool.c
> Log:
> When we signal a condition variable, we need to own the lock that
> is associated with that condition variable. This isn't necessary
> for Solaris, but for Posix it is.
>
Is it really required for Posix compliance, or just recommended
as a standard idiom for guaranteeing that the target thread is
actually waiting on the condition variable? I thought it was
the latter; if so, in threadpool and leader/follower the
lock/unlock/signal idiom should be safe.
--Brian