Delegates/events weirdness

[TahirAhmadov]

I always found it strange how .NET chose to implement delegates, specifically WRT "multi-casting". This is not a language request, more a general discussion - I understand this cannot be changed, short of a full framework re-write. (Or maybe a new delegate system can be created alongside the existing one?)
Basically, right now, any delegate is a multi-cast delegate with some sort of linked list structure behind the scenes. This has already caused me at least one bug where I forgot that I have to await every Task in the invocation list, not just the first one. I'm sure people also had bugs where other types of return values were lost.

Task FooAsync() => Task.Delay(2000);
Task BarAsync() => Task.Delay(1000);
delegate Task AsyncEventHandler(object sender, EventArgs e);
event AsyncEventHandler AsyncEvent;
AsyncEvent += FooAsync;
AsyncEvent += BarAsync;
await AsyncEvent(this, EventArgs.Empty);
// first problem: FooAsync and BarAsync start "almost" simultaneously - 
// provided, they start long running operations early on in their execution flow
// second problem, the await gets BarAsync Task, and if FooAsync happens to run longer than BarAsync,
// then the await resumes after BarAsync completes, and FooAsync completes later - EVIL BUG

// just in general
Func<int> f = () => 1;
f += () => 2; 
Console.Write(f()); // outputs 2; 1 is lost - this is bad, we have undefined behavior
// the method returning 1 probably expected that 1 to be used somewhere

I think it should be simplified to a single invocation delegate, and the event keyword auto-accessors add and remove should create a linked list typed backing field. Also, events should restrict the return type to either void or Task (non-value returning task-like type); that way, it can be ensured that everything is raised correctly and no return values are lost. Below is how I envision it working:

Func<int> f = () => 1; // OK
f += () => 2; // compiler error - += is not defined on delegates

event Func<int> Event; // compiler error
event EventHandler Event; // OK - generates the following:

EventCollection<EventHandler> _<mangled>_Event = new();
public event EventHandler Event
{
  add => _<mangled>_Event.Add(value);
  remove => _<mangled>_Event.Remove(value);
  private get => _<mangled>_Event;
}

Event(this, EventArgs.Empty); // generates the following:
_<mangled>_Event.Invoke();

delegate Task<int> BadAsyncEventHandler(object sender, EventArgs e);
event BadAsyncEventHandler AsyncEvent; // compiler error

delegate Task AsyncEventHandler(object sender, EventArgs e);
event AsyncEventHandler AsyncEvent; // OK
await AsyncEvent(this, EventArgs.Empty); // generates the following:
await _<mangled>_Event.SequentialAsync();
// or, if needed:
await AsyncEvent.ParallelAsync(this, EventArgs.Empty); // this basically compiles as is

PS. For now, I think the only reasonable thing to do is create my own type, which is exposed as a get-only member (readonly field or get-property), and that type does the async invocations correctly and enforces type safety and whatnot.

[CyrusNajmabadi]

Basically, right now, any delegate is a multi-cast delegate with some sort of linked list structure behind the scenes. This has already caused me at least one bug where I forgot that I have to await every Task in the invocation list,

Can you give a sample of code that demonstrates the problem you mention here? It's not readily apparent to me what you mean.

[TahirAhmadov]

I updated the OP for everybody to see the problem - refer to the new code snippet in the middle.

[jnm2]

Also, events should restrict the return type to either void or Task (non-value returning task-like type); that way, it can be ensured that everything is raised correctly and no return values are lost.

Restricting what the C# compiler allows can be achieved today by writing a Roslyn analyzer.

Implicitly looping over the delegate invocation list and calling Task.WhenAll with the results, or even serially awaiting, could change the meaning of existing code.

https://github.com/Techsola/AmbientTasks is a completely different take on this which says that event handlers should not be async, but they can start async things that are most properly tracked at a higher level than at the event invocation sites (which should often be agnostic as to whether something async is being triggered and especially as to whether the handler fails).

[CyrusNajmabadi]

Can you provide an example, or a link to an example? I honestly have no idea what you are talking about.

void IDoSomeStuff()
{
    SomethingAsync();
    Some.Other.Code();
    SomethingElseAsync();
}

Async code returning tasks that this code doesn't care about at all.

[CyrusNajmabadi]

