I have this event handler:

private void OnShutdownRequested(object? sender, ShutdownRequestedEventArgs e)
{
   var canShutdown = lifetime.CanShutdown();
   e.Cancel = !canShutdown;
}

Now, due to design decisions the CanShutdown method has changed from bool to Task<bool>

Task<bool> CanShutDown()
{
   //...
}

So, I need to modify the event handler like this:

private async void OnShutdownRequested(object? sender, ShutdownRequestedEventArgs e)
{
   var canShutdown = await lifetime.CanShutdown();
   e.Cancel = !canShutdown;
}

I've read many times that async void methods have many problems, like unhandled exceptions being thrown directly inside the SynchronizationContext. But one of the valid usages for them are event handlers. This is an event handler. Isn't it?

BUT, I wonder if that code is free of undesired consequences after the "migration".

My concern is that, in the handler, you set the value of e.Cancel.

A colleague has told me that this will happen:

After await, the caller to that method isn't awaiting. It assumes synchronous execution, so it immediately reads e.Cancel, and that hasn't been set yet. That is a problem inside event handler: You realize as soon as the await keyword is hit, the code that called ShutdownRequested.Invoke() immediately returns. The value it will read might not be up-to-date.

I'm afraid my colleague has his point. So, it seems this approach is broken. But I still don't see how to fix that.

How to deal with the EventArgs being shared by sync and async code?

CodePudding user response：

Your colleague is correct, the posted example code will most likely not work as intended.

I would suggest making ShutdownRequestedEventArgs contain something like a List<Task<bool>>, so the caller can await all the task to determine if any of them wants to cancel before shutting down. This might also include a timeout, so that some hanged task does not block the process forever. This moves the problem from the handler of the event to the caller, that hopefully is in a better position to deal with the problem.

Another possibility could be to wait for the task synchronously:

private void OnShutdownRequested(object? sender, ShutdownRequestedEventArgs e)
{
   var canShutdown = lifetime.CanShutdown().Result;
   e.Cancel = !canShutdown;
}

But this may be dangerous in a UI program. If the ShutdownRequested is requested on the UI thread, and CanShutdown needs to execute any code on the UI thread, then the program will deadlock. And this is fairly likely if CanShutdown is not written specifically to avoid this. See configureAwait for more details .

You might also just revert to the synchronous solution. An event like ShutdownRequested does not sound like it is a good fit for an asynchronous solution. I would have expected that such an event would require a immediate response. But I do not know the background of your application or why the method was changed in the first place, so there might very well be good reasons for it.