Critical Sections and Read operations.

[shoppinit]

If I had several threads that could modify a single variable, then I would enclose all the write operations in critical sections. If only one thread can modify the variable but the others can read it, do the read operations need to be enclosed in a critical section?

This question has been bugging me for a while and up to now I'm enclosing all read and write operations of a variable in a critical section. Maybe this is overkill?

[ProgrammerC++]

If it's just one variable that is changing then Critical Sections seem overkill indeed

[D_Drmmr]

If I had several threads that could modify a single variable, then I would enclose all the write operations in critical sections. If only one thread can modify the variable but the others can read it, do the read operations need to be enclosed in a critical section?

If multiple threads access a variable where at least one of the threads writes to it, then all threads must synchronize access to the variable. Otherwise, you have a race condition.

[shoppinit]

OK, I see.

The case in point is a bool bThreadContinue that I test to see if a flag has been set to let the thread terminate. I'm not so concerned about race conditions in this case, but rather access violations...

[ProgrammerC++]

A race condition can't occur in this scenario.

An access violation can not occur if it's a global static boolean. However, it's different in other cases, I suggest something like:


pseudo code

Destructor::~Destructor()
{
bThreadContinue = FALSE; //Local var of class

for all threads
{
ResumeThread( HandleToThread ); // if the thread is in suspended mode
WaitForSingleObject( HandleToThread, INFINITE );
CloseHandle( HandleToThread );
}
}


This way, no access violation to the boolean can occur while all the threads are closing

[D_Drmmr]

The case in point is a bool bThreadContinue that I test to see if a flag has been set to let the thread terminate. I'm not so concerned about race conditions in this case, but rather access violations...
AFAIK, to cause an access violation you will need to do some erroneous pointer arithmetics (either directly or indirectly). If only a bool is involved, I don't see that happening. ;)

You should be worried about race conditions, however. Even with 'just' a bool, you can mess up.

[shoppinit]

I was just wondering about threads running on different cores accessing the same memory area. On a single core processor, I agree that there wouldn't be a problem. I think I'll stick to using critical sections, just to be on the safe side :)

[D_Drmmr]

If it's just one variable that is changing then Critical Sections seem overkill indeed
A race condition can't occur in this scenario.

I don't know if it's because you don't have much experience with multi-threading, but I don't consider this to be very good advice.

It's not that difficult to create a race condition using just a bool variable. Given that you haven't seen a single line of code, how do you conclude that a race condition cannot occur? :confused:

Consider

bool myBool = false; // shared resource

// multiple threads running this function
void ThreadFunction()
{
if (!myBool) {
myBool = true;
// do something that should only be done once
}
}

A naive programmer might think that the code inside the if block won't get executed by more that one thread, but any number of threads running this function can evaluate the if clause before any of them set the bool to true in the first statement in the if clause. If you don't call that a race condition, IMO you are using a useless definition of the term.

[ProgrammerC++]

The thread start explicitly said he only changes the boolean in 1 thread.

[D_Drmmr]

The thread start explicitly said he only changes the boolean in 1 thread.
Then you still didn't get it.
If multiple threads access a variable where at least one of the threads writes to it, then all threads must synchronize access to the variable. Otherwise, you have a race condition.

[ProgrammerC++]

You can only get a race condition when there's two or more threads writing it

[Arjay]

You can only get a race condition when there's two or more threads writing itNot so. You get a race condition if one or more thread write, while other threads are reading (or writing).

Why is thread synchronization so difficult?

[Codeplug]

>> You can only get a race condition when there's two or more threads writing it
No. You only need two threads - one reading and one writing. Without proper synchronization you have no guarantee that a write in thread A will even be seen in thread B.

[Arjay steals my thunder...]

gg

[ProgrammerC++]

I think both of you above me don't know that a race condition can't occur when there's 1 thread writing to 1 boolean and all other threads only read the boolean

[ProgrammerC++]

bad (well, compared to 'good' below, this still works but its slower since you have to go into the useless critical section all the time):


//psuedo code

//globals
CRITICAL_SECTION critsec;
bool closeworkerthreads = false;

bool getit()
{
EnterCriticalSection( &critsec );

bool to_return = closeworkterthreads;

LeaveCriticalSection( &critsec );

return to_return;
}

void setit( bool b )
{
EnterCriticalSection( &critsec );

closeworkterthreads = b;

LeaveCriticalSection( &critsec );
}

