Here is a patch for win32 that has been tested extensively for a few months now. I submitted it to bugzilla
The previous patch addressed only the unlock being called more than once.
This attachment avoids race conditions that the previous patch doesn't. This patch also fixes the multiple calls to unlock. This patch also consolidates the the duplicate efforts in apr_thread_cond_wait and apr_thread_cond_timedwait
Cliff Woolley <[EMAIL PROTECTED]> wrote:
Cliff Woolley <[EMAIL PROTECTED]> wrote:
On Wed, 20 Jul 2005, Paul Querna wrote:
> > -1 for Win32, the condvars deadlock is a serious bug. I knew this is not
> > news, but as the patch had been available for quite a while, is it
> > possible to get it fixed?
>
> No.
>
> I will not commit such a platform specific patch. Anyone who actually
> compiles APR on win32 that wants to do it?
I'll set up a build on my windows box at work this afternoon and check it
out if OtherBill doesn't beat me to it.
--Cliff
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Index: locks/win32/thread_cond.c
===================================================================
RCS file: /home/cvspublic/apr/locks/win32/thread_cond.c,v
retrieving revision 1.13
diff -u -r1.13 thread_cond.c
--- locks/win32/thread_cond.c 13 Feb 2004 09:38:32 -0000 1.13
+++ locks/win32/thread_cond.c 20 Jul 2005 18:35:15 -0000
@@ -38,92 +38,88 @@
(*cond)->mutex = CreateMutex(NULL, FALSE, NULL);
(*cond)->signal_all = 0;
(*cond)->num_waiting = 0;
+ (*cond)->signalled = 0;
return APR_SUCCESS;
}
-APR_DECLARE(apr_status_t) apr_thread_cond_wait(apr_thread_cond_t *cond,
- apr_thread_mutex_t *mutex)
+static APR_INLINE apr_status_t _thread_cond_timedwait(apr_thread_cond_t *cond,
+ apr_thread_mutex_t *mutex,
+ DWORD timeout_ms )
{
DWORD res;
-
- while (1) {
- res = WaitForSingleObject(cond->mutex, INFINITE);
- if (res != WAIT_OBJECT_0) {
- return apr_get_os_error();
- }
- cond->num_waiting++;
- ReleaseMutex(cond->mutex);
-
- apr_thread_mutex_unlock(mutex);
- res = WaitForSingleObject(cond->event, INFINITE);
- cond->num_waiting--;
- if (res != WAIT_OBJECT_0) {
- apr_status_t rv = apr_get_os_error();
- ReleaseMutex(cond->mutex);
- return rv;
- }
- if (cond->signal_all) {
- if (cond->num_waiting == 0) {
- ResetEvent(cond->event);
- }
- break;
- }
- if (cond->signalled) {
- cond->signalled = 0;
- ResetEvent(cond->event);
- break;
- }
- ReleaseMutex(cond->mutex);
+ int done=0;
+ HANDLE wait_events[2];
+ apr_status_t rv;
+
+ while (!done) {
+ res = WaitForSingleObject(cond->mutex, timeout_ms);
+ if (res == WAIT_FAILED) {
+ return apr_get_os_error();
+ }
+ if (res == WAIT_TIMEOUT) {
+ return APR_TIMEUP;
+ }
+ cond->num_waiting++;
+ wait_events[0]=cond->event;
+ wait_events[1]=cond->mutex;
+ ReleaseMutex(cond->mutex);
+
+ apr_thread_mutex_unlock(mutex);
+ res = WaitForMultipleObjects( 2, wait_events, TRUE, timeout_ms);
+ switch(res) {
+ case WAIT_FAILED:
+ rv = apr_get_os_error();
+ res = WaitForSingleObject(cond->mutex, timeout_ms);
+ cond->num_waiting--;
+ if (res != WAIT_FAILED) {
+ ReleaseMutex(cond->mutex);
+ }
+ apr_thread_mutex_lock(mutex);
+ return rv;
+ break;
+ case WAIT_TIMEOUT:
+ res = WaitForSingleObject(cond->mutex, timeout_ms);
+ cond->num_waiting--;
+ if (res != WAIT_FAILED) {
+ ReleaseMutex(cond->mutex);
+ }
+ apr_thread_mutex_lock(mutex);
+ return APR_TIMEUP;
+ break;
+ default:
+ break;
+ }
+ cond->num_waiting--;/*we have the lock(s)*/
+ if (cond->signal_all) {
+ if (cond->num_waiting == 0) {
+ ResetEvent(cond->event);
+ }
+ done=1;
+ }
+ else if (cond->signalled) {
+ cond->signalled = 0;
+ ResetEvent(cond->event);
+ done=1;
+ }
+ ReleaseMutex(cond->mutex);
+ apr_thread_mutex_lock(mutex);
}
- apr_thread_mutex_lock(mutex);
return APR_SUCCESS;
}
+APR_DECLARE(apr_status_t) apr_thread_cond_wait(apr_thread_cond_t *cond,
+ apr_thread_mutex_t *mutex)
+{
+ return _thread_cond_timedwait(cond, mutex, INFINITE);
+}
+
APR_DECLARE(apr_status_t) apr_thread_cond_timedwait(apr_thread_cond_t *cond,
apr_thread_mutex_t *mutex,
apr_interval_time_t
timeout)
-{
- DWORD res;
+{
DWORD timeout_ms = (DWORD) apr_time_as_msec(timeout);
- while (1) {
- res = WaitForSingleObject(cond->mutex, timeout_ms);
- if (res != WAIT_OBJECT_0) {
- if (res == WAIT_TIMEOUT) {
- return APR_TIMEUP;
- }
- return apr_get_os_error();
- }
- cond->num_waiting++;
- ReleaseMutex(cond->mutex);
-
- apr_thread_mutex_unlock(mutex);
- res = WaitForSingleObject(cond->event, timeout_ms);
- cond->num_waiting--;
- if (res != WAIT_OBJECT_0) {
- apr_status_t rv = apr_get_os_error();
- ReleaseMutex(cond->mutex);
- apr_thread_mutex_lock(mutex);
- if (res == WAIT_TIMEOUT) {
- return APR_TIMEUP;
- }
- return apr_get_os_error();
- }
- if (cond->signal_all) {
- if (cond->num_waiting == 0) {
- ResetEvent(cond->event);
- }
- break;
- }
- if (cond->signalled) {
- cond->signalled = 0;
- ResetEvent(cond->event);
- break;
- }
- ReleaseMutex(cond->mutex);
- }
- apr_thread_mutex_lock(mutex);
- return APR_SUCCESS;
+ return _thread_cond_timedwait(cond, mutex, timeout_ms);
}
APR_DECLARE(apr_status_t) apr_thread_cond_signal(apr_thread_cond_t *cond)
