Is it safe to replace Volatile.Write() with simple assignment in a method that contains only that statement?
Asked Answered
F

1

3

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.

Fore answered 29/1, 2022 at 15:14 Comment(2)
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.Erose
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.Fore
A
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.

Azilian answered 29/1, 2022 at 19:13 Comment(5)
How would you implement the release method with Interlocked.Exchange, and what difference would it make for the users of the acquire/release API?Erose
@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 answerAzilian
According to this 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.Erose
Could be. I'm unsure how it interacts with Interlocked.CompareExchange which does do a full barrierAzilian
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.Erose

© 2022 - 2024 — McMap. All rights reserved.