2

This is an advanced question in C# multithreading.

Let's say I have this code that is used as a locking mechanism to enable only one thread to start some operation:

private static int _guard = 0;

private static bool acquire() {
    return Interlocked.CompareExchange(ref _guard, 1, 0) == 0;
}

private static void release() {
    Volatile.Write(ref _guard, 0);
}

This lock is used to protect method that should be execute only by one thread at the time:

public readonly Status Status = new (); // updated from thread that runs someTask

void TryRunningTask {
    if (acquire()) {
        return await someTask();
    } else {
        InfoMessage = "Another user is currently running someTask.";
    }
}

My question is, if I change release() as following:

private static void release() {
    _guard = 0;
}

Would the program still behave completely the same? Would that break the thread-safety? Would this change make any sense?

The reasoning behind my idea for this change is following:

  1. there are no other read/write operations inside of release() method. In MS Docs for Volatile.Write method it says:

Writes a value to a field. On systems that require it, inserts a memory barrier that prevents the processor from reordering memory operations as follows: If a read or write appears before this method in the code, the processor cannot move it after this method.

So because my release() method has no other operations before/after Volatile.Write() call I guess I can just replace it with simple assignment statement _guard = 0; right?

  1. as per C# Standard section 10.6 the _guard = 0; operation is guaranteed to be atomic operation:

10.6 Atomicity of variable references

Reads and writes of the following data types shall be atomic: bool, char, byte, sbyte, short, ushort, uint, int, float, and reference types.

ProgrammingLlama
  • 36,677
  • 7
  • 67
  • 86
mlst
  • 2,688
  • 7
  • 27
  • 57
  • As a side note, why aren't you using a `SemaphoreSlim`? `bool Acquire() => semaphore.Wait(0);`, `void Release() => semaphore.Release();` Using a built-in synchronization primitive should be safer than inventing your own. – Theodor Zoulias Jan 29 '22 at 16:08
  • 1
    I saw this code some in project that is already in production. They use this kind of locking but I didn't really understand why. So I am just trying to understand underlying principle and what exactly is going on here. In my projects I will probably go with `SemaphoreSlim`. – mlst Jan 29 '22 at 17:31

1 Answers1

3

No, you cannot remove the call to Volatile.Write.

You are correct in regards the atomicity point: C# and the CLR mandates that 32-bit and smaller data types shall be atomic.

However, there is not just atomicity to consider. There is also instruction reordering and processor caching to consider.

Reordering may happen by the CLR Jitter, and the size of your function is not relevant, as it may be inlined into any function which calls it (and probably will be given it's short).

The processor also may reorder instructions, if it conforms to the instructions it has been given.

So this needs a memory barrier to be thread-safe.

Processor caching is another issue: if the processor core is not told that a read or write is volatile, it could just use its own cache and ignore an what is in other cores' caches.

But, Volatile.Write may not be enough anyway. I can't tell exactly from what you have shown, but it seems you have multiple threads which read and write. I think you should therefore use Interlocked.Exchange instead.

Charlieface
  • 52,284
  • 6
  • 19
  • 43
  • How would you implement the `release` method with `Interlocked.Exchange`, and [what difference](https://stackoverflow.com/questions/12425738/difference-between-interlocked-exchange-and-volatile-write/12494287) would it make for the users of the `acquire`/`release` API? – Theodor Zoulias Jan 29 '22 at 22:39
  • @TheodorZoulias `Interlocked.Exchange(ref _guard, 0);` It helps if there are threads that both read and write, because it induces a full memory barrier (as opposed to `Volatile` which is only a half-barrier. At least that's my understanding of it, if I've got it wrong please advise, I'd be happy to change my answer – Charlieface Jan 30 '22 at 00:27
  • According to [this](https://stackoverflow.com/questions/12425738/difference-between-interlocked-exchange-and-volatile-write/71002288#71002288) recent answer, the `Volatile.Write` is always preferable to the `Interlocked.Exchange`, unless you also want to get the previous value as an atomic operation. The `release()` method in the OP's locking mechanism does not bother returning the previous state of the lock, so there is no reason to use the relatively more expensive `Interlocked.Exchange`. – Theodor Zoulias Feb 05 '22 at 22:03
  • Could be. I'm unsure how it interacts with `Interlocked.CompareExchange` which does do a full barrier – Charlieface Feb 06 '22 at 01:08
  • Is there any way that we can compare experimentally those two APIs? I mean, if one API is superior to the other, a properly conducted experiment should be able to demonstrate its superiority. – Theodor Zoulias Feb 06 '22 at 13:06