Safely firing an event, Part 3

Take another look at Safely firing an event. According to Grant, there’s another issue here. The JITter may optimize away locals in situations where you think want them for thread safety. So the recommendation to make a local copy is not as valuable as we had hoped.

Grant’s recommendation is to mark the event volatile:

    public volatile event D SomeEvent;

    void FireSomeEvent()

    {

        D temp = SomeEvent;

        if (temp != null)

        {

            temp();

        }

    }

This problem just keeps getting worse & worse!

My conclusion: Use the empty delegate pattern, that I described in Safely firing an event, Part 2:

    public event D SomeEvent = delegate { };

    void FireSomeEvent()

    {

        SomeEvent();

    }

It’s simpler, cleaner, less headache.

If I was going to design the language over from scratch, I would change the semantics of delegate invocation to say that if it’s empty it’s just a nop, instead of a crash.

Comments

• Anonymous
  September 16, 2004
  Why can't you just change it? What would there be to break? I can only see that a change would render all those null checks redundant. Now you even have a new runtime, so the change should be even smoother.
  
  What does the event look like behind the scenes, anyway?
  
  I think it's amazing that this design flaw ever happen. The C# team brags about how much they think of everything and so on. C# is also about being OO, isn't it? The event mechanism doesn't feel very OO. The fact that the object owns the event, but still has to check on it wether the listener are still there? Didn't nobody's stomach react to that?
  
  I wonder what's wrong whith how Java implements events, since you (as in they) chose to do it different.
• Anonymous
  September 16, 2004
  The comment has been removed
• Anonymous
  September 16, 2004
  I think your point is irrelevant. The event mechanism is based on delegates, but it didn't have to. When it does, it could have the restriction on what types of delegates it accepts, like all delegates should be void. That is also recommended, because return values are hard to handle in events.
  
  I also did not propose a null delegate. What I propose, or want, is that when I invoke an event, it should then invoke all its listeners. It may happen that all listeners equals no listeners, but that is really not of my concern.
  
  When I don't know who my listeners are, why should I care when there are none? Isn't that the whole idea with events? That any object can subscribe to my event without me knowing who they are.
  
  The event collection is how the event mechanism should be in the first place.
  
  I also have a great dislike that I can do a += to something which equals to null, and that when I do -= to something, it suddenly becomes null.
  
  No other object in the C# space behaves like this. In fact, I think it's impossible to create such a beast outside of the compiler.
  
  += should map to AddListener(), -= should map to RemoveListener(). The whatever declared with the event keyword should be the event collection and should never be null.
• Anonymous
  September 16, 2004
  The comment has been removed
• Anonymous
  September 16, 2004
  When thread saftey is of concern, in some scenarios, I just make a private static method and in a the static constructor of the class that declares the event, wire it to the private static method, eliminating the need for (does event == null) checks.
• Anonymous
  September 17, 2004
  Folks: You've got some pretty strong feedback on this topic. I've sent it to the appropriate folks for them to ponder. Hopefully they'll think up something great.
