[Freeswitch-dev] Why is _thread_cond_timedwait written the way it is?
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
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@freeswitch.org http://www.freeswitchsolutions.com
FreeSWITCH-powered IP PBX: The CudaTel Communication Server http://www.cudatel.com
Official FreeSWITCH Sites http://www.freeswitch.org http://wiki.freeswitch.org http://www.cluecon.com
FreeSWITCH-dev mailing list FreeSWITCH-dev@lists.freeswitch.org http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev http://www.freeswitch.org
It seems this is more of a question for the APR developers. The basic idea for APR is to make a lib that works the same way on multiple OS'es, so I guess it might look strange in some cases. However, do you really have a problem that you think might cause issues in FS? /Peter 11 apr 2012 kl. 07:53 skrev "Richard Lewis Haggard" <haggard@msn.com<mailto:haggard@msn.com>>: 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 !DSPAM:4f8519dc32766378388994! _________________________________________________________________________ Professional FreeSWITCH Consulting Services: consulting@freeswitch.org<mailto:consulting@freeswitch.org> http://www.freeswitchsolutions.com FreeSWITCH-powered IP PBX: The CudaTel Communication Server http://www.cudatel.com Official FreeSWITCH Sites http://www.freeswitch.org http://wiki.freeswitch.org http://www.cluecon.com FreeSWITCH-dev mailing list FreeSWITCH-dev@lists.freeswitch.org<mailto:FreeSWITCH-dev@lists.freeswitch.org> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev http://www.freeswitch.org !DSPAM:4f8519dc32766378388994!
participants (3)
-
Brian West
-
Peter Olsson
-
Richard Lewis Haggard