1

I want to create boolean flag that is both thread-safe and non-blocking on both 32 bit and 64 bit systems. Does this solution satisfy those 2 requirements? If not, what will?

public bool Flag {
    get {
        return Interlocked.Read( ref _flag ) == 1;
    }
    set {
        if ( value ) {
            // set flag to "true" if it's false
            Interlocked.CompareExchange( ref _flag, 1, 0 );
        }
        else {
            // set flag to "false" if it's true
            Interlocked.CompareExchange( ref _flag, 0, 1 );
        }
    }
}
private Int32 _flag;

(edited) to use Int32 via. @M.kazemAkhgary

Example: 3 timers, each with intentionally overlapping tick intervals for this example. 2 timer callbacks are reading the flag. 1 timer callback reads and writes to the flag.

// Inline function
void Callback( object msg ) {
    if ( Flag ) Debug.WriteLine( (string) msg );
}

// Checks Flag
var timer1 = new Timer( 
    Callback,
    "Tick", // callback arg
    0,      // start timer now
    100     // callback interval ms
);

// Checks Flag
var timer2 = new Timer( 
    Callback, 
    "Tock", // callback arg
    0,      // start timer now
    20      // callback interval ms
);

// Toggles Flag every second
var timer3 = new Timer( 
    _ => {
        Flag = !Flag;
    }, 
    null, // callback arg
    0,    // start timer now
    1000  // callback interval ms
);
Casey Anderson
  • 313
  • 3
  • 12
  • 1
    Whether or not that's appropriate is going to depend on how you use it, and what you actually expect it to do (and not do). – Servy Oct 11 '17 at 21:51
  • Define "safely". No thread will crash, if that's what you mean. If you mean "if any thread sets the flag to X at time T, will any thread reading the flag at time T + 1 read X", then [we don't really know](https://stackoverflow.com/q/6139699/4137916). – Jeroen Mostert Oct 11 '17 at 21:56
  • @JeroenMostert It's a private flag and only accessed via Interlocked methods. As such, I'm not sure the link you posted is applicable as there are no other actors trying a read on the (private) field. – spender Oct 11 '17 at 22:03
  • @spender: Private vs. public is completely irrelevant, as far as concurrent operations are concerned. The only thing that matters is whether multiple threads are involved, which the question posits there are. – Jeroen Mostert Oct 11 '17 at 22:04
  • 1
    It is not invalid, but very inefficient due to the full memory barrier and the pain of making Int64 work in 32-bit code. Use Thread.VolatileRead/Write(). Int32 will do. ManualResetEventSlim is likely the better way since it is waitable. – Hans Passant Oct 11 '17 at 22:04
  • 1
    you don't need interlocked exchange. read write for fields of primitive types and reference types are atomic. – M.kazem Akhgary Oct 11 '17 at 22:05
  • @JeroenMostert : In a comment below the answer you shared "Interlocked methods are safe and they always operate with fresh values. However, if you just read the variable value elsewhere, the value you get is not necessarily the same that you have updated with the Interlocked methods" There is no "reading elsewhere" because the access to the flag is private. – spender Oct 11 '17 at 22:07
  • 1
    @spender: that comment isn't what I was referring to. That is also correct, but not the issue here. The issue is whether `Interlocked` is enough, or you need `volatile`, or maybe neither, or both. The answers and the comments (and all the online material I've ever read, even by authorities) have a devil of a time agreeing on this stuff. I'm not questioning whether reading it without *any* synchronization would be correct, that's much simpler to settle. – Jeroen Mostert Oct 11 '17 at 22:10
  • @JeroenMostert Agreed. A constant source of confusion. I cant remember the last time I relied on code like this because it is confusing to verify. Most of the time, higher level (and well-tested) abstractions (such as Task) take care of these details for you. – spender Oct 11 '17 at 22:14
  • why are you using `Int64` for flag? you should provide more detail in your question. – M.kazem Akhgary Oct 11 '17 at 22:16
  • @M.kazemAkhgary, `Interlocked` methods do more than just read and write stuff. They also insert fences. And `Int64` reads/writes are definitely not atomic on x86, although why `Int64` is even used here is beyond me. – Kirill Shlenskiy Oct 11 '17 at 22:53
  • "both thread-safe and non-blocking"? No - you must choose one or the other, unless you can accept stale (or possibly corrupt) data. – Enigmativity Oct 12 '17 at 00:14
  • @M.kazemAkhgary You're assuming that all that matters is that the code is atomic, and nothing else. That's quite likely a false assumption, although since the question doesn't mention anything about how the code is used or what it needs to do, there really is no way of knowing for sure. Odds are though that atomicity isn't enough. – Servy Oct 12 '17 at 13:07
  • @M.kazemAkhgary Updated with an example and added clarification of 32/64 bit requirements. According to the [MSDN on Interlocked](https://msdn.microsoft.com/en-us/library/yss0xc74(v=vs.110).aspx), it says "The compare and exchange operations are performed as an atomic operation." Because this happens with 1 atomic transaction, is this not thread-safe and non-blocking? – Casey Anderson Oct 12 '17 at 18:00
  • Just because it is atomic does not mean it is non-blocking. It makes the operation atomic by one of two methods, either waiting for the resource to become available, taking a exclusive lock, then doing it's operation, or it does the operation, then checks after the operation is done if it got the expected value, if not it does the operation again after waiting for a small period of time. Both of those techniques require blocking on the method till it can complete it's action. – Scott Chamberlain Oct 12 '17 at 20:13
  • @ScottChamberlain: It looks like [Interlocked](https://msdn.microsoft.com/en-us/library/yss0xc74(v=vs.110).aspx) is non-blocking and atomic according to this [source](http://www.albahari.com/threading/part4.aspx): "Interlocked’s methods have a typical overhead of 10 ns — half that of an uncontended lock. Further, they can never suffer the additional cost of context switching due to blocking. The flip side is that using Interlocked within a loop with many iterations can be less efficient than obtaining a single lock around the loop (although Interlocked enables greater concurrency)." – Casey Anderson Oct 12 '17 at 20:42

1 Answers1

0

You're close but check MSDN documentation on CompareExchange: https://msdn.microsoft.com/en-us/library/801kt583(v=vs.110).aspx

First of all, you can use an int instead of Int64 to keep the state of your flag, and reading an int is an atomic operation. Only reading an Int64 on a 32bit machine is not atomic.

Your code should be something like: (i haven't tested it)

public bool Flag {
get {
    return _flag == 1; // no Interlocked.Read here, reading an int from memory is an atomic operation
}
set {
    if ( value ) {
        // set flag to "true" if it's false

        int newFlagValue = 1;
        int initialFlagValue;
        do {
        // Save the current flag value in a local variable.
        initialFlagValue = _flag; 
        } while (initialFlagValue != Interlocked.CompareExchange(
            ref _flag, newFlagValue, initialFlagValue));            
    }
    else {
        // similar as above.
    }
}
}
private int _flag;

Update:

Regarding the use of volatile, there are a lot of miss usages of it, so I would try to avoid it. Check this great post: When should the volatile keyword be used in C#?

Dan Dinu
  • 32,492
  • 24
  • 78
  • 114
  • 2
    "Trying to avoid it" is fine and all, except that by not using it (nor any kind of memory barrier) you've lost any guarantees that the read of `_flag` won't be cached (possibly indefinitely) as a (thread-)local value, if the local code never writes to it. This is not an actual optimization the JIT will perform on x86 or x64 (I *think*), but it is at least theoretically permitted and valid (I *think*), unless you indicate that you really would like the compiler not to consider that. I think that a `Volatile.Read` would not be amiss. – Jeroen Mostert Oct 12 '17 at 00:08