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 -
1.1
+++ include/arch/win32/apr_arch_thread_cond.h 25 Jun 2003 23:25:04 -
@@ -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 - 1.12
+++ locks/win32/thread_cond.c 25 Jun 2003 23:25:04 -
@@ -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);
-
-a