DWORD WINAPI workerthread()
{
while( getit() == false )
{
//... do some work

Sleep( 100 );
}
}

int main()
{
InitializeCriticalSection( &critsec );

HANDLE handles[8];
handles[0] = CreateThread( workerthread );
handles[1] = CreateThread( workerthread );
handles[2] = CreateThread( workerthread );
handles[3] = CreateThread( workerthread );
handles[4] = CreateThread( workerthread );
handles[5] = CreateThread( workerthread );
handles[6] = CreateThread( workerthread );
handles[7] = CreateThread( workerthread );

Sleep( 5000 ); //wait 5 seconds before writing to it
setit( true ); //manager thread decides all worker threads can close

WaitForMultipleObjects( handles ); //Wait till all threads are closed

DeleteCriticalSection( &critsec );
}


good:


//psuedo code

//globals
bool closeworkerthreads = false;

DWORD WINAPI workerthread()
{
while( closeworkerthreads == false )
{
//... do some work

Sleep( 100 );
}
}

int main()
{
HANDLE handles[8];
handles[0] = CreateThread( workerthread );
handles[1] = CreateThread( workerthread );
handles[2] = CreateThread( workerthread );
handles[3] = CreateThread( workerthread );
handles[4] = CreateThread( workerthread );
handles[5] = CreateThread( workerthread );
handles[6] = CreateThread( workerthread );
handles[7] = CreateThread( workerthread );

Sleep( 5000 ); //wait 5 seconds before writing to it
closeworkerthreads = true; //manager thread decides all worker threads can close

WaitForMultipleObjects( handles ); //Wait till all threads are closed
}


Show me with good examples that I'm wrong, please.

[D_Drmmr]

Show me with good examples that I'm wrong, please.
That's not the way things work. If you want to know if your code is thread safe, you don't ask someone else to show that it's not thread safe, you analyze the code to prove that it is thread safe. That start with making all assumptions clear, which is something you haven't done in any of your posts in this thread, thereby creating needless confusion.

Now, I'm not sure if your code will be thread safe on a particular version of a particular compiler. Again, that's not up to me to disprove. However, I do know that your code is not thread safe in general - for the reason mentioned numerous times above. That means that if you use this, you're not guaranteed it will work with another compiler, with another version of the same compiler or even with different compiler settings. So why use it, let alone advice other people to use it?

[Codeplug]

>> Show me with good examples that I'm wrong, please.
Your example will do just fine. Since you don't believe that, what good is another example?

There is no guarantee that any of the threads will see 'closeworkerthreads' change value. Your observation with your particular compiler is no guarantee.

Posix and C++0x are the only standards I know that cover multi-threading. Your code has undefined behavior under both.

If you want to make your code correct for any Win32 compiler, you need to use a Win32 synchronization primitive - like an interlocked function or a critical section etc...

gg

[ProgrammerC++]

gg

[Arjay]

I think both of you above me don't know that a race condition can't occur when there's 1 thread writing to 1 boolean and all other threads only read the booleanActually I understand this very well. If the shared resource is a boolean which can be changed in an atomic operation, then no race condition will occur. This is the one special case where no synchronizating is required.

However, every other variable (BOOL, int, string, etc), you will encounter a race condition if one thread writes while one or more threads are reading or writing.

When I answered this question, I'm not interested in talking about the one special case (i.e. boolean) where sharing a resource across threads works without synchronization.

Folks tend to find thread synchronizating hard enough as it is. To me, it's easier to try to list the rules where you need synchronization:

1) If at least one thread is writing while one or more threads are reading or writing, then you need synchronization.
2) If all threads only every read, then no synchronization is required.

It's easier to apply these rules to all variables/shared resources including a boolean type. For the boolean, synchronization isn't technically required, but doesn't hurt.

As far as boolean, I never use these sorts of flag variables anyway to signal the thread to perform some action. Instead I'll use a windows event. Advantages of using an events are they can be used in the WaitFor functions and they can be used across processes.

[Codeplug]

>> If the shared resource is a boolean which can be changed in an atomic operation, then no race condition will occur.
That is still an [x86] generalization that doesn't account for memory-visibility and cache-coherency algorithms that other architectures may employ. In other words, an [atomic] write is of little use if another core never "sees" that write.

