Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is the standard C# event invocation pattern thread-safe without a memory barrier or cache invalidation? What about similar code?

In C#, this is the standard code for invoking an event in a thread-safe way:

var handler = SomethingHappened;
if(handler != null)
    handler(this, e);

Where, potentially on another thread, the compiler-generated add method uses Delegate.Combine to create a new multicast delegate instance, which it then sets on the compiler-generated field (using interlocked compare-exchange).

(Note: for the purposes of this question, we don't care about code that runs in the event subscribers. Assume that it's thread-safe and robust in the face of removal.)


In my own code, I want to do something similar, along these lines:

var localFoo = this.memberFoo;
if(localFoo != null)
    localFoo.Bar(localFoo.baz);

Where this.memberFoo could be set by another thread. (It's just one thread, so I don't think it needs to be interlocked - but maybe there's a side-effect here?)

(And, obviously, assume that Foo is "immutable enough" that we're not actively modifying it while it is in use on this thread.)


Now I understand the obvious reason that this is thread-safe: reads from reference fields are atomic. Copying to a local ensures we don't get two different values. (Apparently only guaranteed from .NET 2.0, but I assume it's safe in any sane .NET implementation?)


But what I don't understand is: What about the memory occupied by the object instance that is being referenced? Particularly in regards to cache coherency? If a "writer" thread does this on one CPU:

thing.memberFoo = new Foo(1234);

What guarantees that the memory where the new Foo is allocated doesn't happen to be in the cache of the CPU the "reader" is running on, with uninitialized values? What ensures that localFoo.baz (above) doesn't read garbage? (And how well guaranteed is this across platforms? On Mono? On ARM?)

And what if the newly created foo happens to come from a pool?

thing.memberFoo = FooPool.Get().Reset(1234);

This seems no different, from a memory perspective, to a fresh allocation - but maybe the .NET allocator does some magic to make the first case work?


My thinking, in asking this, is that a memory barrier would be required to ensure - not so much that memory accesses cannot be moved around, given the read is dependent - but as a signal to the CPU to flush any cache invalidations.

My source for this is Wikipedia, so make of that what you will.

(I might speculate that maybe the interlocked-compare-exchange on the writer thread invalidates the cache on the reader? Or maybe all reads cause invalidation? Or pointer dereferences cause invalidation? I'm particularly concerned how platform-specific these things sound.)


Update: Just to make it more explicit that the question is about CPU cache invalidation and what guarantees .NET provides (and how those guarantees might depend on CPU architecture):

  • Say we have a reference stored in field Q (a memory location).
  • On CPU A (writer) we initialize an object at memory location R, and write a reference to R into Q
  • On CPU B (reader), we dereference field Q, and get back memory location R
  • Then, on CPU B, we read a value from R

Assume the GC does not run at any point. Nothing else interesting happens.

Question: What prevents R from being in B's cache, from before A has modified it during initialisation, such that when B reads from R it gets stale values, in spite of it getting a fresh version of Q to know where R is in the first place?

(Alternate wording: what makes the modification to R visible to CPU B at or before the point that the change to Q is visible to CPU B.)

(And does this only apply to memory allocated with new, or to any memory?)+


Note: I've posted a self-answer here.

like image 761
Andrew Russell Avatar asked Jun 10 '15 14:06

Andrew Russell


1 Answers

This is a really good question. Let us consider your first example.

var handler = SomethingHappened;
if(handler != null)
    handler(this, e);

Why is this safe? To answer that question you first have to define what you mean by "safe". Is it safe from a NullReferenceException? Yes, it is pretty trivial to see that caching the delegate reference locally eliminates that pesky race between the null check and the invocation. Is it safe to have more than one thread touching the delegate? Yes, delegates are immutable so there is no way that one thread can cause the delegate to get into a half-baked state. The first two are obvious. But, what about a scenario where thread A is doing this invocation in a loop and thread B at some later point in time assigns the first event handler? Is that safe in the sense that thread A will eventually see a non-null value for the delegate? The somewhat surprising answer to this is probably. The reason is that the default implementations of the add and remove accessors for the event create memory barriers. I believe the early version of the CLR took an explicit lock and later versions used Interlocked.CompareExchange. If you implemented your own accessors and omitted a memory barrier then the answer could be no. I think in reality it highly depends on whether Microsoft added memory barriers to the construction of the multicast delegate itself.

On to the second and more interesting example.

var localFoo = this.memberFoo;
if(localFoo != null)
    localFoo.Bar(localFoo.baz);

Nope. Sorry, this actually is not safe. Let us assume memberFoo is of type Foo which is defined like the following.

public class Foo
{
  public int baz = 0;
  public int daz = 0;

  public Foo()
  {
    baz = 5;
    daz = 10;
  }

  public void Bar(int x)
  {
    x / daz;
  }
}

And then let us assume another thread does the following.

this.memberFoo = new Foo();

Despite what some may think there is nothing that mandates that instructions have to be executed in the order that they were defined in the code as long as the intent of the programmer is logically preserved. The C# or JIT compilers could actually formulate the following sequence of instructions.

/* 1 */ set register = alloc-memory-and-return-reference(typeof(Foo));
/* 2 */ set register.baz = 0;
/* 3 */ set register.daz = 0;
/* 4 */ set this.memberFoo = register;
/* 5 */ set register.baz = 5;  // Foo.ctor
/* 6 */ set register.daz = 10; // Foo.ctor

Notice how the assignment to memberFoo occurs before the constructor is run. That is valid because it does not have any unintended side-effects from the perspective of the thread executing it. It could, however, have a major impact on other threads. What happens if your null check of memberFoo on the reading thread occurred when the writing thread just fininished instruction #4? The reader will see a non-null value and then attempt to invoke Bar before the daz variable got set to 10. daz will still hold its default value of 0 thus leading to a divide by zero error. Of course, this is mostly theoretical because Microsoft's implementation of the CLR creates a release-fence on writes that would prevent this. But, the specification would technically allow for it. See this question for related content.

like image 154
Brian Gideon Avatar answered Sep 22 '22 17:09

Brian Gideon