Events get a little overhaul in C# 4, Afterward: Effective Events

Bài viết
03/30/2010

In Parts I, II, and III, I talked about the slight changes that we made to field like events, to the += and -= event accessor operators, and how this may affect you. Just so we’re all on the same page, I want to consolidate here a list of Dos and Don’ts so you know how to use these language features effectively and safely.

As with any list of recommendations someone gives you for your source code, please feel free to adapt them or ignore them if your particular set of requirements is in conflict with mine. I am really interested here in field-like events; if you’re implementing your own event accessors, then you know better than I do what you need.

The event pattern

For the following, suppose that you want a public event of type EventHandler. EventHandler is a delegate type that takes a “sender” object and some “args,” and it’ll serve as our example here, but it’s just a stand-in. Use any delegate you like. Anyway, the standard event pattern is the following:

 public event EventHandler MyEvent;
protected void OnMyEvent(EventArgs e)
{
    EventHandler myEvent = MyEvent;
    if (myEvent != null)
    {
        myEvent(this, e);
    }
}

There are a few important details here:

This is exactly the same as the event pattern has always been. If you implemented C# 3 events this way, you’re golden.
It’s important that you copy the backing delegate field to a local in the “On...” method (see Synchronization below).
These members can have any accessibility you like, though a private event would be weird.

Synchronization

Never use “lock(this)” for any reason, especially not to synchronize any action you take on your events. You’ve probably heard enough of this by now, but in C# 3 this is unwise and in C# 4 it’s wrong. If you have an invocation of the delegate under a “lock(this)”, that’s especially awful, as it violates the idea that you should do as little as possible while holding a lock (you’re calling arbitrary event handlers!) and opens the door to deadlock. Instead...

Reads

If you want to read the value of your event delegate, it is safe to simply do so. If you test it and take further action on it, as in the “On...” method, then you should copy it to a local first. Otherwise there may be a mutation between the reads. Remember, delegates are immutable, and when the event delegate changes it’s just the reference that’s changing. Therefore all of your reads are effectively snapshots in time, and consecutive reads will represent intervening mutations. (Note: if it’s not clear, calling the delegate is a “read” operation because we’re talking about reading the reference).

Read-modify-writes

This is the purpose of event accessors, which += and -= allow access to. If you want to look at the event delegate, do something with it, and write the result back, you should use += and -=, which means you are limited to Combine and Remove. If these don’t suffice, you should probably consider implementing your own accessors and not using field-like events. Although it is possible to mimic the compare-and-swap that the C# 4 accessors provide, I don’t recommend it and I won’t put the code here; it’s very easy to get wrong.

Writes

If you want to write to the delegate without using any information about its current state, please do. The only reason I can imagine that one might do this is to say

 myEvent = null;

in order to reset everything, but even in this case you need to be prepared that a client could call your add handler immediately following the reset.

Other

It’s kind of weird that this much attention has been paid to the safety of the event delegate via the accessors, given that your “On...” method is going to invoke the delegate on whatever thread it pleases. But precisely how the handlers are called (on what thread) is left as a matter of a contract between you and your clients. Be careful about that. Furthermore, that contract really ought to specify that the handlers should not throw—error recovery and multicast delegates don’t go together well. But again, this is a matter to settle between you and your clients.

Of course, if you have special knowledge that no one on any other thread can possibly call your accessors (e.g., you are in your constructor), then feel free to do whatever you want with your delegate field.

Single-threaded events

If you are operating in a single threaded environment, and are somehow encumbered by the synchronization that field-like events provide and which you determined that you do not want, write your own event as follows:

 private EventHandler myEventField;
public event EventHandler MyEvent
{
    add { myEventField += value; }
    remove { myEventField -= value; }
}
protected void OnMyEvent(EventArgs e)
{
    if (myEventField != null)
    {
        myEventField(this, e);
    }
}

No-op Events

If you are required to implement an event that appears on an interface, but you never intend to call it, use this pattern for an empty event implementation.

 interface I
{
    event EventHandler MyEvent;
}

class C
{
    event EventHandler I.MyEvent
    {
        add { }
        remove { }
    }
}

Virtual events

Never override a virtual event with a field-like event. The compiler does not handle this gracefully, and it never has. If you need to override virtual events, write your own handlers.

Events and co- and contravariance

Never expose an event of generic delegate type when that delegate type is covariant or contravariant in any of its type parameters. Examples of such delegates include all of the generic System.Actions and System.Funcs.

The problem is that in .NET 4, Delegate.Combine and Delegate.Remove will not work properly when two delegate instances have different actual types. This means that generally, multicast delegates of these “variant” types are dicey, and events in particular expose the Combine and Remove functionality pretty directly. When I say “will not work properly,” I mean they’ll throw.

By the way, the generic EventHandler, EventHandler<T>, is not contravariant in T precisely because of this limitation.

This may be remedied in future versions of the framework, but I am not on the team that owns delegates in the runtime and I don’t represent them, so I can’t make any promises.

And finally…

Sorry if this post is dry! I mostly want to make sure it’s up here for easy searchability. No more of this event stuff next time; I’m planning on writing a little more about dynamic.

And a note to those of you from the comments who want to hear about weakly referenced event handlers: perhaps later. There are some tricks to play, but also some fundamental incompatibilities with the way events and delegates are represented in .NET. But I hear you.

Comments

