(Note: I already asked this question, but the answer was specific to Java, and so I am asking the same question for C# and the .NET framework. It is NOT a duplicate.)
I have been using this pattern for a while, but I only recently came to think that it might not be OK to do this. Basically, I use some variant of this pattern:
public class SampleAsync
{
public SampleAsync() { }
private bool completed;
public void Start()
{
var worker = new BackgroundWorker();
worker.DoWork += (sender, e) => {
//... do something on a different thread
completed = true;
};
worker.RunWorkerAsync();
}
public void Update()
{
if (!completed) return;
//... do something else
}
}
*The user is responsible for making sure Start
is only called once. Update
is called wherever and whenever.
I've always assumed this is threadsafe in C#/the .NET framework, because even though nothing is strictly synchronized, I only ever set completed
to true. Once it has been observed to be true
, it will not reset to false
. It is initialized to false in the constructor, which is by definition thread safe (unless you do something stupid in it). So, is it thread safe to use unresettable flags in this way? (And if so, does it even provide any performance benefits?)
Thanks
It is heavily dependent on the target architecture. Intel processors have a strong memory model so you tend to get away with code like this. But then the jitter may well screw you up. The x86 jitter for example is apt to store the variable in a cpu register, particularly when the if() statement appears in a tight loop. And only does so in the Release build, awesome debugging nightmare. Declaring the variable volatile is a band-aid for that. The x64 jitter doesn't need that, at least in its current version. But band-aids tend to not stop the bleeding on processors with a weak memory model, like ARM and Itanium. They certainly don't promise that the updated state of the variable is visible in another thread any time soon. The thread scheduler tends to get the cpu caches flushed. Eventually.
There's just no point in not doing this correctly. Use a proper synchronization object, like AutoResetEvent. Or Interlocked.CompareExchange() if you fret about cycles.
Your code is thread safe, because bool
is an atomic type.
MSDN:
Reads and writes of the following data types are atomic: bool, char, byte, sbyte, short, ushort, uint, int, float, and reference types. In addition, reads and writes of enum types with an underlying type in the previous list are also atomic. Reads and writes of other types, including long, ulong, double, and decimal, as well as user-defined types, are not guaranteed to be atomic. Aside from the library functions designed for that purpose, there is no guarantee of atomic read-modify-write, such as in the case of increment or decrement.
See: http://msdn.microsoft.com/en-us/library/aa691278(v=vs.71).aspx
Please mark your field with volatile
:
private volatile bool completed;
MSDN:
The volatile keyword indicates that a field can be modified in the program by something such as the operating system, the hardware, or a concurrently executing thread.
See: http://msdn.microsoft.com/en-us/library/x13ttww7(v=vs.71).aspx
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With