Multithread Interlocking -- What is wrong with this soluition?

[nde_plume]

I need a simple multi-thread/process interlock mechanism, that works on
Windows and Linux. However, they have rather different mechanisms. I
put together a system that seems to me to work, and be simple, without
any OS intervention. However, I wanted to ask if anyone saw any
possible leakage. Here it is:

static volatile int lock = 0; // Variable for lock

void controlledAccess() // Function to control access
{
for(;
{
if(++lock == 1)
{
// locked section, free access to protected resource
--lock;
break;
}
else
{
--lock;
sleep(1000); // Or whatever }
}
}

Anyone see a way that two threads can get in the critical section?

 

[Victor Bazarov]

I need a simple multi-thread/process interlock mechanism, that works
on Windows and Linux. However, they have rather different mechanisms.
[..]
Anyone see a way that two threads can get in the critical section?

Since C++ does not define any behaviour WRT threads, your post is much
better suited for 'comp.programming.threads'. That said, 'volatile' is
a good first step, as I see it. But do visit 'c.p.t', regardless.

V

 

[Joe Seigh]

I need a simple multi-thread/process interlock mechanism, that works on
Windows and Linux. However, they have rather different mechanisms. I
put together a system that seems to me to work, and be simple, without
any OS intervention. However, I wanted to ask if anyone saw any
possible leakage. Here it is:

static volatile int lock = 0; // Variable for lock

void controlledAccess() // Function to control access
{
for(;
{
if(++lock == 1)
{
// locked section, free access to protected resource
--lock;
break;
}
else
{
--lock;
sleep(1000); // Or whatever }
}
}

Anyone see a way that two threads can get in the critical section?

Yes, the ++ and -- operations aren't atomic so you can get race
conditions. Two or more threads could see ++lock return 1, e.g.

t1: load reg,lock // reg and lock equal zero
t1: inc reg // reg equals 1
t2: load reg,lock // reg and lock equal zero
t1: store reg,lock // lock equals 1
t2: inc reg // reg equals 1
t2: store reg,lock // lock equals 1

Also you don't have any memory barriers or compiler
optimization barriers to prevent code movement by the
compiler.

And even if the above weren't a problem, you could have
enough threads such that the lock value never went to zero
and cause starvation of the waiting threads. Putting in
a
while (lock != 0) { sleep(1000); }
before the ++lock would help but not prevent the problem
entirely.

Wait loops like that aren't very efficient or responsive.

Lastly, volatile has no meaning with respect to threading.
So using it won't make a threaded program any more or less
correct.

 

[Eric Jensen]

I need a simple multi-thread/process interlock mechanism, that works on
Windows and Linux. However, they have rather different mechanisms. I
put together a system that seems to me to work, and be simple, without
any OS intervention. However, I wanted to ask if anyone saw any
possible leakage. Here it is:

static volatile int lock = 0; // Variable for lock

void controlledAccess() // Function to control access
{
for(;
{
if(++lock == 1)
{
// locked section, free access to protected resource
--lock;
break;
}
else
{
--lock;
sleep(1000); // Or whatever }
}
}

Anyone see a way that two threads can get in the critical section?

That is absolutely not thread safe
2 or more threads could see lock == 1.

For win32 you can use critical section on a single cpu system and semaphores
on linux
the following sample is thread safe on a single cpu'ed system.
For multi cpu'ed systems win32 has functions like InterlockedIncrement(),
InterlockedDecrement(), InterlockedExchange() they will make sure that
processors are instructed to force their memory caches to agree
with main memory (so we're sure both cpus see the same value of an object).

For win32 i made my own critical section, that simply uses
InterlockedEchange
to compare the thread id's and set the lock. You could do the same and make
it work on both platforms.

#if defined (_WIN32)
CRITICAL_SECTION cs;
#else
sem_t semlock;
#endif

DWORD WINAPI SomeThread(LPVOID lpParam) {
int iTrd = (int)lpParam;
while (condition) {
#if defined (_WIN32)
EnterCriticalSection(&cs);
#else
while (sem_wait(&semlock) == -1) {
if (errno != EINTR) {
// failed to lock
}
}
#endif
std::cout << "Thread " << iTrd << " says: Hello World!" <<
std::endl;
#if defined (_WIN32)
LeaveCriticalSection(&cs);
#else
if (sem_post(&semlock) == -1) {
// failed to unlock
}
#endif
Sleep(200);
}
}

void main() {
#if defined (_WIN32)
InitializeCriticalSection(&cs);
#else
if (sem_init(&semlock, 0, 1) == -1) {
// failed to init
}
#endif
for (int i=0; i<max_trd; i++) {
CreateThread(params);
}
while (condition) {
// program main loop
}
#if defined (_WIN32)
DeleteCriticalSection(&cs);
#else
if (sem_destroy(&semlock) == -1) {
// failed
}
#endif
}

I hope this was any help to you

//eric

 

[mlimber]

I need a simple multi-thread/process interlock mechanism, that works on
Windows and Linux. However, they have rather different mechanisms.

[snip]

In addition to what the others have said, you might consider
Boost.Threads, which is primitive but works on POSIX-style UNIX and on
Windows. See:

http://boost.org/doc/html/threads.html

Cheers! --M