56

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?

Matthew Murdoch
  • 30,874
  • 30
  • 96
  • 127

6 Answers6

28

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
  • 243,077
  • 51
  • 345
  • 550
  • Unless you specify a volatile the read (or the write) might *never* be performed. Sooner or later is not good enough. – EFraim Oct 16 '09 at 12:34
  • 4
    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 Oct 16 '09 at 13:34
  • @jalf: No, if you need only a single flag. Read the question again. – EFraim Oct 17 '09 at 16:14
  • 1
    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 Oct 17 '09 at 16:15
  • 3
    But if a flag is being checked to determine the next step in his app, either (a) he needs to share some other data between threads. (b) the flag-check is meaningless and he can perform next steps always. Assuming (a) is the scenario, "other data" might not be ready just because the flag variable is set. Assuming that it is because of the flag is a likely suspect for runs-in-debug-crashes-in-release-build. – peter karasev Dec 13 '12 at 07:48
10

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
  • 3,960
  • 3
  • 28
  • 34
  • 1
    +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 Oct 06 '09 at 15:37
  • @Ermelli do we still need mutex if we want to use busy wait loop anyway (because the loop might do something else meanwhile) – Nick Jan 07 '17 at 16:24
2

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
  • 4,967
  • 20
  • 15
  • 2
    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 Oct 06 '09 at 12:23
  • 1
    Taking in account enum values that are read in one instruction. – Arkaitz Jimenez Oct 06 '09 at 12:26
  • 1
    @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 Oct 06 '09 at 13:21
2

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
  • 12,811
  • 4
  • 46
  • 62
  • Why do you think volatile matters? – jalf Oct 13 '09 at 14:34
  • 2
    @jalf: volatile tells the compiler to not perform optimizations that might cause your code to see a stale copy of the variable. – Stephen C Oct 13 '09 at 14:52
  • 2
    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 Oct 13 '09 at 15:16
  • @jalf: Your sooner or later might be infinity in case of a tight loop – EFraim Oct 16 '09 at 12:33
  • 1
    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 Oct 16 '09 at 13:39
  • @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 Nov 12 '09 at 14:10
  • Memory barriers weren't even part of any C++ standard till several years ago. (threading was not) Compilers weren't forced to do the right thing by the standard for volatile but most of them did. So this was more or less the only "portable" way for spinlock implementation. – EFraim Dec 21 '16 at 09:55
2

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
  • 325
  • 2
  • 7
  • > "it doesn't lock the object as long as there are no write operations.", but if you don't lock reads, how do you know write operations are not happening? in the wikipedia page you link, the pseudocode implementations do lock readers so that either readers are reading or a writer is writing, but not both. – jmmut Apr 25 '23 at 10:23
0

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.
  • 34,624
  • 22
  • 86
  • 128