Printable View
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?
If it's just one variable that is changing then Critical Sections seem overkill indeed
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.Quote:
Originally Posted by 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...
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:
This way, no access violation to the boolean can occur while all the threads are closingCode: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 );
}
}
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. ;)Quote:
Originally Posted by shoppinit
You should be worried about race conditions, however. Even with 'just' a bool, you can mess up.
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 :)
Quote:
Originally Posted by ProgrammerC++
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.Quote:
Originally Posted by ProgrammerC++
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
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.Code:bool myBool = false; // shared resource
// multiple threads running this function
void ThreadFunction()
{
if (!myBool) {
myBool = true;
// do something that should only be done once
}
}
The thread start explicitly said he only changes the boolean in 1 thread.
Then you still didn't get it.Quote:
Originally Posted by ProgrammerC++
Quote:
Originally Posted by D_Drmmr
You can only get a race condition when there's two or more threads writing it
Not so. You get a race condition if one or more thread write, while other threads are reading (or writing).Quote:
Originally Posted by ProgrammerC++
Why is thread synchronization so difficult?
>> 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
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
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):
good:Code://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 );
}
Show me with good examples that I'm wrong, please.Code://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
}
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.Quote:
Originally Posted by ProgrammerC++
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?
>> 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
gg
Actually 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.Quote:
Originally Posted by ProgrammerC++
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.
>> 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.
>> 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
That'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.Quote:
Originally Posted by Codeplug
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.
Just an aside to the main conversation, ResumeThread(), like SuspendThread(), should never be used in any Windows programming.Quote:
Originally Posted by ProgrammerC++
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
Actually the documentation says SuspendThread can be used but only by the thread 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.Quote:
..signal the other thread to suspend itself...
This is not documented though but sounds most logic
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.Quote:
Originally Posted by ProgrammerC++
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:
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.Code:Thread 1 (writer):
stopWriting = true;
write file summary to end of file
Thread 2 (reader):
while (!stopWriting)
write record to end of file
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.
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.Quote:
Originally Posted by ProgrammerC++
The documentation is clear and unambiguous, and includes the following warning (from "SuspendThread Function" at http://msdn.microsoft.com/en-us/libr...8VS.85%29.aspx):
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.Quote:
Originally Posted by MSDN
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.Quote:
Originally Posted by ProgrammerC++
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
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)
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.Quote:
Originally Posted by ProgrammerC++
Or you could do nothing.
The choice seems obvious.
guys, could you please recomend some books (THREADS)
thnks !
For Windows, the following book is the process and threading standard:
Programming Applications For Microsoft Windows by Jeffrey Richter
Wikipedia defines the race condition asQuote:
Originally Posted by Arjay
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.Quote:
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.
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.).Quote:
Originally Posted by itsmeandnobodyelse