[Freeswitch-dev] Why is _thread_cond_timedwait written the way it is?

[Brian West]

Can you clarify which files and line numbers specifically you're talking about. Many of these lines appear in many different files.  Some are win32 related some are not.  What exactly are you trying to understand?

Thanks,
Brian

On Apr 10, 2012, at 8:27 PM, Richard Lewis Haggard wrote:

> I was looking at _thread_cond_timedwait  in thread_code. There is a section of the code that I do not understand the purpose of. Could someone please explain to me what the purpose of the other part of the code is?
>  
> Before the code enters a loop it serializes access to the passed in cond structure pointer, increments a counter and makes a local copy of a value before releasing the cond again.
>  
>     EnterCriticalSection(&cond->csection);
>     cond->num_waiting++;
>     generation = cond->generation;
>     LeaveCriticalSection(&cond->csection);
>  
> What is the purpose of ‘num_waiting’ and ‘generation’?
>  
> The next part looks odd to me. Why does the code unlock the mutex when what is desired is to lock it?
>  
>     apr_thread_mutex_unlock(mutex);
>  
> The code then enters a do { } while (true); loop.
>  
>     do {
>  
> The first thing it does is to wait for access to a semaphore. This is a limited time wait and may time out before the event is encountered. The result of this wait is checked later.
>  
>         res = WaitForSingleObject(cond->semaphore, timeout_ms);
>  
> The ‘cond’ structure is going to be modified again and the code needs to have exclusive access.
>  
>         EnterCriticalSection(&cond->csection);
>  
> I do not know what the purpose of this is. Can someone explain it to me? What is the purpose of this particular block?
>  
>         if (cond->num_wake) {
>             if (cond->generation != generation) {
>                 cond->num_wake--;
>                 cond->num_waiting--;
>                 rv = APR_SUCCESS;
>                 break;
>             } else {
>                 wake = 1;
>             }
>         }
>         else if (res != WAIT_OBJECT_0) {
>             cond->num_waiting--;
>             rv = APR_TIMEUP;
>             break;
>         }
>  
> If neither ‘break’ executes then the code releases the critical section. The code would be less obtuse (and smaller) if the two ‘break’ functions were removed. That would save having duplicate LeaveCriticalSection() functions. In any case, the code releases the critical section.
>  
>         LeaveCriticalSection(&cond->csection);
>  
> What is the purpose of the next bock? It only executes if the semaphone acquisition was successful but the ‘cond->generation’ has changed while attempting to determine if it is OK to execute the mutex lock.
>  
>         if (wake) {
>             wake = 0;
>             ReleaseSemaphore(cond->semaphore, 1, NULL);
>         }
> Note: If the two ‘break’ statements were removed from above and the following lines added. At this point in the code the critical section would have been released so it would not be necessary to explicitly do so outside of the forever loop.
>         else {
>             break;
>         }
>  
> Under what conditions can ‘cond->generation’ change?
>  
> The bottom of the loop is encountered.
>  
>     } while (1);
>  
> To get here one of the two ‘break’ statements must have been hit. The critical section is still locked and must be released. Note: if my suggested changes were implemented then it would not be necessary to have this line:
>  
>     LeaveCriticalSection(&cond->csection);
>  
> And now, finally, the mutex gets locked.
>  
>     apr_thread_mutex_lock(mutex);
>  
>  
> ==============================
> Richard Lewis Haggard 
> 
>  
> _________________________________________________________________________
> Professional FreeSWITCH Consulting Services:
> consulting at freeswitch.org
> http://www.freeswitchsolutions.com
> 
> 
> 
> 
> Official FreeSWITCH Sites
> http://www.freeswitch.org
> http://wiki.freeswitch.org
> http://www.cluecon.com
> 
> FreeSWITCH-dev mailing list
> FreeSWITCH-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.freeswitch.org/pipermail/freeswitch-dev/attachments/20120411/6d774dde/attachment-0001.html