Read Introduction in C# - how to protect against it?

Question

An article in MSDN Magazine discusses the notion of Read Introduction and gives a code sample which can be broken by it.

public class ReadIntro {
  private Object _obj = new Object();
  void PrintObj() {
    Object obj = _obj;
    if (obj != null) {
      Console.WriteLine(obj.ToString()); // May throw a NullReferenceException
    }
  }
  void Uninitialize() {
    _obj = null;
  }
}

Notice this "May throw a NullReferenceException" comment - I never knew this was possible.

So my question is: how can I protect against read introduction?

I would also be really grateful for an explanation exactly when the compiler decides to introduce reads, because the article doesn't include it.

Answer 1

Let me try to clarify this complicated question by breaking it down.

What is "read introduction"?

"Read introduction" is an optimization whereby the code:

public static Foo foo; // I can be changed on another thread!
void DoBar() {
  Foo fooLocal = foo;
  if (fooLocal != null) fooLocal.Bar();
}

is optimized by eliminating the local variable. The compiler can reason that if there is only one thread then foo and fooLocal are the same thing. The compiler is explicitly permitted to make any optimization that would be invisible on a single thread, even if it becomes visible in a multithreaded scenario. The compiler is therefore permitted to rewrite this as:

void DoBar() {
  if (foo != null) foo.Bar();
}

And now there is a race condition. If foo turns from non-null to null after the check then it is possible that foo is read a second time, and the second time it could be null, which would then crash. From the perspective of the person diagnosing the crash dump this would be completely mysterious.

Can this actually happen?

As the article you linked to called out:

Note that you won’t be able to reproduce the NullReferenceException using this code sample in the .NET Framework 4.5 on x86-x64. Read introduction is very difficult to reproduce in the .NET Framework 4.5, but it does nevertheless occur in certain special circumstances.

x86/x64 chips have a "strong" memory model and the jit compilers are not aggressive in this area; they will not do this optimization.

If you happen to be running your code on a weak memory model processor, like an ARM chip, then all bets are off.

When you say "the compiler" which compiler do you mean?

I mean the jit compiler. The C# compiler never introduces reads in this manner. (It is permitted to, but in practice it never does.)

Isn't it a bad practice to be sharing memory between threads without memory barriers?

Yes. Something should be done here to introduce a memory barrier because the value of foo could already be a stale cached value in the processor cache. My preference for introducing a memory barrier is to use a lock. You could also make the field volatile, or use VolatileRead, or use one of the Interlocked methods. All of those introduce a memory barrier. (volatile introduces only a "half fence" FYI.)

Just because there's a memory barrier does not necessarily mean that read introduction optimizations are not performed. However, the jitter is far less aggressive about pursuing optimizations that affect code that contains a memory barrier.

Are there other dangers to this pattern?

Sure! Let's suppose there are no read introductions. You still have a race condition. What if another thread sets foo to null after the check, and also modifies global state that Bar is going to consume? Now you have two threads, one of which believes that foo is not null and the global state is OK for a call to Bar, and another thread which believes the opposite, and you're running Bar. This is a recipe for disaster.

So what's the best practice here?

First, do not share memory across threads. This whole idea that there are two threads of control inside the main line of your program is just crazy to begin with. It never should have been a thing in the first place. Use threads as lightweight processes; give them an independent task to perform that does not interact with the memory of the main line of the program at all, and just use them to farm out computationally intensive work.

Second, if you are going to share memory across threads then use locks to serialize access to that memory. Locks are cheap if they are not contended, and if you have contention, then fix that problem. Low-lock and no-lock solutions are notoriously difficult to get right.

Third, if you are going to share memory across threads then every single method you call that involves that shared memory must either be robust in the face of race conditions, or the races must be eliminated. That is a heavy burden to bear, and that is why you shouldn't go there in the first place.

My point is: read introductions are scary but frankly they are the least of your worries if you are writing code that blithely shares memory across threads. There are a thousand and one other things to worry about first.

Answer 2

You cant really "protect" against read introduction as it's a compiler optimization (excepting using Debug builds with no optimization of course). It's pretty well documented that the optimizer will maintain the single-threaded semantics of the function, which as the article notes can cause issues in multi-threaded situations.

That said, I'm confused by his example. In Jeffrey Richter's book CLR via C# (v3 in this case), in the Events section he covers this pattern, and notes that in the example snippet you have above, in THEORY it wouldn't work. But, it was a recommended pattern by Microsoft early in .Net's existence, and therefore the JIT compiler people he spoke to said that they would have to make sure that sort of snippet never breaks. (It's always possible they may decide that it's worth breaking for some reason though - I imagine Eric Lippert could shed light on that).

Finally, unlike the article, Jeffrey offers the "proper" way to handle this in multi-threaded situations (I've modified his example with your sample code):

Object temp = Interlocked.CompareExchange(ref _obj, null, null);
if(temp != null)
{
    Console.WriteLine(temp.ToString());
}

Answer 3

I only skimmed the article, but it seems that what the author is looking for is that you need to declare the _obj member as volatile.