5

(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

leviathanbadger
  • 1,682
  • 15
  • 23
  • In C# as well as Java? Is it threadsafe otherwise? – leviathanbadger May 09 '13 at 13:14
  • 1
    "Once it has been observed to be true" is the bit that may never happen if you don't use `volatile`. – Damien_The_Unbeliever May 09 '13 at 13:41
  • 1
    Why not just use the background worker's `IsBusy` property to determine if it's finished? I would imagine they ensured that it can safely be accessed from any thread. – Servy May 09 '13 at 14:43
  • @Servy I wasn't really asking about `BackgroundWorker` - that was just for demonstration. The point was that `completed` is set in a different thread, not necessarily at the end of the thread's lifetime. – leviathanbadger May 28 '13 at 03:12
  • @aboveyou00 And my point is that this is specifically why classes such as `BackgroundWorker` exist; to help manage the synchronization between threads from the most common use cases. You should learn to leverage that. – Servy May 28 '13 at 13:47

2 Answers2

4

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.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Does volatile guarantee that the field is directly stored and always re-loaded from memory? (Both properties are required to make this work). I'm unclear on what guarantees volatile actually does provide. – usr May 09 '13 at 14:21
  • @usr: You need another property as well -- that it isn't written early or speculatively. – Ben Voigt May 09 '13 at 16:01
  • 2
    +1 Your last paragraph sums it up perfectly. You shouldn't have to ask if something is thread-safe. Use the synchronization object and *know*. – Jim Mischel May 09 '13 at 18:08
3

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

Martin Mulder
  • 12,642
  • 3
  • 25
  • 54
  • ...but all bets are off if you explicitly align the field using `[FieldOffset]` or `[StructLayout]`. See [this question](http://stackoverflow.com/q/15249626/146622). – Daniel A.A. Pelsmaeker May 09 '13 at 14:15
  • 1
    Does this guarantee that a write to that field propagates to the other thread(s)? I don't think so. The value might be enregistered. – usr May 09 '13 at 14:19
  • 1
    The write to `completed` might be atomic and "safe" by itself, but it might never be seen by `Update` if `Update` caches the value of `completed` in a register (or does other transformations that have the same effect). – usr May 09 '13 at 14:27
  • @usr: You are correct. That is why the programmer should use `volatile` to prevent caching. – Martin Mulder May 09 '13 at 14:28
  • @virtlink: Nice one! Still I do not think a task switch will occur. Setting a boolean field will result in one MOV-operation (x86 CPU's). If the allignment of the field is not on a native word boundry, the processor has to make two writes operations into memory, but I do not think hat a task switch will occur between those two writes. If a task switch WOULD occur (in case if I am wrong, or perhaps on anohter CPU type), the programmer stil does not have a problem since he is only modifying one (!!!) bit (since the other 31 bits remain the same). And you cannot get more atomic than one bit! :) – Martin Mulder May 09 '13 at 14:34
  • No, you can get a task/context switch in between instructions. The reason is that the processor (including x86 processors) get a timer _interrupt_ every so often. The OS decides that after X timer interrupts the current task has had its share of the processor time, stores the current values of the registers, load the registers of another task, and continues with that task. Such a timer interrupt can very well occur in between any two instructions, unless the OS has disabled interrupts. The OS almost never disables interrupts, and programs _can't_ disable interrupts. – Daniel A.A. Pelsmaeker May 09 '13 at 14:43
  • And, booleans are _not_ guaranteed to be just 0 or 1 (for bit 0). Actually, any non-zero integer value is boolean `true`. For example, [VB.NET's boolean is](http://stackoverflow.com/q/3621037/146622) -1, or 0xFFFFFFFF. So when a task switch occurs between the two `mov` instructions, it is possible to end up with 0xFF000000 or 0x00FFFFFF depending on how the boolean is aligned and the order in which the two `mov` instructions write the boolean's value. – Daniel A.A. Pelsmaeker May 09 '13 at 14:46
  • 1
    @virtlink: This is not 100% correct. All boolean `True` are the same: They are all bitwise equal to new Int32(1). Otherwise a `True` from a VB.NET-library would not be equal to a `True` from a C#.Library when using `Object.Equals`. I just tried it with a struct with explicit layout having a bool and an int overlap each other. So the OP is in fact still modifying one bit. BUt... i case you would be right, if any bit in the boolean would have been set as part of a two-write operation, the `Update` method still would have receive the correct signal. – Martin Mulder May 09 '13 at 15:02
  • @virtlink: I posted an answer at the question you gave me (http://stackoverflow.com/questions/3621037/casting-a-boolean-to-an-integer-returns-1-for-true). Perhapse this clearifies more. – Martin Mulder May 09 '13 at 15:17
  • Yes, I was just testing the same and came to the same conclusion! And... if you use your struct and assign `CInt(true)` to the integer instead, then even when both booleans are `true`, they don't compare equal. – Daniel A.A. Pelsmaeker May 09 '13 at 15:20
  • @Virtlink: Correct. `object.Equals` does a bitwise comparison. – Martin Mulder May 09 '13 at 15:22