26

Consider the following sample code:

class MyClass
{
    public long x;

    public void DoWork()
    {
        switch (x)
        {
            case 0xFF00000000L:
                // do whatever...
                break;

            case 0xFFL:
                // do whatever...
                break;

            default:
                //notify that something going wrong
                throw new Exception();
        }
    }
}

Forget the uselessness of the snippet: my doubt is about the behavior of the switch statement.

Suppose that the x field could have only two values: 0xFF00000000L or 0xFFL. The switch above should not fall into the "default" option.

Now imagine that one thread is executing the switch with "x" equal to 0xFFL, thus the first condition won't match. At the same time, another thread modifies the "x" variable to 0xFF00000000L. We know a 64-bit operation is not atomic, so that the variable will have the lower dword zeroed first, then the upper set afterward (or vice versa).

If the second condition in the switch will be done when the "x" is zero (i.e. during the new assignment), will we fall into the undesired "default" case?

VMAtm
  • 27,943
  • 17
  • 79
  • 125
Mario Vernari
  • 6,649
  • 1
  • 32
  • 44
  • I don't have actual facts to back up my opinion, but I'm 99.9% sure that it's not thread-safe – Dyppl Jul 11 '11 at 08:50
  • 1
    It isn't specific to switch, *any* statement that uses the variable may observe it in a temporary inconsistent state. – Hans Passant Jul 11 '11 at 09:10
  • Yes, very dodgy. Whenever I see a class like this approaching me, I try to create an instance per calling thread & so avoid the issue, (yes, not always possible, depending on functionality and data). – Martin James Jul 11 '11 at 12:01

4 Answers4

17

Yes, the switch statement itself, as shown in your question, is thread-safe. The value of field x is loaded once into a (hidden) local variable and that local is used for the switch block.

What isn't safe is the initial load of the field x into a local variable. 64-bit reads aren't guaranteed to be atomic, so you could be getting stale and/or torn reads at that point. This could easily be resolved by using Interlocked.Read, or similar, to explicitly read the field value into the local in a thread-safe way:

long y = Interlocked.Read(ref x);
switch (y)
{
    // ...
}
LukeH
  • 263,068
  • 57
  • 365
  • 409
12

You're actually posting two questions.

Is it threadsafe?

Well, obviously it is not, another thread might change the value of X while the first thread is going into the switch. Since there's no lock and the variable is not volatile you'll switch based on the wrong value.

Would you ever hit the default state of the switch?

Theoretically you might, as updating a 64 bits is not an atomic operation and thus you could theoretically jump in the middle of the assignment and get a mingled result for x as you point out. This statistically won't happen often but it WILL happen eventually.

But the switch itself is threadsafe, what's not threadsafe is read and writes over 64 bit variables (in a 32 bit OS).

Imagine instead of switch(x) you have the following code:

long myLocal = x;
switch(myLocal)
{
}

now the switch is made over a local variable, and thus, it's completely threadsafe. The problem, of course, is in the myLocal = x read and its conflict with other assignments.

matt b
  • 138,234
  • 66
  • 282
  • 345
