apr_thread_cond_wait win32 implementation

[Kevin Seguin <se...@motive.com>]

hi.  i just happened to be looking through the win32 implementation of
apr_thread_cond_wait, and it seems to me there could be some issues.  now,
i'm not really an expert in this area, so i could be totally off, but i
figured it might be worthwhile to share my thoughts.  at the very least, i
might learn something :)

anyway, below is the implementation with my comments sprinkled throughout:


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

    while (1) {
        WaitForSingleObject(cond->mutex, INFINITE);
        cond->num_waiting++;
        ReleaseMutex(cond->mutex);

        apr_thread_mutex_unlock(mutex);
        rv = WaitForSingleObject(cond->event, INFINITE);
        /* shouldn't cond->mutex be acquired here with:
         *   WaitForSingleObject(cond->mutex, INFINITE);
         * before decrementing cond->num_waiting and
         * examining it later (when cond->signal_all is 1)??
         */
        cond->num_waiting--;
        if (rv == WAIT_FAILED) {
            /* if cond->mutex is acquired above, it should
             * be released here.
             */
            return apr_get_os_error();
        }
        if (cond->signal_all) {
            if (cond->num_waiting == 0) {
                /* shouldn't cond->signal_all be reset here with:
                 *   cond->signal_all = 0;
                 * ??  otherwise, a call to apr_thread_cond_broadcast()
                 * will result in subsequent calls to
apr_thread_cond_signal()
                 * appearing to be calls to apr_thread_cond_broadcast().  i
                 * suppose it's an unlikely scenario where for a given
condition,
                 * you'd use broadcast sometimes and signal other times.
not
                 * totally unthinkable though.
                 */
                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;
}