For x86 architectures, I believe it is safe to say that a store to memory by one core will eventually be "seen" (via a load) by other cores. But that is thanks to the x86 cache coherency protocol (http://en.wikipedia.org/wiki/MESI).

>> For the boolean, synchronization isn't technically required, but doesn't hurt.
The only way I can agree with this statement is to qualify it with "on x86 hardware, using assembly language - or compiler documented extensions ;)".

gg

[Arjay]

>> If the shared resource is a boolean which can be changed in an atomic operation, then no race condition will occur.
That is still an [x86] generalization that doesn't account for memory-visibility and cache-coherency algorithms that other architectures may employ. In other words, an [atomic] write is of little use if another core never "sees" that write.

For x86 architectures, I believe it is safe to say that a store to memory by one core will eventually be "seen" (via a load) by other cores. But that is thanks to the x86 cache coherency protocol (http://en.wikipedia.org/wiki/MESI).

>> For the boolean, synchronization isn't technically required, but doesn't hurt.
The only way I can agree with this statement is to qualify it with "on x86 hardware, using assembly language - or compiler documented extensions ;)".

ggThat's great. I generally don't rely on any type to synchronize themselves. Last I checked on booleans, they were atomic but that doesn't really matter to me for two reasons. 1) I'm looking for a generic solution that works regardless of the type of shared resource. 2) I have never found a need to synchronize a bool. I never use flag variables, so I never synchronize booleans.

Finally, I'm always looking for a solution that doesn't require me to have hardware specific knowledge. If I use a critical section to synchronize, I know it will work across Windows.

[MikeAThon]

....

pseudo code

Destructor::~Destructor()
{
bThreadContinue = FALSE; //Local var of class

for all threads
{
ResumeThread( HandleToThread ); // if the thread is in suspended mode
WaitForSingleObject( HandleToThread, INFINITE );
CloseHandle( HandleToThread );
}
}


Just an aside to the main conversation, ResumeThread(), like SuspendThread(), should never be used in any Windows programming.

There is one exception, but it's a very narrow one, unlikely to be encountered frequently. The exception is this: if you are writing a debugger for other programs, then you may use Suspend/ResumeThread on the threads of the other programs.

The documentation explains this clearly.

Mike

[ProgrammerC++]

Actually the documentation says SuspendThread can be used but only by the thread itself.


..signal the other thread to suspend itself...


I believe Suspending a thread is better (on itself) is better than to use something like WaitForSingleObject(event, infinite) because when a thread is suspended 0 CPU is used ever on that thread while I think for WaitForSingleObject still probably uses abit CPU to keep checking the state of the event.

This is not documented though but sounds most logic

[Lindley]

I think both of you above me don't know that a race condition can't occur when there's 1 thread writing to 1 boolean and all other threads only read the boolean

In the specific case of using a boolean to let a thread know it's okay to terminate, this may to be true. In fact, it probably is on most compilers.

However, this relies on the writer thread not needing to do anything critical after modifying the boolean, only before. In the other case, you certainly *can* get a race condition:


Thread 1 (writer):

stopWriting = true;
write file summary to end of file

Thread 2 (reader):

while (!stopWriting)
write record to end of file


Here, thread 1 has no way of knowing when it's safe to actually proceed and write the file footer, so this is a race condition.

The lesson is: it's possible to write lock-free code, and very occasionally necessary, but as a rule of thumb: lock everything that even *might* need it. You'll suffer far less grief with a slight performance hit than spending 10 hours searching for an elusive Hiesenbug because you thought you didn't need to lock something.

[MikeAThon]

Actually the documentation says SuspendThread can be used but only by the thread itself.
I think you have missed the main point, which is not to call SuspendThread ever, unless you fall into one narrow exception that is unlikely to be encountered by most Windows programmers.

The documentation is clear and unambiguous, and includes the following warning (from "SuspendThread Function" at http://msdn.microsoft.com/en-us/library/ms686345%28VS.85%29.aspx):
This function is primarily designed for use by debuggers. It is not intended to be used for thread synchronization.
It doesn't really matter what follows after this warning. It's enough to know that SuspendThread "is not intended to be used for thread synchronization". If you are using SuspendThread for thread synchronization, then you are probably doing something wrong.
I believe Suspending a thread is better (on itself) is better than to use something like WaitForSingleObject(event, infinite) because when a thread is suspended 0 CPU is used ever on that thread while I think for WaitForSingleObject still probably uses abit CPU to keep checking the state of the event.

This is not documented though but sounds most logic
This conclusion seems unfounded. The documentation also states that it is the user-mode code for a thread that's suspended. Just like WFSO, it's probable that kernel-mode code is still executed: in the case of WFSO, a check for the event, and in the case of SuspendThread, a check of the suspend count.

But even if a Suspend'edThread is somehow "more suspended" than a WFSO thread, that's hardly a reason to use it improperly, for purposes of thread synchronization, in the face of the admonition of Microsoft not to.

Mike

[ProgrammerC++]

I use SuspendThread on a thread that's used for each socket to be the receiver for that socket.

If the thread is waiting for a new socket to handle its inside SuspendThread and another thread will simply say ResumeThread to tell it its socket is ready to receive (so on connect or accept).

blocking socket (blocking recv()) obviously.

after the socket is closed the thread goes back in SuspendThread and there we start again.

I guess I could achieve the same result with Events and WaitForSingleObject but at the time I wrote that I didnt know of these functions. I don't think rewriting will make any positive difference anyway (only negative since it costs time to implement and test)

[Arjay]

I don't think rewriting will make any positive difference anyway (only negative since it costs time to implement and test)On the other hand, you have a current problem and have been offerred an alternate approach to solve the problem. If you want to solve the problem, it seems worth it to try the new approach.

Or you could do nothing.

The choice seems obvious.

[exsan]

guys, could you please recomend some books (THREADS)
thnks !

[Arjay]

For Windows, the following book is the process and threading standard:

Programming Applications For Microsoft Windows by Jeffrey Richter (http://www.amazon.com/Programming-Applications-Microsoft-Windows-General/dp/1572319968/ref=sr_1_4?s=books&ie=UTF8&qid=1292714352&sr=1-4)

[itsmeandnobodyelse]

Not so. You get a race condition if one or more thread write, while other threads are reading (or writing).

Why is thread synchronization so difficult?

Wikipedia defines the race condition as


A race condition or race hazard is a flaw in an electronic system or process whereby the output and/or result of the process is unexpectedly and critically dependent on the sequence or timing of other events. The term originates with the idea of two signals racing each other to influence the output first.


In case a (main) thread sets a shared boolean variable to true while multiple threads make a periodically check on that boolean therefore isn't a race condition. There is nothing unexpectedly and critically added when one of the threads reads the 'true' in the next period.

An atomic operation on a shared variable is only required if you want to be able to write from more than one thread and make sure that the second writer was reading the current value written by the frist writer. That normally only makes sense if you increment an integer or toggle a boolean cause if the writers don't (have to) care for the old contents (and that behavior isn't an error ) it actually isn't a race condition despite of two writers cause the result written is arbitrary anyhow depending on which thread was first or second.

In case of an incrementation or toggle you need synchronisation or an atomic setting cause otherwise you couldn't prevent from the case that two threads both do increment/toggle the old value hence one incrementation/toggle gets lost what is a race condition.

BTW, if the operation system offers an atomic incrementation (e. g. InterlockedIncrement of WINAPI) the atomic operation is guaranteed also for multi-processor and multi-core architectures. But of course on the latter systems an i++ in general isn't an atomic operation.

[Arjay]

Wikipedia defines the race condition as



In case a (main) thread sets a shared boolean variable to true while multiple threads make a periodically check on that boolean therefore isn't a race condition. There is nothing unexpectedly and critically added when one of the threads reads the 'true' in the next period.

An atomic operation on a shared variable is only required if you want to be able to write from more than one thread and make sure that the second writer was reading the current value written by the frist writer. That normally only makes sense if you increment an integer or toggle a boolean cause if the writers don't (have to) care for the old contents (and that behavior isn't an error ) it actually isn't a race condition despite of two writers cause the result written is arbitrary anyhow depending on which thread was first or second.

In case of an incrementation or toggle you need synchronisation or an atomic setting cause otherwise you couldn't prevent from the case that two threads both do increment/toggle the old value hence one incrementation/toggle gets lost what is a race condition.

BTW, if the operation system offers an atomic incrementation (e. g. InterlockedIncrement of WINAPI) the atomic operation is guaranteed also for multi-processor and multi-core architectures. But of course on the latter systems an i++ in general isn't an atomic operation.As I mentioned before - I'm not much interested in the special case of synchronization of boolean variables because I don't much use them in my multithreaded apps. While I might be classifying unwanted behavior as a 'race condition' for a non-synchronized boolean variable when it technically isn't a race condition, it still is unwanted behavior. At the end of the day, thread safety rules will still determine when a shared resource needs to be synchronized - it doesn't much matter what type the resource is (bool, int, string, etc.).