views:

36

answers:

3

Hi all, Suppose I have two threads A and B that are both incrementing a ~global~ variable "count". Each thread runs a for loop like this one:

for(int i=0; i<1000; i++)
    count++; //alternatively, count = count + 1;

i.e. each thread increments count 1000 times, and let's say count starts at 0. Can there be sync issues in this case? Or will count correctly equal 2000 when the execution is finished? I guess since the statement "count = count + 1" may break down into TWO assembly instructions, there is potential for the other thread to be swapped in between these two instructions? Not sure. What do you think?

+4  A: 
blucz
+1 for atomic ops suggestion.
Drew Hall
+3  A: 

Yes, there can be.

There is no guarantee that an increment itself is an atomic operation.

For example, if one thread reads the value for increment then gets swapped out, the other thread could come in and change it, then the first thread will write back the wrong value:

+-----+
|   0 | Value stored in memory (0).
+-----+
|   0 | Thread 1 reads value into register (r1 = 0).
+-----+
|   0 | Thread 2 reads value into register (r2 = 0).
+-----+
|   1 | Thread 2 increments r2 and writes back.
+-----+
|   1 | Thread 1 increments r1 and writes back.
+-----+

So you can see that, even though both threads have tried to increment the value, it's only increased by one. This is just one of the possible problems. It may even be that the write itself is not atomic and one thread may update only part of the value before being swapped out.

Use mutexes. That's what they're for.

paxdiablo
The issue you've exemplified barely makes sense on modern CPUs. Typically the problem is not in atomically updating multiple bytes since machines generally update a word at a time atomically. Furthermore, all you'd need to be safe from your example is a volatile keyword. The problem that you usually see in practice is much simpler: two threads read the same value, each increment it independently in a register, then write back the same value.
blucz
@blucz, I've provided a simpler situation (in progress while you were typing your comment by the looks of it) but you're dead wrong. POSIX threads do _not_ make assumptions about the hardware they're running on and whether its writes are atomic or not. If you're going to follow a standard, you should follow it. If you want to introduce further assumptions, that's okay, just be aware of the ramifications and don't pretend to be following the standard :-)
paxdiablo
I think you misunderstood my comment. I didn't say anything about the pthread standard. I just suggested that in practice, the example you were giving was very unlikely, which it was, for the reason I gave. Is it possible to have that error? Sure..on a system that 99.999% of us will never see.
blucz
Yes, unfortunately I _work_ on such a beast every day and yet it's almost certainly responsible for tracking every cent that enters or exits your bank account :-) The System z mainframe is very adept at discovering code that makes unwarranted assumptions, things like those people assuming that the characters `A` thru `Z` are contiguous. The fact that 99.999% of the population will never see it doesn't mean it will only affect 0.001% if something goes wrong.
paxdiablo
But, be that as it may, it's irrelevant since I've replaced that possibility with a much more likely one. That doesn't diminish the seriousness of the earlier problem. As I stated, channelling Yoda: "No. Try not! Do. Or do not!" - either follow the standard or don't Most of the time, you're right in that it may not matter but I'd prefer to be safe. Others may have different priorities.
paxdiablo
+2  A: 

Count clearly needs to be protected with a mutex or other synchronization mechanism.

At a fundamental level, the count++ statment breaks down to:

load count into register
increment register
store count from register

A context switch could occur before/after any of those steps, leading to situations like:

Thread 1:  load count into register A (value = 0)
Thread 2:  load count into register B (value = 0)
Thread 1:  increment register A (value = 1)
Thread 1:  store count from register A (value = 1)
Thread 2:  increment register B (value = 1)
Thread 2:  store count from register B (value = 1)

As you can see, both threads completed one iteration of the loop, but the net result is that count was only incremented once.

You probably would also want to make count volatile to force loads & stores to go to memory, since a good optimizer would likely keep count in a register unless otherwise told.

Also, I would suggest that if this is all the work that's going to be done in your threads, performance will dramatically drop from all the mutex locking/unlocking required to keep it consistent. Threads should have much bigger work units to perform.

Drew Hall