Re: Wanted: Read many, Write Exclusive Lock
Chris Thomasson
news:%23uFC1l2...@TK2MSFTNGP05.phx.gbl...
> OK. Need to refresh your C++. 'throw' without arguments just rethrows an
> original exception. It doesn't make sense to use it to abort a constructor
> or other functions. If you actually wanted to throw some exception, then
> empty throw() qualifier doesn't make sense, either.
>
> It doesn't make sense to declare m_count mutable. You don't provide any
> const methods, anyway.
>
> Now analyse the following sequence:
>
> rdlock()
> rdlock()
> rdlock()
>
> wrlock() // waiting
>
> rdunlock() // now what?
Reload the code. There was a race-condition. On your case rdunlock would
detect a m_count < 1 and posted to the event waking the writer. Problem...
Please re-download the link, and tell me what you think:
http://appcore.home.comcast.net/~appcore/misc/rwmutex_win_hpp.html
Okay, here is a little contest... Who can identify the race-condition I
found? Hint: it has to do with rdunlock responding to wrlock... Anyway, the
corrected code is on the website.
Thank you for looking! Please feel free to ask any other questions you may
have wrt the new code I just posted. Extra points if you can explain how the
race could possible violate reader/writer constraints... Sorry about the
bugged code... I have updated it, and appreciate it if you could review it
once more!
:^)
Chris Thomasson
"Chris Thomasson" <cri...@comcast.net> wrote in message
news:ho2dnSIv8sHZb7ra...@comcast.com...
> "Alexander Grigoriev" <al...@earthlink.net> wrote in message
> news:%23uFC1l2...@TK2MSFTNGP05.phx.gbl...
>> OK. Need to refresh your C++. 'throw' without arguments just rethrows an
>> original exception. It doesn't make sense to use it to abort a
>> constructor or other functions. If you actually wanted to throw some
>> exception, then empty throw() qualifier doesn't make sense, either.
>>
>> It doesn't make sense to declare m_count mutable. You don't provide any
>> const methods, anyway.
>>
>> Now analyse the following sequence:
>>
>> rdlock()
>> rdlock()
>> rdlock()
>>
>> wrlock() // waiting
>>
>> rdunlock() // now what?
>
> Reload the code. There was a race-condition. On your case rdunlock would
> detect a m_count < 1 and posted to the event waking the writer. Problem...
> Please re-download the link, and tell me what you think:
>
> http://appcore.home.comcast.net/~appcore/misc/rwmutex_win_hpp.html
>
>
>
> Okay, here is a little contest... Who can identify the race-condition I
> found? Hint: it has to do with rdunlock responding to wrlock... Anyway,
> the corrected code is on the website.
______________________________________________________________
#if ! defined(RWMUTEX_WIN_H)
#define RWMUTEX_WIN_H
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <assert.h>
#include <limits.h>
class rwmutex_win {
rwmutex_win(rwmutex_win const&);
rwmutex_win const& operator =(rwmutex_win const&);
private:
mutable LONG m_count;
mutable LONG m_rdwake;
HANDLE m_wrwset;
HANDLE m_rdwset;
LONG const m_rddepth;
CRITICAL_SECTION m_wrlock;
public:
rwmutex_win(LONG rddepth = LONG_MAX)
: m_count(rddepth),
m_rdwake(0),
m_wrwset(CreateEvent(0, FALSE, FALSE, 0)),
m_rdwset(CreateSemaphore(0, 0, rddepth, 0)),
m_rddepth(rddepth) {
if (! m_rdwset || ! m_wrwset) {
if (m_rdwset) { CloseHandle(m_rdwset); }
if (m_wrwset) { CloseHandle(m_wrwset); }
throw;
}
InitializeCriticalSection(&m_wrlock);
}
~rwmutex_win() throw() {
DeleteCriticalSection(&m_wrlock);
if (! CloseHandle(m_rdwset)) { assert(0); }
if (! CloseHandle(m_wrwset)) { assert(0); }
}
public:
void wrlock() throw() {
EnterCriticalSection(&m_wrlock);
LONG count =
InterlockedExchangeAdd(&m_count, -m_rddepth);
if (count < m_rddepth) {
if ( InterlockedExchangeAdd(&m_rdwake, m_rddepth - count) +
m_rddepth - count) {
if (WaitForSingleObject(m_wrwset,
INFINITE) != WAIT_OBJECT_0) {
assert(0); throw;
}
}
}
}
void wrunlock() throw() {
LONG count =
InterlockedExchangeAdd(&m_count, m_rddepth);
if (count < 0) {
if (! ReleaseSemaphore(m_rdwset, count * -1, 0)) {
assert(0); throw;
}
}
LeaveCriticalSection(&m_wrlock);
}
public:
void rdlock() throw() {
LONG count = InterlockedDecrement(&m_count);
if (count < 0) {
if (WaitForSingleObject(m_rdwset,
INFINITE) != WAIT_OBJECT_0) {
assert(0); throw;
}
}
}
void rdunlock() throw() {
LONG count = InterlockedIncrement(&m_count);
if (count < 1) {
if (! InterlockedDecrement(&m_rdwake)) {
if (! SetEvent(m_wrwset)) {
assert(0); throw;
}
}
}
}
};
______________________________________________________________
Chris Thomasson
>
> "Chris Thomasson" <cri...@comcast.net> wrote in message
> news:ho2dnSIv8sHZb7ra...@comcast.com...
>> "Alexander Grigoriev" <al...@earthlink.net> wrote in message
>> news:%23uFC1l2...@TK2MSFTNGP05.phx.gbl...
>>> OK. Need to refresh your C++. 'throw' without arguments just rethrows an
>>> original exception. It doesn't make sense to use it to abort a
>>> constructor or other functions. If you actually wanted to throw some
>>> exception, then empty throw() qualifier doesn't make sense, either.
>>>
>>> It doesn't make sense to declare m_count mutable. You don't provide any
>>> const methods, anyway.
>>>
>>> Now analyse the following sequence:
>>>
>>> rdlock()
>>> rdlock()
>>> rdlock()
>>>
>>> wrlock() // waiting
>>>
>>> rdunlock() // now what?
>>
>> Reload the code. There was a race-condition. On your case rdunlock would
>> detect a m_count < 1 and posted to the event waking the writer.
>> Problem... Please re-download the link, and tell me what you think:
>>
>> http://appcore.home.comcast.net/~appcore/misc/rwmutex_win_hpp.html
>>
>>
>>
>> Okay, here is a little contest... Who can identify the race-condition I
>> found? Hint: it has to do with rdunlock responding to wrlock... Anyway,
>> the corrected code is on the website.
>
> ______________________________________________________________
> ______________________________________________________________
>
>
Has anybody tried this yet? It looks good to me.
Alexander Grigoriev
>>> Okay, here is a little contest... Who can identify the race-condition I
>>> found? Hint: it has to do with rdunlock responding to wrlock... Anyway,
>>> the corrected code is on the website.
>>
>> ______________________________________________________________
> [...]
>> ______________________________________________________________
>>
>>
>
> Has anybody tried this yet? It looks good to me.
No apparent holes. But 'throw' statements still make no sense.
Chris Thomasson
_______________________________________
> #include <assert.h>
> #include <limits.h>
[...]
#include <cassert>
#include <climits>
of course.