30

While investigating this question I got curious about how the new covariance/contravariance features in C# 4.0 will affect it.

In Beta 1, C# seems to disagree with the CLR. Back in C# 3.0, if you had:

public event EventHandler<ClickEventArgs> Click;

... and then elsewhere you had:

button.Click += new EventHandler<EventArgs>(button_Click);

... the compiler would barf because they're incompatible delegate types. But in C# 4.0, it compiles fine, because in CLR 4.0 the type parameter is now marked as in, so it is contravariant, and so the compiler assumes the multicast delegate += will work.

Here's my test:

public class ClickEventArgs : EventArgs { }

public class Button
{
    public event EventHandler<ClickEventArgs> Click;

    public void MouseDown()
    {
        Click(this, new ClickEventArgs());
    }
}

class Program
{    
    static void Main(string[] args)
    {
        Button button = new Button();

        button.Click += new EventHandler<ClickEventArgs>(button_Click);
        button.Click += new EventHandler<EventArgs>(button_Click);

        button.MouseDown();
    }

    static void button_Click(object s, EventArgs e)
    {
        Console.WriteLine("Button was clicked");
    }
}

But although it compiles, it doesn't work at runtime (ArgumentException: Delegates must be of the same type).

It's okay if you only add either one of the two delegate types. But the combination of two different types in a multicast causes the exception when the second one is added.

