Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Experimental Condition-Variable for Windows...

6 views
Skip to first unread message

Chris Thomasson

unread,
Dec 7, 2007, 8:30:25 PM12/7/07
to
Here is the crude code sketch:
___________________________________________________________
class condvar_win {
LONG m_request; // signal requests; set to 0
LONG m_issued; // signals issued; 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) {
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

Chris Thomasson

unread,
Dec 7, 2007, 8:43:56 PM12/7/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:N5udnYakYceVb8Ta...@comcast.com...

> Here is the crude code sketch:
> ___________________________________________________________
[...]

> ___________________________________________________________
>
>
> Can you notice anything bad here?

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.

Chris Thomasson

unread,
Dec 8, 2007, 1:25:25 AM12/8/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:PtWdnXsrsbetaMTa...@comcast.com...

> "Chris Thomasson" <cri...@comcast.net> wrote in message
> news:N5udnYakYceVb8Ta...@comcast.com...
>> Here is the crude code sketch:
>> ___________________________________________________________
> [...]
>> ___________________________________________________________
>>
>>
>> Can you notice anything bad here?
>
> I think I spotted a race-condition that can cause deadlock. Here is a
> little contest, can you find the culprit???

Humm... Now that I examine it some more, I am not so sure what I found is a
problem...

Chris Thomasson

unread,
Dec 8, 2007, 9:21:27 PM12/8/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:N5udnYakYceVb8Ta...@comcast.com...
> Here is the crude code sketch:
> ___________________________________________________________
> class condvar_win {
> LONG m_request; // signal requests; set to 0
> LONG m_issued; // signals issued; set to 0

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.

Chris Thomasson

unread,
Dec 8, 2007, 10:43:46 PM12/8/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:N5udnYakYceVb8Ta...@comcast.com...
> Here is the crude code sketch:
> ___________________________________________________________
[...]

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.

Chris Thomasson

unread,
Dec 9, 2007, 3:29:19 AM12/9/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:gaidnUazrZTq0sba...@comcast.com...
[...]

> 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
^^^^^^

it should ensure that another thread -CANNOT- sneak

Chris Thomasson

unread,
Dec 10, 2007, 5:07:41 PM12/10/07
to
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:u9OdnaiBxOlf_8ba...@comcast.com...

> "Chris Thomasson" <cri...@comcast.net> wrote in message
> news:N5udnYakYceVb8Ta...@comcast.com...
>> Here is the crude code sketch:
>> ___________________________________________________________
> [...]
>
[...]

> void signal() {
> LONG temp, cmp = m_request;
> do {
> temp = cmp;
> cmp = InterlockedCompareExchange(
> &m_request,
> (cmp) ? cmp - 1 : 0,
> cmp);
> } while(cmp == temp);
^^^^^^^^^^^^^^^^^^^^^^^^^^

} while(cmp != temp);

of course!!!


> if (cmp) {
> sys_issue(1);
> }
> }


0 new messages