It's better in some cases; in other cases, event is better, say, you have components within a single application and the event is on a UI element of some sort, and initialization is done before a form is shown, or an MVC controller renders, etc. To go with an interface approach here would be a whole bunch of boilerplate for no gain.

Well... except for the gain of not having this problem :)

then a foreach with await x.FooAsync(),

I mean... it seems unclear to me why we'd change things here given that you've described what looks to be clear, easy, and idiomatic for solving this problem. You have a list of asynchronous work you care about entirely finishing. You can do what you said there, or even simply await Task.WhenAll(all_the_work). You now have composability here that makes a ton of sense.

[TahirAhmadov]

Async code returning tasks that this code doesn't care about at all.

That to me looks no different than async void. Why return Task at all, if the async-completion of that method isn't important? And even if it's dual-use, meaning awaiting is needed in some cases and not others, sure - I'm not saying that shouldn't be allowed. However, it's much more likely that you want to await. Even under my proposal, you can just do this:

_ = AsyncEvent.ParallelAsync(...); // the resulting Task is ignored, voila

[CyrusNajmabadi]

That to me looks no different than async void. Why return Task at all, if the async-completion of that method isn't important?

Because the callee doesn't determine that. The caller does. A callee makes it available in case a caller needs it. If the caller doesn't, they don't.

However, it's much more likely that you want to await. Even under my proposal, you can just do this:

I mean, if you think so, that's fine :)
The C# position is effectively: don't care. It's a specific policy choice of a particular domain.

Tasks aren't special, and "auto-awaiting, but opting out" was not viewed as anything hte lang should care about. Perhaps if it's all done all over again, maybe people will feel differently. HOwever, i've genuinely not seen any sentiment toward that internally. It's remained the opposite: await should be explicit.

[TahirAhmadov]

Tasks aren't special, and "auto-awaiting, but opting out" was not viewed as anything hte lang should care about. Perhaps if it's all done all over again, maybe people will feel differently. HOwever, i've genuinely not seen any sentiment toward that internally. It's remained the opposite: await should be explicit.

await is explicit in my proposal, too. You still need to do await AsyncEvent(...) to await. Or you can do _ = AsyncEvent(...) which kicks off the sequential execution of each handler, but without the raiser waiting for it.

[CyrusNajmabadi]

Note: i think if you really want this sort of behavior, i would not invoke delegates manually. I would use GetInvocationList and control exactly how you think every individual delegate should be treated.

[TahirAhmadov]

@TahirAhmadov

It's consistent with how multicast delegates have always work. async and Task have nothing to do with this. If you returned int you'd still only get one value back, not the sum of the values or some other arbitrary way to fold them.

That's the thing - my argument is that how multicast delegate have always worked is a terrible design flaw.
Anyway, I wrote all I had to say about this. Please feel free to read up out back and forth with Cyrus above - I've answered all these questions.

[CyrusNajmabadi]

That's the thing - my argument is that how multicast delegate have always worked is a terrible design flaw.

This seems entirely unrelated to Tasks though.

[CyrusNajmabadi]

Yes and no. It's consistent if you use the implementation details of how async works as the guideline; c

This is not true, and I mentioned this above. It's not an implementation detail. It's literally just methods and tasks. Legitimately that is spec'ed as not at all ensuring completion of the asynchronous work. The methods being called just have to returns task. That's it. And it is normal and expected for the behavior to work this way (and the entire .net async programming model is built off of that concept).

[CyrusNajmabadi]

Meaning, to know how these 2 things are consistent, it requires the user to know the ins and outs of async implementation in C#.

You don't at all. Indeed, c# itself does not know or care about it's own implementation. The methods (and tasks) can come from anywhere. All that matters is the API shape of the Task-like types (and associated builders and whatnot).

While it's good knowledge to have in any case, I don't think it should be what drives decisions on what is elegant syntax at the high level.

The point is that you don't need to know. The Tasks are just a way for an API to express async work. The TPL then has lots of great Apis and helpers to work with those tasks, and the language has a nice optional feature (with async/await) to easily work with those Tasks as well.

[HaloFour]

I might agree that it's unfortunate that all delegates are effectively multicast. It feels like there probably should have been more of a distinction between unicast and multicast delegates, and an explicit way to go from one to the other. But here we are and there isn't much we can do about it. But we can fully override how events work to not depend on multicast delegates and to iterate through the invocation list as we see fit.