A common pattern for raising an event in C# is the following code:
EventHandler handler = MyEvent;
if (handler != null)
handler(this, EventArgs.Empty);
A warning associated with this example (that we have helped spread) is that the code is subject to potentially unsafe JIT optimizations. The ultimate source for this seems to be Juval Lowy’s book, Programming .NET Components, 2nd Edition (p250):
By copying the delegate to a temporary variable, you keep a copy of the original state of the delegate, irrespective of any thread context switches. Unfortunately, however, the JIT compiler may optimize the code, eliminate the temporary variable, and use the original delegate directly. That puts you back where you started, susceptible to the race condition.
The subject may have been first raised by a post on GrantRi’s weblog about
AMD64 JIT
optimisations.
He wrote about Hashtable, but his logic is applicable to this example. He
stated that the AMD64 JIT could legally ignore a local (handler
) in the code
above, and simply perform two reads of a field (MyEvent
) instead. This could
allow another thread to set the field to null, causing handler
to appear to
be null inside the body of the if statement—a logical contradiction (and a
serious bug).
However, this blog post was written in 2004 (and the book in early 2005), before CLR 2.0 introduced a much stronger memory model with guarantees that eliminate this bug. As per a MSDN article on memory models, CLR 2.0’s memory model rules include:
Joe Duffy also describes the problem (for fields, not for events specifically) and states that the memory model prevents it, in Concurrent Programming on Windows, pp517-8:
As an example of when a load might be introduced, consider this code:
MyObject mo = …; int f = mo.field; if (f == 0) { // do something Console.WriteLine(f); }
If the period of time between the initial read of
mo.field
into variablef
and the subsequent use off
in theConsole.WriteLine
was long enough, a compiler may decide it would be more efficient to rereadmo.field
twice. … Doing this would be a problem ifmo
is a heap object and threads are writing concurrently tomo.field
. Theif
-block may contain code that assumes the value read intof
remained0
, and the introduction of reads could break this assumption. In addition to prohibiting this forvolatile
variables, the .NET memory model prohibits it for ordinary variables referring to GC heap memory too.
Since reads can’t be introduced for fields in CLR 2.0, there’s no need to use a non-inlineable helper method to raise the event; the JIT won’t perform aggressive optimisations that violate the memory model. The typical code that’s given for raising events is correct. (But if you do happen to be programming for a non-Microsoft CLR that only follows the ECMA rules, and your code is running on a processor with a notoriously weak hardware memory model, such as the Intel Itanium, you would need to protect this code appropriately.)
Posted by Bradley Grainger on November 20, 2008