Anonymous
March 30, 2010
Thanks Chris. I have found this series to be very informative. I look forward to your future posts.
Anonymous
March 30, 2010
The "Compiler does not handle this gracefully" link appears to be broken. [cburrows: thanks! fixed]
Anonymous
March 30, 2010
There's one aspect of the pattern which puzzles me a little: the way that you fetch the event before invoking it. Does the .NET memory model guarantee that any one thread will "see" the most recent subscription (or unsubscription)? I realise that the new field-like event behaviour guarantees that it's published, but that doesn't necessarily mean it will be fetched, does it?I'd expect to see something like:EventHandler e = Interlocked.CompareExchange(ref MyEvent, null, null);Perhaps I'm being too paranoid? Of course it doesn't matter if you're not too fussed about possibly missing a recent subscription.One other pattern to consider is this declaration:public event EventHandler MyEvent = delegate {};You can then ignore the nullity checks, as you'll always have a no-op event handler. Admittedly that means a slight overhead of calling the pointless empty handler, but that should be relatively small.Comments on either of these points would be extremely welcome :)
Anonymous
March 30, 2010
The comment has been removed
Anonymous
March 31, 2010
@Chris Lomont:There is no user expectation of "resetting" an event (in any way whatsoever), because it is not something code outside the class can do by nature of events. The only operations clients of the class have on events are += and -=.
Anonymous
March 31, 2010
@Chris BurrowsI do have to ask... for all the great lengths you've gone to make sure that auto-generated add/remove works well in the face of multi-threading - thus creating a strong implication that it is always fine to add/remove event handlers from several threads concurrently - why still place the burden of properly raising the event on the programmer writing the code?While "fetch to local first" pattern is documented and well-known, I've still seen a lot of code that ignores it, and in many cases it's because the wrong approach really works in so many cases. If event accessors were not thread-safe, it might be discovered earlier, when e.g. some events don't get fired when they should. But as it is, the only case where the wrong pattern actually fails is when one thread raises the event, and another one concurrently removes the handler - which is quite rare in practice. The result is a lot of code that's wrong, but people don't know it.Even for those aware of it, the code is 100% boilerplate. I've never in my life written an On... method which didn't have the null check, and which therefore didn't have to use the pattern. Excepting broken code, I haven't seen one, either.Now, in VB, "RaiseEvent" does all the null-checking itself, and it does it properly WRT threading, so programmer really doesn't have to care about all those things at all. If, in practice, it really is that way for practically all auto-implemented events (as my experience shows), then why not have some similar syntactic sugar for it in C# as well?
Anonymous
March 31, 2010
The comment has been removed
Anonymous
March 31, 2010
The comment has been removed
Anonymous
March 31, 2010
@Jon,On your first point, yes, the read in the method can take a stale event. I believe this is no different than before, and also no big deal, but you can put in a memory barrier or do a volatile read if it's important. Let me say I don't have the time to be really thorough about this right now but I'll follow up later. There is a principle about discussions about the memory model: almost everyone is almost always wrong whenever they make a statement about it. =) I'll be careful and get back to you (and possibly update my post--I don't want to be wrong).For your second point about initializing the delegate reference with a non-null, that's great. I know of people (on the C# team!) who swear by it. I kind of prefer the regular pattern, but I said the thing about "e.g., you are in a constructor" with this in mind.@Chris,What Pavel said.@Pavel,There has been consideration of doing something like VB's RaiseEvent from all the way back in C# 1. However, it hasn't met the bar for a number of reasons. For one thing, the existing code that does it wrong wouldn't be fixed. And also, we can't hide the delegate from the user at this point. I wouldn't expect the language to update this any time soon.@Jeremy,What Jon said. Perhaps the point you're missing is that delegates are immutable?
Anonymous
March 31, 2010
@Chris,Could you provide examples where co- and contravariance are probelmatic?
Anonymous
March 31, 2010
@Paulo, Try this method: Action<string> Boom(Action<string> f, Action<string> g)
{
return f + g;
} on these two arguments: Action<object> a_o = x => Console.WriteLine(x);
Action<string> a_s = x => Console.WriteLine(x);
Boom(a_o, a_s); Once you've done that, imagine what this would mean for an event of type Action<string>.
Anonymous
April 03, 2010
Given this:void a_o(object x) { Console.WriteLine(x); }void a_s(string x) { Console.WriteLine(x); }This works;Boom(a_o, a_s)("test");So, what would be the problem? What am I missing?
Anonymous
April 03, 2010
@Paulo,My example crashes and yours doesn't because you pass two Action<string>'s, and I pass one Action<string> and one Action<object>.You see, in your code, the arguments are method groups, and they get converted to the actual delegate types of the parameters by way of a contravariant conversion from the method group to the elegate. Generics don't enter into it.In my code, the arguments are themselves delegates, and generic contravariance is used for the conversion.You saw example crash, right?
Anonymous
April 04, 2010
Yes. That's why I'm not getting why contravariance is not allowed. What could ever go wrong?
Anonymous
April 05, 2010
We are forced to do a "fetch to local first", which raises a number of problems:We have to understand the implementation so we can understand the need.It's duplication. It breaks the Publish / Subscribe pattern. I understand the technical reasons why "fetch to local first" is needed, but not why the need is there in the first place.I can also accept that it's too late to "fix" this.What I don't understand, is why we still have to deal with this. Why is it impossible to give us a fail safe, non-duplicated way to invoke our event handlers?
Anonymous
April 29, 2010
Chris: First of all, thank you for going into such detail on this topic.As for the "On..." methods, I agree with Pavel Minaev's post.It's great that it's backwards-compatible, but it would be nice to be able to get rid of them in the future and only implement this pattern in the very few cases where you need some special handling.I think something like this would be sweet:public event EventHandler MyEvent;private void DoStuff() { MyEvent.Invoke(); (or Raise() or what makes most sense)}Where Invoke() implements the standard thread-safe OnEvent pattern.It would make it a lot cleaner and easier to implement events, I've personally always hated to have to make all these "On.." methods that rarely differs from the recommended event pattern, thus just making my class more complex and less readable.

Chia sẻ qua