5

I have multiple threads accessing a single Int32 variable with "++" or "--" operation.

Do we need to lock before accessing it as below?

    lock (idleAgentsLock)
    {
       idleAgents--;  //we should place it here.
    }

Or what consequences will there be if I don't do the locking?

Jason Maldonado
  • 802
  • 1
  • 8
  • 9
smwikipedia
  • 61,609
  • 92
  • 309
  • 482
  • 1
    The better question is likely, is what *uses* `idleAgents` concurrency safe? – user2246674 Jul 26 '13 at 02:50
  • possible duplicate of [Is the ++ operator thread safe?](http://stackoverflow.com/questions/4628243/is-the-operator-thread-safe) – nawfal Oct 11 '13 at 20:55

4 Answers4

6

It is not "atomic", not in the multi-threaded sense. However your lock protects the operation, theoretically at least. When in doubt you can of course use Interlocked.Decrement instead.

kprobst
  • 16,165
  • 5
  • 32
  • 53
5

No ++/-- is not an atomic operation, however reading and writing to integers and other primitive times is considered an atomic operation.

See these links:

Also this blog post way be of some interest to you.

I would suggest the Interlocked.Increment in the Interlocked class for an atomic implementation of ++/-- operators.

Community
  • 1
  • 1
jrbeverly
  • 1,611
  • 14
  • 20
1

It depends on the machine architecture. In general, no, the compiler may generate a load instruction, increment/decrement the value and then store it. So, other threads may indeed read the value between those operations.

Most CPU instructions sets have a special atomic test & set instruction for this purpose. Assuming you don't want to embed assembly instructions into your C# code, the next best approach is to use mutual exclusion, similar to what you've shown. The implementation of that mechanism ultimately uses an instruction that is atomic to implement the mutex (or whatever it uses).

In short: yes, you should ensure mutual exclusion.

Beyond the scope of this answer, there are other techniques for managing shared data that may be appropriate or not depending on the domain logic of your situation.

DavidJ
  • 4,369
  • 4
  • 26
  • 42
1

A unprotected increment/decrement is not thread-safe - and thus not atomic between threads. (Although it might be "atomic" wrt the actual IL/ML transform1.)

This LINQPad example code shows unpredictable results:

void Main()
{
    int nWorkers = 10;
    int nLoad = 200000;
    int counter = nWorkers * nLoad;
    List<Thread> threads = new List<Thread>();
    for (var i = 0; i < nWorkers; i++) {
        var th = new Thread((_) => {
            for (var j = 0; j < nLoad; j++) {
                counter--; // bad
            }
        });
        th.Start();
        threads.Add(th);
    }
    foreach (var w in threads) {
        w.Join();
    }
    counter.Dump();
}

Note that the visibility between threads is of importance. Synchronization guarantees this visibility in addition to atomicity.

This code is easily fixed, at least in the limited context presented. Switch out the decrement and observe the results:

counter--;                           // bad
Interlocked.Decrement(ref counter);  // good
lock (threads) { counter--; }        // good

1 Even when using a volatile variable, the results are still unpredictable. This seems to indicate that (at least here, when I just ran it) it is also not an atomic operator as read/op/write of competing threads were interleaved. To see that the behavior is still incorrect when visibility issues are removed (are they?), add

class x {
    public static volatile int counter;
}

and modify the above code to use x.counter instead of the local counter variable.

user2246674
  • 7,621
  • 25
  • 28