private:
void sys_issue(LONG CONST request) {
if (request) {
WaitForSingleObject(m_wmtx);
if (m_issued) {
InterlockedExchangeAdd(&m_request, request);
} else {
m_issued = request;
ResetEvent(m_wgate);
ReleaseSemaphore(m_wset, request, 0);
}
SetEvent(m_wmtx);
}
}
public:
void broadcast() {
sys_issue(InterlockedExchange(&m_request, 0));
}
void signal() {
LONG temp, cmp = m_request;
do {
temp = cmp;
cmp = InterlockedCompareExchange(
&m_request,
(cmp) ? cmp - 1 : 0,
cmp);
} while(cmp == temp);
if (cmp) {
sys_issue(1);
}
}
void wait(mutex_win& umtx) {
WaitForSingleObject(m_wmtx);
InterlockedIncrement(&m_request);
umtx.unlock();
SignalObjectAndWait(m_wmtx, m_wgate);
WaitForSingleObject(m_wset);
umtx.lock();
WaitForSingleObject(m_wmtx);
--m_issued;
if (! m_issued) {
SetEvent(m_wgate);
}
SetEvent(m_wmtx);
}
};
___________________________________________________________
Can you notice anything bad here? I would appreciate any comments before I
create a compliable prototype.
Thank you.
--
Chris M. Thomasson
http://appcore.home.comcast.net
I think I spotted a race-condition that can cause deadlock. Here is a little
contest, can you find the culprit??? I am going to fix the condition and
repost the code, but I will give it a day or two.
Humm... Now that I examine it some more, I am not so sure what I found is a
problem...
add new variable:
LONG m_pending; // pending requests; set to 0
> HANDLE m_wmtx; // auto-reset; set to true
> HANDLE m_wgate; // manual-reset; set to true
> HANDLE m_wset; // semaphore; set to 0
>
>
> private:
> void sys_issue(LONG CONST request) {
> if (request) {
> WaitForSingleObject(m_wmtx);
> if (m_issued) {
I am thinking that instead of adding the request back into m_request during
a pending broadcast/signal:
> InterlockedExchangeAdd(&m_request, request);
I will just add it to a running total:
m_pending += request;
> } else {
> m_issued = request;
And add it to m_issued here:
m_issued = request + m_pending;
m_pending = 0;
> ResetEvent(m_wgate);
> ReleaseSemaphore(m_wset, request, 0);
> }
> SetEvent(m_wmtx);
> }
> }
[...]
> void wait(mutex_win& umtx) {
> WaitForSingleObject(m_wmtx);
> InterlockedIncrement(&m_request);
> umtx.unlock();
> SignalObjectAndWait(m_wmtx, m_wgate);
> WaitForSingleObject(m_wset);
> umtx.lock();
> WaitForSingleObject(m_wmtx);
> --m_issued;
> if (! m_issued) {
And only add it back to m_request at the end of a broadcast/signal here:
if (m_pending) {
InterlockedExchangeAdd(&m_request, m_pending);
m_pending = 0;
}
> SetEvent(m_wgate);
> }
> SetEvent(m_wmtx);
>
}
Here is the pseudo-code in full:
___________________________________________________________
class condvar_win {
LONG m_request; // signal requests; set to 0
LONG m_issued; // signals issued; set to 0
LONG m_pending; // signals pending; set to 0
HANDLE m_wmtx; // auto-reset; set to true
HANDLE m_wgate; // manual-reset; set to true
HANDLE m_wset; // semaphore; set to 0
private:
void sys_issue(LONG CONST request) {
if (request) {
WaitForSingleObject(m_wmtx);
if (m_issued) {
m_pending += request;
} else {
m_issued = request + m_pending;
m_pending = 0;
if (m_pending) {
InterlockedExchangeAdd(&m_request, m_pending);
m_pending = 0;
}
SetEvent(m_wgate);
}
SetEvent(m_wmtx);
}
};
___________________________________________________________
What do you all think? Unless I am missing something, SignalObjectAndWait
should keep the wait on the m_wgate variable atomic wrt the ResetEvent and
SetEvent calls within the critical-section, -and- the unlock/lock operations
themselves. Since I am using SignalObjectAndWait to atomically unlock the
mutex and wait on the gate, it should ensure that another thread can sneak
in and lock the mutex in the middle of the unlock and subsequent wait.
OOPS! I should not add the m_pending to m_issued! That would lose all those
waiters!
void sys_issue(LONG CONST request) {
if (request) {
WaitForSingleObject(m_wmtx);
if (m_issued) {
m_pending += request;
} else {
m_issued = request + m_pending;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
That is BAD. It has to be:
m_issued = request
ResetEvent(m_wgate);
ReleaseSemaphore(m_wset, request, 0);
}
SetEvent(m_wmtx);
}
}
Here is corrected code sketch:
class condvar_win {
LONG m_request; // signal requests; set to 0
LONG m_issued; // signals issued; set to 0
LONG m_pending; // signals pending; set to 0
HANDLE m_wmtx; // auto-reset; set to true
HANDLE m_wgate; // manual-reset; set to true
HANDLE m_wset; // semaphore; set to 0
private:
void sys_issue(LONG CONST request) {
if (request) {
WaitForSingleObject(m_wmtx);
if (m_issued) {
m_pending += request;
} else {
assert(! m_pending);
if (m_pending) {
InterlockedExchangeAdd(&m_request, m_pending);
m_pending = 0;
}
SetEvent(m_wgate);
}
SetEvent(m_wmtx);
}
};
> ___________________________________________________________
[...]
I am sorry for any confusions!
:^(...
Well, I think I will just code this thing up for real, and post it here.
That way, everybody here can test it if they are interested in it.
it should ensure that another thread -CANNOT- sneak
} while(cmp != temp);
of course!!!
> if (cmp) {
> sys_issue(1);
> }
> }