I guess this is a bug in the CLR in beta 1 (the compiler's behaviour looks hopefully right).

Update for Release Candidate:

The above code no longer compiles. It must be that the contravariance of TEventArgs in the EventHandler<TEventArgs> delegate type has been rolled back, so now that delegate has the same definition as in .NET 3.5.

That is, the beta I looked at must have had:

public delegate void EventHandler<in TEventArgs>(object sender, TEventArgs e);

Now it's back to:

public delegate void EventHandler<TEventArgs>(object sender, TEventArgs e);

But the Action<T> delegate parameter T is still contravariant:

public delegate void Action<in T>(T obj);

The same goes for Func<T>'s T being covariant.

This compromise makes a lot of sense, as long as we assume that the primary use of multicast delegates is in the context of events. I've personally found that I never use multicast delegates except as events.

So I guess C# coding standards can now adopt a new rule: don't form multicast delegates from multiple delegate types related through covariance/contravariance. And if you don't know what that means, just avoid using Action for events to be on the safe side.

Of course, that conclusion has implications for the original question that this one grew from...

Community
  • 1
  • 1
Daniel Earwicker
  • 108,589
  • 35
  • 194
  • 274
  • 2
    Thoiugh interesting want's the question? – Rune FS Oct 05 '12 at 07:04
  • I am surprised this hole escaped the notice of C# team, should be one of the first things they would have tested after introducing variance for generic delegates isnt it? C# 5 exhibit it too (the clr version being the same). – nawfal Jul 08 '14 at 17:56

4 Answers4

11

Very interesting. You don't need to use events to see this happening, and indeed I find it simpler to use simple delegates.

Consider Func<string> and Func<object>. In C# 4.0 you can implicitly convert a Func<string> to Func<object> because you can always use a string reference as an object reference. However, things go wrong when you try to combine them. Here's a short but complete program demonstrating the problem in two different ways:

using System;

class Program
{    
    static void Main(string[] args)
    {
        Func<string> stringFactory = () => "hello";
        Func<object> objectFactory = () => new object();

        Func<object> multi1 = stringFactory;
        multi1 += objectFactory;

        Func<object> multi2 = objectFactory;
        multi2 += stringFactory;
    }    
}

This compiles fine, but both of the Combine calls (hidden by the += syntactic sugar) throw exceptions. (Comment out the first one to see the second one.)

This is definitely a problem, although I'm not exactly sure what the solution should be. It's possible that at execution time the delegate code will need to work out the most appropriate type to use based on the delegate types involved. That's a bit nasty. It would be quite nice to have a generic Delegate.Combine call, but you couldn't really express the relevant types in a meaningful way.

One thing that's worth noting is that the covariant conversion is a reference conversion - in the above, multi1 and stringFactory refer to the same object: it's not the same as writing

Func<object> multi1 = new Func<object>(stringFactory);

(At that point, the following line will execute with no exception.) At execution time, the BCL really does have to deal with a Func<string> and a Func<object> being combined; it has no other information to go on.

It's nasty, and I seriously hope it gets fixed in some way. I'll alert Mads and Eric to this question so we can get some more informed commentary.

Jon Skeet
  • 1,261,211
  • 792
  • 8,724
  • 8,929
  • Cool, I arrived at practically the same example code when tinkering with it on the train home. Would be very interesting to hear the gnarly details. – Daniel Earwicker Jul 13 '09 at 19:18
  • What's the status of this? I would like to use generics and variance restriction in runtime delegate instances, and I don't need the multicast capability but I would like to prevent the next developer from assuming multicasting is in play and it crashes with this mind bender. Is there an established best practice or maybe a compiler warning that will detect this and provide a hint to use the New constructor pattern. – Andyz Smith Jul 28 '19 at 12:28
  • @AndyzSmith: There's been no change that I'm aware of, I'm afraid. – Jon Skeet Jul 28 '19 at 13:27
3

I just had to fix this in my application. I did the following:

// variant delegate with variant event args
MyEventHandler<<in T>(object sender, IMyEventArgs<T> a)

// class implementing variant interface
class FiresEvents<T> : IFiresEvents<T>
{
    // list instead of event
    private readonly List<MyEventHandler<T>> happened = new List<MyEventHandler<T>>();

    // custom event implementation
    public event MyEventHandler<T> Happened
    {
        add
        {
            happened.Add(value);
        }
        remove
        {
            happened.Remove(value);
        }
    }

    public void Foo()
    {
        happened.ForEach(x => x.Invoke(this, new MyEventArgs<T>(t));
    }
}

I don't know if there are relevant differences to regular multi-cast events. As far as I used it, it works ...

By the way: I never liked the events in C#. I don't understand why there is a language feature, when it doesn't provide any advantages.

Community
  • 1
  • 1
Stefan Steinegger
  • 60,747
  • 15
  • 122
  • 189
  • 2
    The primary difference is that multicast delegates are immutable, so it is thread-safe to add/remove handlers during invocation. In your implementation, the list could mutate during event invocation with unpredictable results. – Søren Boisen May 27 '15 at 16:56
  • @SørenBoisen: very good point. Thank you for this one. I could use a `ConcurrentBag` instead of the list. – Stefan Steinegger May 28 '15 at 07:18
  • 2
    Another option is using ImmutableArray or ImmutableList from http://blogs.msdn.com/b/dotnet/archive/2013/09/25/immutable-collections-ready-for-prime-time.aspx. That would optimize for dispatch vs add/remove, but more importantly, they are available in PCL projects :-) – Søren Boisen May 28 '15 at 08:59
  • great start - I'm sure it can be optimized – Demetris Leptos Mar 16 '20 at 21:42
  • this actually worked (!): private event MyEventHandler happened; public event MyEventHandler Happvened { add => this.happened += new MyEventHandler(value); remove => this.happened -= new MyEventHandler(value); } – Demetris Leptos Mar 16 '20 at 23:13
0

Are you getting the ArgumentException from both? If the exception is being thrown by just the new handler, then I would think that it's backward-compatible.

BTW, I think you have your comments mixed up. In C# 3.0 this:

button.Click += new EventHandler<EventArgs>(button_Click); // old

wouldn't have run. That's c#4.0

Wesley Wiser
  • 8,398
  • 4
  • 40
  • 63
  • I've removed all references to versioning in the question, as it seems to have just caused you confusion. The comments about new and old related to the issue I was originally thinking about, which I split this question out from. This question has nothing to do with backward compatibility per se. It's about the compiler assuming that a multicast delegate can bind to methods of contravariant types, which then turns out not to be true at runtime. – Daniel Earwicker Jul 13 '09 at 19:04
0

The following code actually works if you have the delegates normalized to the same type from within upon attaching/detaching! I didn't expect detaching to work but it does. It may not be the most optimized solution but it is clean and simple and it solves a serious problem I've had with my Foundation where co-variance is important for example IObservableList : IObservableListOut. At some point extensions created different delegate types depending on context T where T can be 2 different interfaces per context and therefore the delegates created ended up of different type. It wasn't until today that I actually understood how this issue resulted. Detaching won't work with plain "- value".

    private event MyEventHandler<T> happened;
    public event MyEventHandler<T> Happened
    {
        add => this.happened += new MyEventHandler<T>(value);
        remove => this.happened -= new MyEventHandler<T>(value);
    }
Demetris Leptos
  • 1,260
  • 9
  • 12