Missing WaitForSingleObject on apr_thread_cont's mutex?

[Henry Jen <he...@ztune.net>]

Hi,

I was looking at the apr_thread_cond.c, and wondering if the
apr_thread_cond_wait & apr_thread_cond_timedwait code is missing a
WaitForSingleObject call on the mutex. Would someone share some light
with me? :-) 

The following is code snippet from the SVN trunk, and it seems to be
same as in apr-0.9.6. Comments in the code.

Cheers,
-- 
Henry Jen aka slowhog
http://blogs.sun.com/slowhog


APR_DECLARE(apr_status_t) apr_thread_cond_wait(apr_thread_cond_t *cond,
                                               apr_thread_mutex_t *mutex)
{
    DWORD res;

    while (1) {
        res = WaitForSingleObject(cond->mutex, INFINITE);
        if (res != WAIT_OBJECT_0) {
            return apr_get_os_error();
        }
        cond->num_waiting++;
/* cond->mutec had been released here */
        ReleaseMutex(cond->mutex);

        apr_thread_mutex_unlock(mutex);
        res = WaitForSingleObject(cond->event, INFINITE);
/* should we add a mutex lock here to protect num_waiting value? */
/* WaitForSingleObject(cond->mutex, INFINITE); */
        cond->num_waiting--;
        if (res != WAIT_OBJECT_0) {
            apr_status_t rv = apr_get_os_error();
/* Aren't we releasing a mutex without locking it first? */
            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;
        }
/* Aren't we releasing a mutex without locking it first? */
        ReleaseMutex(cond->mutex);
    }
    apr_thread_mutex_lock(mutex);
    return APR_SUCCESS;
}

Re: Missing WaitForSingleObject on apr_thread_cont's mutex?

[Branko Čibej <br...@xbc.nu>]

Henry Jen wrote:

>Hi,
>
>I was looking at the apr_thread_cond.c, and wondering if the
>apr_thread_cond_wait & apr_thread_cond_timedwait code is missing a
>WaitForSingleObject call on the mutex. Would someone share some light
>with me? :-) 
>
>The following is code snippet from the SVN trunk, and it seems to be
>same as in apr-0.9.6. Comments in the code.
>  
>
The whole condition variable thing is broken on Windows. It has more 
race conditions than (insert faviourite expression). Adding a lock won't 
help. Someone has to rewrite the whole thing.

(I'd actually started doing that at one point, but dropped it. Lack of 
time, etc.)

-- Brane