Asked  7 Months ago    Answers:  5   Viewed   205 times

Let's say that a class has a public int counter field that is accessed by multiple threads. This int is only incremented or decremented.

To increment this field, which approach should be used, and why?

  • lock(this.locker) this.counter++;,
  • Interlocked.Increment(ref this.counter);,
  • Change the access modifier of counter to public volatile.

Now that I've discovered volatile, I've been removing many lock statements and the use of Interlocked. But is there a reason not to do this?

 Answers

14

Worst (won't actually work)

Change the access modifier of counter to public volatile

As other people have mentioned, this on its own isn't actually safe at all. The point of volatile is that multiple threads running on multiple CPUs can and will cache data and re-order instructions.

If it is not volatile, and CPU A increments a value, then CPU B may not actually see that incremented value until some time later, which may cause problems.

If it is volatile, this just ensures the two CPUs see the same data at the same time. It doesn't stop them at all from interleaving their reads and write operations which is the problem you are trying to avoid.

Second Best:

lock(this.locker) this.counter++;

This is safe to do (provided you remember to lock everywhere else that you access this.counter). It prevents any other threads from executing any other code which is guarded by locker. Using locks also, prevents the multi-CPU reordering problems as above, which is great.

The problem is, locking is slow, and if you re-use the locker in some other place which is not really related then you can end up blocking your other threads for no reason.

Best

Interlocked.Increment(ref this.counter);

This is safe, as it effectively does the read, increment, and write in 'one hit' which can't be interrupted. Because of this, it won't affect any other code, and you don't need to remember to lock elsewhere either. It's also very fast (as MSDN says, on modern CPUs, this is often literally a single CPU instruction).

I'm not entirely sure however if it gets around other CPUs reordering things, or if you also need to combine volatile with the increment.

InterlockedNotes:

  1. INTERLOCKED METHODS ARE CONCURRENTLY SAFE ON ANY NUMBER OF COREs OR CPUs.
  2. Interlocked methods apply a full fence around instructions they execute, so reordering does not happen.
  3. Interlocked methods do not need or even do not support access to a volatile field, as volatile is placed a half fence around operations on given field and interlocked is using the full fence.

Footnote: What volatile is actually good for.

As volatile doesn't prevent these kinds of multithreading issues, what's it for? A good example is saying you have two threads, one which always writes to a variable (say queueLength), and one which always reads from that same variable.

If queueLength is not volatile, thread A may write five times, but thread B may see those writes as being delayed (or even potentially in the wrong order).

A solution would be to lock, but you could also use volatile in this situation. This would ensure that thread B will always see the most up-to-date thing that thread A has written. Note however that this logic only works if you have writers who never read, and readers who never write, and if the thing you're writing is an atomic value. As soon as you do a single read-modify-write, you need to go to Interlocked operations or use a Lock.

Tuesday, June 1, 2021
 
VieStar
answered 7 Months ago
15

Declaring a static variable in Java, means that there will be only one copy, no matter how many objects of the class are created. The variable will be accessible even with no Objects created at all. However, threads may have locally cached values of it.

When a variable is volatile and not static, there will be one variable for each Object. So, on the surface it seems there is no difference from a normal variable but totally different from static. However, even with Object fields, a thread may cache a variable value locally.

This means that if two threads update a variable of the same Object concurrently, and the variable is not declared volatile, there could be a case in which one of the thread has in cache an old value.

Even if you access a static value through multiple threads, each thread can have its local cached copy! To avoid this you can declare the variable as static volatile and this will force the thread to read each time the global value.

However, volatile is not a substitute for proper synchronisation!
For instance:

private static volatile int counter = 0;

private void concurrentMethodWrong() {
  counter = counter + 5;
  //do something
  counter = counter - 5;
}

Executing concurrentMethodWrong concurrently many times may lead to a final value of counter different from zero!
To solve the problem, you have to implement a lock:

private static final Object counterLock = new Object();

private static volatile int counter = 0;

private void concurrentMethodRight() {
  synchronized (counterLock) {
    counter = counter + 5;
  }
  //do something
  synchronized (counterLock) {
    counter = counter - 5;
  }
}

Or use the AtomicInteger class.

Friday, June 4, 2021
 
ammezie
answered 6 Months ago
39

You should never use Thread.VolatileRead/Write(). It was a design mistake in .NET 1.1, it uses a full memory barrier. This was corrected in .NET 2.0, but they couldn't fix these methods anymore and had to add a new way to do it, provided by the System.Threading.Volatile class. Which is a class that the jitter is aware of, it replaces the methods at jit time with a version that's suitable for the specific processor type.

The comments in the source code for the Volatile class as available through the Reference Source tells the tale (edited to fit):

// Methods for accessing memory with volatile semantics.  These are preferred over 
// Thread.VolatileRead and Thread.VolatileWrite, as these are implemented more
// efficiently.
//
// (We cannot change the implementations of Thread.VolatileRead/VolatileWrite 
// without breaking code that relies on their overly-strong ordering guarantees.)
//
// The actual implementations of these methods are typically supplied by the VM at 
// JIT-time, because C# does not allow us to express a volatile read/write from/to 
// a byref arg. See getILIntrinsicImplementationForVolatile() in jitinterface.cpp.

And yes, you'll have trouble finding examples of its usage. The Reference Source is an excellent guide with megabytes of carefully written, tested and battle-scarred C# code that deals with threading. The number of times it uses VolatileRead/Write: zero.

Frankly, the .NET memory models are a mess with conflicting assumptions made by the CLR mm and C# mm with new rules added for ARM cores just recently. The weirdo semantics of the volatile keyword that means different things for different architectures is some evidence for that. Albeit that for a processor with a weak memory model you can typically assume that what the C# language spec says is accurate.

Do note that Joe Duffy has given up all hope and just flat out discourages all use of it. It is in general very unwise to assume that you can do better than the primitives provided by the language and framework. The Remarks section of the Volatile class bring the point home:

Under normal circumstances, the C# lock statement, the Visual Basic SyncLock statement, and the Monitor class provide the easiest and least error-prone way of synchronizing access to data, and the Lazy class provides a simple way to write lazy initialization code without directly using double-checked locking.

Monday, July 12, 2021
 
mschuett
answered 5 Months ago
47

Another option is to get the HashCode of each URL, then divide it by a prime number and use it as an index into an array of locks. This will limit the number of locks you need while letting you control the probability of a “false locking” by choose the number of locks to use.

However the above is only worthwhile if it is too costly just have one lock per active url.

Sunday, August 1, 2021
 
Asher
answered 4 Months ago
81

Whenever I see a question like this, my knee-jerk reaction is "assume nothing!" The .NET memory model is quite weak and the C# memory model is especially noteworthy for using language that can only apply to a processor with a weak memory model that isn't even supported anymore. Nothing what's there tells you anything what's going to happen in this code, you can reason about locks and memory barriers until you're blue in the face but you don't get anywhere with it.

The x64 jitter is quite clean and rarely throws a surprise. But its days are numbered, it is going to be replaced by Ryujit in VS2015. A rewrite that started with the x86 jitter codebase as a starting point. Which is a concern, the x86 jitter can throw you for a loop. Pun intended.

Best thing to do is to just try it and see what happens. Rewriting your code a little bit and making that loop as tight as possible so the jitter optimizer can do anything it wants:

class Test {
    public bool myBool;
    private static object myLock = new object();
    public int Foo() {
        lock (myLock) {
            int cnt = 0;
            while (!myBool) cnt++;
            return cnt;
        }
    }
}

And testing it like this:

    static void Main(string[] args) {
        var obj = new Test();
        new Thread(() => {
            Thread.Sleep(1000);
            obj.myBool = true;
        }).Start();
        Console.WriteLine(obj.Foo());
    }

Switch to the Release build. Project + Properties, Build tab, tick the "Prefer 32-bit" option. Tools + Options, Debugging, General, untick the "Suppress JIT optimization" option. First run the Debug build. Works fine, program terminates after a second. Now switch to the Release build, run and observe that it deadlocks, the loop never completes. Use Debug + Break All to see that it hangs in the loop.

To see why, look at the generated machine code with Debug + Windows + Disassembly. Focusing on the loop only:

                int cnt = 0;
013E26DD  xor         edx,edx                      ; cnt = 0
                while (myBool) {
013E26DF  movzx       eax,byte ptr [esi+4]         ; load myBool 
013E26E3  test        eax,eax                      ; myBool == true?
013E26E5  jne         013E26EC                     ; yes => bail out
013E26E7  inc         edx                          ; cnt++
013E26E8  test        eax,eax                      ; myBool == true?
013E26EA  jne         013E26E7                     ; yes => loop
                }
                return cnt;

The instruction at address 013E26E8 tells the tale. Note how the myBool variable is stored in the eax register, cnt in the edx register. A standard duty of the jitter optimizer, using the processor registers and avoiding memory loads and stores makes the code much faster. And note that when it tests the value, it still uses the register and does not reload from memory. This loop can therefore never end and it will always hang your program.

Code is pretty fake of course, nobody will ever write this. In practice this tends to work by accident, you'll have more code inside the while() loop. Too much to allow the jitter to optimize the variable way entirely. But there are no hard rules that will tell you when this happens. Sometimes it does pull it off, assume nothing. Proper synchronization should never be skipped. You really are only safe with an extra lock for myBool or an ARE/MRE or Interlocked.CompareExchange(). And if you want to cut such a volatile corner then you must check.

And noted in the comments, try Thread.VolatileRead() instead. You need to use a byte instead of a bool. It still hangs, it is not a synchronization primitive.

Friday, November 5, 2021
 
Martin Pilkington
answered 4 Weeks ago
Only authorized users can answer the question. Please sign in first, or register a free account.
Not the answer you're looking for? Browse other questions tagged :
 
Share