Branko Äibej wrote: >I've been trying to use the APR condition variables on Win32, and the >dam' things kept deadlocking on me whatever I did. So, out of sheer >desperation, I took a look at the implementation... well, let's say that >I was awed by the number of bugs and race conditions I found in there. :-) > >Anyway: I went and rewrote the whole thing; now I'd like some review and >feedback before I commit this. I'm 99% sure it's foolproof, but you >never know... > > Well, I was a bit hasty posting that patch -- there is indeed a much simpler was to implement condition variables on Windows. Here it is.
-- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/ Index: include/arch/win32/apr_arch_thread_cond.h =================================================================== RCS file: /home/cvs/apr/include/arch/win32/apr_arch_thread_cond.h,v retrieving revision 1.1 diff -u -u -r1.1 apr_arch_thread_cond.h --- include/arch/win32/apr_arch_thread_cond.h 6 Jan 2003 23:44:27 -0000 1.1 +++ include/arch/win32/apr_arch_thread_cond.h 25 Jun 2003 23:25:04 -0000 @@ -59,11 +59,8 @@ struct apr_thread_cond_t { apr_pool_t *pool; - HANDLE event; - HANDLE mutex; - int signal_all; - int num_waiting; - int signalled; + HANDLE broadcast_event; + HANDLE signal_event; }; #endif /* THREAD_COND_H */ Index: locks/win32/thread_cond.c =================================================================== RCS file: /home/cvs/apr/locks/win32/thread_cond.c,v retrieving revision 1.12 diff -u -u -r1.12 thread_cond.c --- locks/win32/thread_cond.c 27 Feb 2003 19:20:24 -0000 1.12 +++ locks/win32/thread_cond.c 25 Jun 2003 23:25:04 -0000 @@ -63,8 +63,8 @@ static apr_status_t thread_cond_cleanup(void *data) { apr_thread_cond_t *cond = data; - CloseHandle(cond->mutex); - CloseHandle(cond->event); + CloseHandle(cond->broadcast_event); + CloseHandle(cond->signal_event); return APR_SUCCESS; } @@ -73,133 +73,78 @@ { *cond = apr_palloc(pool, sizeof(**cond)); (*cond)->pool = pool; - (*cond)->event = CreateEvent(NULL, TRUE, FALSE, NULL); - (*cond)->mutex = CreateMutex(NULL, FALSE, NULL); - (*cond)->signal_all = 0; - (*cond)->num_waiting = 0; + (*cond)->broadcast_event = CreateEvent(NULL, TRUE, FALSE, NULL); + (*cond)->signal_event = CreateEvent(NULL, FALSE, FALSE, NULL); return APR_SUCCESS; } -APR_DECLARE(apr_status_t) apr_thread_cond_wait(apr_thread_cond_t *cond, - apr_thread_mutex_t *mutex) +static apr_status_t cond_wait(apr_thread_cond_t *cond, + apr_thread_mutex_t *mutex, + DWORD timeout_ms) { - DWORD res; + apr_status_t rv; + HANDLE waitlist[2]; + + waitlist[0] = cond->broadcast_event; + waitlist[1] = cond->signal_event; + apr_thread_mutex_unlock(mutex); + + switch (WaitForMultipleObjects(2, waitlist, FALSE, timeout_ms)) + { + case WAIT_FAILED: + rv = apr_get_os_error(); + break; - 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); + case WAIT_TIMEOUT: + rv = APR_TIMEUP; + break; + + default: + rv = APR_SUCCESS; + break; } + apr_thread_mutex_lock(mutex); - return APR_SUCCESS; + return rv; +} + +APR_DECLARE(apr_status_t) apr_thread_cond_wait(apr_thread_cond_t *cond, + apr_thread_mutex_t *mutex) +{ + return cond_wait(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 cond_wait(cond, mutex, (DWORD) apr_time_as_msec(timeout)); } -APR_DECLARE(apr_status_t) apr_thread_cond_signal(apr_thread_cond_t *cond) + +static apr_status_t cond_signal(apr_thread_cond_t *cond, int broadcast) { - apr_status_t rv = APR_SUCCESS; - DWORD res; + BOOL result; - res = WaitForSingleObject(cond->mutex, INFINITE); - if (res != WAIT_OBJECT_0) { + if (broadcast) + result = PulseEvent(cond->broadcast_event); + else + result = PulseEvent(cond->signal_event); + + if (result) + return APR_SUCCESS; + else return apr_get_os_error(); - } - cond->signalled = 1; - res = SetEvent(cond->event); - if (res == 0) { - rv = apr_get_os_error(); - } - ReleaseMutex(cond->mutex); - return rv; } -APR_DECLARE(apr_status_t) apr_thread_cond_broadcast(apr_thread_cond_t *cond) +APR_DECLARE(apr_status_t) apr_thread_cond_signal(apr_thread_cond_t *cond) { - apr_status_t rv = APR_SUCCESS; - DWORD res; + return cond_signal(cond, 0); +} - res = WaitForSingleObject(cond->mutex, INFINITE); - if (res != WAIT_OBJECT_0) { - return apr_get_os_error(); - } - cond->signalled = 1; - cond->signal_all = 1; - res = SetEvent(cond->event); - if (res == 0) { - rv = apr_get_os_error(); - } - ReleaseMutex(cond->mutex); - return rv; +APR_DECLARE(apr_status_t) apr_thread_cond_broadcast(apr_thread_cond_t *cond) +{ + return cond_signal(cond, 1); } APR_DECLARE(apr_status_t) apr_thread_cond_destroy(apr_thread_cond_t *cond)
