views:

629

answers:

9

I have a class that has a state (a simple enum) and that is accessed from two threads. For changing state I use a mutex (boost::mutex). Is it safe to check the state (e.g. compare state_ == ESTABLISHED) or do I have to use the mutex in this case too? In other words do I need the mutex when I just want to read a variable which could be concurrently written by another thread?

A: 

Yes. If thread a reads a variable while thread b is writing to it, you can read an undefined value. The read and write operation are not atomic, especially on a multi-processor system.

Jeff Ober
while a writer thread does (fetch->write->store) I don't see when in the middle of those a reader would fetch an undefined value, either the previous or the later one, but never undefined.
Arkaitz Jimenez
Taking in account enum values that are read in one instruction.
Arkaitz Jimenez
@Arkaitz: undefined was probably not the right word. But CPU/memory architectures get more and more complex with added levels of caches, increased latencies etc. The answer is simple: Say No! to lockfree sharing of data. Even experts make many mistakes in this area.
sellibitze
+1  A: 

Generally speaking you don't, if your variable is declared with "volatile". And ONLY if it is a single variable - otherwise you should be really careful about possible races.

EFraim
Why do you think volatile matters?
jalf
@jalf: volatile tells the compiler to not perform optimizations that might cause your code to see a stale copy of the variable.
Stephen C
But the question wasn't about stale copies. Even if the variable is not volatile, it will be written out sooner or later. It'll be atomic regardless of volatility, and volatile does not prevent load/store reordering. So volatile doesn't really buy you anything in this case. It doesn't solve the problem that needs solving, and it solves one that'd have been solved anyway.
jalf
@jalf: Your sooner or later might be infinity in case of a tight loop
EFraim
yes, which is why a correct solution should use a memory barrier, not volatile. Volatile is basically useless in the context of multithreading. It slows your code down, without providing the guarantees you *need*.
jalf
@jalf: No, it is not useless. Bounded wait vs. endless wait is the distinction. Do you know that Spin locks are actually based on volatiles often?
EFraim
+5  A: 

This question has a good discussion on this very topic:

Is it safe to read an integer variable thats being concurrently modified?

RC
A: 

The access to the enum ( read or write) should be guarded.

Another thing: If the thread contention is less and the threads belong to same process then Critical section would be better than mutex.

aJ
A: 

Yes, but the interlocked class will be easier to work with

Steve
+5  A: 

You have two threads, they exchange information, yes you need a mutex and you probably also need a conditional wait.

In your example (compare state_ == ESTABLISHED) indicates that thread #2 is waiting for thread #1 to initiate a connection/state. Without a mutex or conditionals/events, thread #2 has to poll the status continously.

Threads is used to increase performance (or improve responsiveness), polling usually results in decreased performance, either by consuming a lot of CPU or by introducing latencey due to the poll interval.

Ernelli
+1 for suggesting conditional variables. From the sounds of it he has a thread that needs to respond to a state change by another change. If that's the case conditional variables is much more apropriate.
Falaina
A: 

You know Lightweight Lock, easy to use and very fast. For Windows environment.

Good article about Understand the Impact of Low-Lock Techniques in Multithreaded Apps

lsalamon
A: 

actually, there is no reason to lock access to the object for reading. you only want to lock it while writing to it. this is exactly what a reader-writer lock is. it doesn't lock the object as long as there are no write operations. it improves performance and prevents deadlocks. see the following links for more elaborate explanations :

wikipedia codeproject

geva30
A: 

It depends.

The C++ language says nothing about threads or atomicity.

But on most modern CPU's, reading an integer is an atomic operation, which means that you will always read a consistent value, even without a mutex.

However, without a mutex, or some other form of synchronization, the compiler and CPU are free to reorder reads and writes, so anything more complex, anything involving accessing multiple variables, is still unsafe in the general case.

Assuming the writer thread updates some data, and then sets an integer flag to inform other threads that data is available, this could be reordered so the flag is set before updating the data. Unless you use a mutex or another form of memory barrier.

So if you want correct behavior, you don't need a mutex as such, and it's no problem if another thread writes to the variable while you're reading it. It'll be atomic unless you're working on a very unusual CPU. But you do need a memory barrier of some kind to prevent reordering in the compiler or CPU.

jalf
Unless you specify a volatile the read (or the write) might *never* be performed. Sooner or later is not good enough.
EFraim
But even with volatile, CPU or compiler might reorder the writes, making them meaningless. The correct solution is a memory barrier, and then volatile is just a needless deoptimization.
jalf
@jalf: No, if you need only a single flag. Read the question again.
EFraim
And it's probably the solutions like you propose that lead to much of the bloated code. No, you DO NOT need a barrier for a single flag
EFraim