12
\$\begingroup\$

I have a MVC WinForms application. I am using Dependency Injection with Ninject as the IoC container.

public class SqlObjectExplorerController : ToolController, ISqlObjectExplorerController
{
    private ISqlObjectExplorerView view = null;
    private SqlServerStructureProvider structureProvider;
    private IProgress<IProgressInfo> progress;

    public SqlObjectExplorerController(ISqlObjectExplorerView view)
    {
        if (view == null)
            throw new ArgumentNullException("view");

        this.view = view;
        InitializeEventHandlers();
        ...
    }

    private void InitializeEventHandlers()
    {
        (view as ToolView).Initialize += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());
        view.OnRefreshObjectExplorerClicked += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());
        view.OnAddServerInstanceClicked += new EventHandler(async (s, e) => await AddServerInstanceAsync());
        view.OnNewSqlQueryClicked += new EventHandler<NewSqlQueryRequestEventArgs>((s, e) => OpenNewSqlQueryDocument(e));
        view.OnExpandRequested += new EventHandler<BeforeExpandEventArgs>((s, e) => BuildSubStructureForDatabaseNode(e));
    }

    private async Task RefreshObjectExplorerAsync()
    {
        await InitializeObjectExplorerAsync();
        view.InitializeObjectExplorer(ServerCache);
    }

    ... // Lots more code.

My question concerns the use of async/await in setting up the likes of += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());. There are some tasks I need to do on a background thread, so I am using async/await, my questions are about my understanding:

  1. In doing async (s, e) => await SomeMethodAsync() I am merely setting up an event handler equivalent to private async void SomeMethodAsync(object s, EventArgs e) { ... }. Which I think is fine in this case of the fire-and-forget Task I want to undertake, is it?

  2. I don't think there are any problems in setting this up these async handlers in the ctor of the controller, am I right?

The code seems to work fine, but is there something I am not seeing?

\$\endgroup\$
  • 1
    \$\begingroup\$ FYI- Re anonymous async handlers in ctor: I just ran into a rare case where a slight change to an async handler resulted in System.InvalidOperationException running CompilerServices.AsyncMethodBuilderCore when ctor first ran (or was loaded/JIT?). Moving code to a named method avoided the bug. [Xamarin iOS; don't know whether Win platforms would be affected]. (Good news is if this happens, it always happens; it doesn't require the event code to actually run - so its not a "lurking" problem.) \$\endgroup\$ – ToolmakerSteve Feb 17 at 19:44
14
\$\begingroup\$

The things to consider when it comes to async event handlers are:

  • Exceptions thrown for the handler might be rethrown on the UI SynchronizationContext, which usually crashes the application.
  • After you raise the event, the handlers won't be completed yet. The execution of a handler might be interleaved with the execution of the code after the raising and execution of multiple handlers can also be interleaved with each other.

Assuming those caveats are acceptable for you, code like this should be fine.


Also:

view.OnRefreshObjectExplorerClicked += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());

You should be able to simplify this to just:

view.OnRefreshObjectExplorerClicked += async (s, e) => await RefreshObjectExplorerAsync();

It seems you never unsubscribe the event handlers, are you sure that's okay?

\$\endgroup\$
  • \$\begingroup\$ How do you unsubscribe from such an event handler? It's showing the warning Event unsubscription via anonymous delegate \$\endgroup\$ – Stephan Sep 27 '18 at 21:01
  • 3
    \$\begingroup\$ @Stephan Either you use a regular method instead of a lambda to both subscribe and unsubscribe or you store the delegate that represents the lambda in a variable and use that to subscribe and unsubscribe. \$\endgroup\$ – svick Sep 27 '18 at 21:47

protected by Jamal Jan 3 at 1:50

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.