apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <br...@xbc.nu>
Subject Re: Win32 condition variable rewrite
Date Wed, 25 Jun 2003 23:29:36 GMT
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   <brane@xbc.nu>   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)




Mime
View raw message