apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Klaus Keppler <klaus.kepp...@informatik.stud.uni-erlangen.de>
Subject Re: locks/win32/thread_cond.c
Date Tue, 01 Jun 2004 20:57:23 GMT
Hello Ryan,

>>b. In apr_thread_cond[_timed]_wait cond->mutex and mutex are reset
>>   multiple times, why?

> I don't know.  I'll need to dig into this.

Recently I wrote a multithreaded application and tested it with
heavy load under both linux and windows.
The proposed patch (bugzilla #27654) takes care of releasing
the mutex (given by parameter) only once; without the "unlock_once"
workaround the apr_thread_cond_(timed)wait function ran into a
deadlock situation after some loops.
The MSDN docs say that you have to unlock a mutex as often as
it was locked; so I think that unlocking a mutex more often
than it was locked will result in a negative "lock count".

Having a second look at the code, cond->mutex is only released
a second time at the end of the while(1) loop, just before acquiring the
mutex againt (in WaitForSingleObject at loop start).
What's the reason for this? Ryan, do you remember?

Here's my actual (patched) version of apr_thread_cond_wait
(as a base for discussion, maybe?)

Bye,

    Klaus


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);

     int unlock_once = 1;

     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);

/* start patch */
         if (unlock_once) {
             unlock_once = 0;
             apr_thread_mutex_unlock(mutex);
         }
/* end patch */
         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); /* <- what's this for? */
     }
     apr_thread_mutex_lock(mutex);
     return APR_SUCCESS;
}



Mime
View raw message