• Anonymous
  September 17, 2004
  Well, I do agree that perhaps delegates were not the best thing to use for events.
  
  Or, perhaps, the decision to remove Unicast delegates was a bit short-sighted (not to mention it's odd that the class structure still suggests that there are two kinds of delegates). Here's my idea for a design:
  
  For delegates that act as function-pointers (things like comparisons, starting threads, signal callbacks, etc.), a Unicast delegate that can point to zero or one method would be nice. Having more than one function in the invocation list isn't really what is needed in these cases. In a unicast delegate, a check for null should always be necessary if a return value is involved, and the null-reference exception would be be thrown as currently is the case.
  
  Multicast delegates, which events would still be based on, would not allow any out parameters or return values, since they really don't make sense. Since there would be no return values, a multi-cast delegate that points to no functions could be invoked with no problem.
  
  To me, I don't agree with the decision to drop unicast delegates. The argument was that having two types of delegates could lead to confusion. Maybe unicast delegates could have been called delegates and multicast delegates could have been called events. The keywords of the languages could have been designed to reflect this.
  
  To me, it always seems odd that a delegate such as a thread-start method or a comparison can point to two methods.
• Anonymous
  September 19, 2004
  Oh, by the way...
  
  I've been playing around with some code, and at first I thought that some of these problems could possibly be solved very elegantly if we could write generic wrapper classes with a delegate as a type parameter.
  
  I was thinking about writing a ThreadSafeDelegate<T> where T : Delegate, but the Delegate constraint cannot be used at all. I started coding away without this constraint, performing runtime checks on the type of T, but I still couldn't get an elegant solution.
  
  I also tried this technique to make WeakDelegate<T> where T : Delegate, but I had those same problems. One of the problems is invocation.
  
  However, if you follow the event model, and don't care about delegates in general, this can be very easily done with a TreadSafeEvent<T> where T : EventArgs and WeakEvent<T> where T : EventArgs that simply raise EventHandler<T> events.
  
  I will see about implementing weak events and thread-safe events into my EventCollection class. I'm excited about the possibilities here.
• Anonymous
  September 19, 2004
  Matthew,
  
  That sounds pretty cool. I'm looking forward to seeing the result.
  
  A couple times I've run in to situations where I wanted to constrain to delegates, but it can't be done. I made a suggestion to the langauge team to add support constraining to:
  
  - 'delegate' - Delegate types, but not System.Delegate
  - [] - array types, but not System.Array
  - enum - enum types, but not System.Enum
  
  (Imagine these being similar to 'class' and 'struct' constraints.)
  
  The response was that they weren't really useful. These didn't enable anything you couldn't do otherwise.
  
  For example, even if you constrain to a delegate, you can't safely invoke the delegate, because you can't supply a method with the correct signature. (You could provide Invoke(params[] object args), but it's not typesafe.)
  
  If you can find a situation in which extending the constraint system actually was useful, let me know & I'll forward it to the language designers.
• Anonymous
  September 19, 2004
  I think that the "enum" constraint could be very important and useful, but only if they add generic type-safe versions of the static methods on Enum, such as Parse. I believe that this has been suggested, and if it ever gets implemented an enum constraint may be necessary. I don't quite understand why System.Enum is not allowed as a constraint, except the possible question of what happens when you actually use System.Enum itself and not a derived type as a parameter.
  
  Plus, using an Enum constraint should theoretically let you use all of the bitwise operators, since they're available for all the underlying types of enums.
  
  The only place I've thought of where this would be useful is in writing a helper class "Flags<T> where T : enum" that allowed you to get/set/toggle bits by an indexer..But after thinking about it the great performance benefits of bit-set enums would be completely destroyed by the increased working-set size of a program using such a generic type.
  
  As for arrays, I'm failing to see how T : [] would be better than just using T with a T[] member/parameter, but I think I'm probably missing something.
  
  Delegates would be really nice, too, but like you said, I've found no way other than dynamic-invocation to make good use of them. If the runtime/compiler could get around this, and somehow magically let generics based on delegates insert dynamic method signatures based on the signature of the type parameter, then I could put this to use.
• Anonymous
  September 19, 2004
  Matthew:
  
  Constraining to 'System.Enum' or any of the other special classes could allow you to actually create an instance of one of them (or so I'm told). Obviously that should be stopped! That's why we would use the keyword syntax for the constratint (like 'where T: struct' instead of 'where T: System.ValueType').
  
  When thinking about the performance impact of bit flags vs. arrays of bool, remember that bit-flags are essentially unaligned. Depending on specific details of the context in which your application runs, the performance comparison can change quite significantly. (I've always wanted to compare booleans in 1-, 2-, and 4-byte storage on x86 to see if there are situations where the larger storage could be faster.)
  
  This just reinforces the principle of coding for clarity, and taking real perforamnce measurements before compromising clarity for performance.
• Anonymous
  September 19, 2004
  I really doin't see how you could create an instance of System.Enum if it was a constraint, but then again I didn't design the framework. How is this any different than constraining on an interface or an abstract class?
  
  Is it because ValueType and Enum are themselves reference types? Does it have something to do with serialization? Does IL enforce these rules, or is it simply a C#/VB/C++ thing?
• Anonymous
  September 19, 2004
  The comment has been removed
• Anonymous
  September 20, 2004
  How about this solutions:
  
  public event D SomeEvent;
  void FireSomeEvent()
  {
  D temp = null;
  temp += SomeEvent;
  if (temp != null)
  temp();
  }
  
  or:
  
  public event D SomeEvent;
  void FireSomeEvent()
  {
  
  D temp = delegate{};
  temp += SomeEvent;
  
  if (temp.GetInvocationList().Lenght >=2)
  temp();
  }
• Anonymous
  September 20, 2004
  Good Idea Juval. If this does indeed prevent the problem (which I think it does, if I had to wager), then you can simplify this even further:
  
  public event D SomeEvent;
  void FireSomeEvent() {
  D handler = null + SomeEvent;
  if(handler != null) handler();
  }
  
  I'd try to avoid using "delgate{}" on each firing of the event...actually I'd avoid it alltogether (although it's certainly elegant).
  
  The problem with throwing delegate{} all over the place is that you end up generating a TON of auto-generated methods that will have to be loaded and jitted by the runtime. I'm not sure what kind of overhead an empty static method on a class creates for the runtime, but it certainly bloats the metadata. Anonymous methods are great when they do something, but empty anonymous methods seem wasteful to me. For example, if you have two such events, you will end up with something like a method "<.ctor>b__0()" and a "<.ctor>b__1()".
  
  I can't argue with the elegance of delegate{}, but I'd certainly prefer them to share an empty method somewhere.
  
  Hopefully, Juval's idea will eliminate the need for anything as drastic as delegate{}. And it certainly beats "volatile", which I try to avoid at all costs since few programmers understand it (and it's almost always unclear why a programmer is using it)