Jorge Córdoba
  • 51,063
  • 11
  • 80
  • 130
  • 3
    Your "...imagine instead of..." code is pretty much exactly what the compiler generates for a `switch` block anyway. The switch-expression -- loading from field `x`, in this case -- is only performed once and the result stored in a local; that local is then used for the switch block. – LukeH Jul 11 '11 at 09:22
  • Maybe it is because I am not so fluent in English, but I know the snippet is *far* from being thread-safe. The thread-safety was only focused around the "switch" statement. Having a peek with IL-disass, I see that there is a copy of the value, then compared with every test. This is much like the assembler code would do. – Mario Vernari Jul 11 '11 at 09:29
  • 2
    @Mario Vernari: You've proven that a compiler _can_ generate code like that, but not that it will always do so. Real-world compilers are influenced by factors such as number of switch statement, the value thereof, the number of variables used and the memory pressure resulting from it, etc. The disassembly you see is therefore not representative of all situations. – MSalters Jul 11 '11 at 10:11
  • 2
    @MSalters: The switch-expression is only evaluated once according to the [ECMA C# spec](http://www.ecma-international.org/publications/standards/Ecma-334.htm) (section 15.7.2 if you're interested). – LukeH Jul 11 '11 at 10:29
  • @LukeH: That's indeed the proper way to show what a compiler _must_ do, instead of merely showing what it _can_ do. But note that 10.10 Execution Order gives quite a bit of freedom to reorder the evaluation of non-volatile expressions. Evaluating a non-volatile switch expression is not a side effect. – MSalters Jul 11 '11 at 10:38
  • @Mario Vernari: if you turn on optimization, then the compiler might decide not to cache the variable; or some future version might add that optimization. Even if it works for now, unless it is guaranteed that the compiler will cache the variable, then it is not. – Lie Ryan Jul 11 '11 at 17:15
  • @LukeH So what you're saying then is that this answer is wrong; the default case will *never* be hit, right? – Asad Saeeduddin Apr 19 '14 at 04:41
  • @Asad: No, that's not what I'm saying. If you're unlucky, the default case *could* be hit because the initial 64-bit read from field `x` is not guaranteed to be thread-safe. – LukeH Apr 23 '14 at 23:07
2

C#'s switch statement isn't evaluated as a series of if conditions (as VB's can be). C# effectively builds up a hashtable of labels to jump to based on the value of the object and jumps straight to the correct label, rather than iterating through each condition in turn and evaluating it.

This is why a C# switch statement doesn't deteriorate in terms of speed as you increase the number of cases. And it's also why C# is more restrictive with what you can compare to in the switch cases than VB, in which you can do ranges of values, for example.

Therefore you don't have the potential race condition that you've stated, where a comparison is made, the value changes, the second comparison is made, etc, because there is only one check performed. As for whether it's totally threadsafe - I wouldn't assume so.

Have a dig with reflector looking through a C# switch statement in IL and you'll see what's happening. Compare it to a switch statement from VB which includes ranges in the values and you'll see a difference.

It's been quite a few years since I looked at it, so things may have changed slightly...

See more detail about switch statement behaviour here: Is there any significant difference between using if/else and switch-case in C#?

Community
  • 1
  • 1
Niall Connaughton
  • 15,518
  • 10
  • 52
  • 48
  • 1
    Some answers to [this question](http://stackoverflow.com/questions/6647257/i-am-a-seo-beginner-what-to-do) claim a switch might be implemented as a series of if-else statements, jump table or otherwise. Have no idea myself, though... – Eran Jul 11 '11 at 09:01
  • 1
    @eran well the link is broken – V4Vendetta Jul 11 '11 at 09:04
  • 1
    @Niall, @eran: Yep there's a certain threshold below which the compiler just uses a bunch of if conditions. – LukeH Jul 11 '11 at 09:08
  • 1
    I think the ECMA standard does in no way dictate how switch statements should be compiled into CLR instructions. JIT-ters may use heuristics to optimize this, and a jump table might be used only in specific cases (large switches being the more obvious case) – sehe Jul 11 '11 at 09:09
  • @V4Vendetta thanks, don't know how that got there... The right link: [C# switch statement limitations - why?](http://stackoverflow.com/questions/44905/c-switch-statement-limitations-why) – Eran Jul 11 '11 at 09:18
1

As you already assumed, the switch statement is not thread-safe and might fail in certain scenarios.

Furthermore, using lock on your instance variable won't work neither, because the lock statement expects an object causing your instance variable to be boxed. Every time the instance variable is boxed, a new boxed variable will be created, making the lock effectively useless.

In my opinion you have several options solving this issue.

  1. Use lock on a private instance variable of any reference type (object will do the job)
  2. Use ReaderWriterLockSlim to let multiple threads read the instance variable but only one thread write the instance variable at a time.
  3. Atomically store the value of the instance variable to a local variable in your method (e.g. using Interlocked.Read or Interlocked.Exchange) and perform the switch on the local variable. Note that this way you might use the old value for the switch. You have to decide if this can cause problems in your concrete use-case.
Florian Greinacher
  • 14,478
  • 1
  • 35
  • 53