Ed Holyat wrote:
I like that you were able to get rid of an extra mutex.
Their is some problems though.
There is no way to safely signal the condition, a free read and/or
write on signalled and signal_all introduces multiple race
conditions. In order for this operation to be safe, it would have to
be made atomic.(not a bad idea)
But, this will never eliminate the race in checking the following
area of code. Without mutexing the if block, It is possible for
signal_all to be false, we fall into the else if.. another CPU
executes the apr_thread_cond_broadcast we will ResetEvent prematurely
if num_waiting > 0
if (cond->signal_all) { if (cond->num_waiting == 0) {
ResetEvent(cond->event); } done=1; } else if (cond->signalled) {
cond->signalled = 0; ResetEvent(cond->event); done=1; }
As I explained, we rely on the associated mutex to protect those
variables and that should be effcient.
According to the doc for apr_thread_cond_signal:
Although it is not required, if predictable scheduling is desired,
that mutex must be locked while calling this function.
Also, on *nix systems, check out the man page of pthread_cond_broadcast
says the same thing.
Thanks,
Henry
