Async implementation of Custom Commands in VSIX

1.4k views Asked by At

When you add a template custom command in a VSIX project, the scaffolding code that Visual Studio generates includes the following general structure:

    /// <summary>
    /// Initializes a new instance of the <see cref="GenerateConfigSetterCommand"/> class.
    /// Adds our command handlers for menu (commands must exist in the command table file)
    /// </summary>
    /// <param name="package">Owner package, not null.</param>
    /// <param name="commandService">Command service to add command to, not null.</param>
    private GenerateConfigSetterCommand(AsyncPackage package, OleMenuCommandService commandService)
    {
        this.package = package ?? throw new ArgumentNullException(nameof(package));
        commandService = commandService ?? throw new ArgumentNullException(nameof(commandService));

        var menuCommandID = new CommandID(CommandSet, CommandId);
        var menuItem = new MenuCommand(this.Execute, menuCommandID);
        commandService.AddCommand(menuItem);
    }

    /// <summary>
    /// This function is the callback used to execute the command when the menu item is clicked.
    /// See the constructor to see how the menu item is associated with this function using
    /// OleMenuCommandService service and MenuCommand class.
    /// </summary>
    /// <param name="sender">Event sender.</param>
    /// <param name="e">Event args.</param>
    private void Execute(object sender, EventArgs e)
    {
        ThreadHelper.ThrowIfNotOnUIThread();
        
        // TODO: Command implementation goes here
    }

    /// <summary>
    /// Initializes the singleton instance of the command.
    /// </summary>
    /// <param name="package">Owner package, not null.</param>
    public static async Task InitializeAsync(AsyncPackage package)
    {
        // Switch to the main thread - the call to AddCommand in GenerateConfigSetterCommand's constructor requires
        // the UI thread.
        await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(package.DisposalToken);

        OleMenuCommandService commandService = await package.GetServiceAsync((typeof(IMenuCommandService))) as OleMenuCommandService;
        Instance = new GenerateConfigSetterCommand(package, commandService);
    }

Note that the framework-provided MenuCommand class takes a standard synchronous event-handling delegate with the signature void Execute(object sender, EventArgs e). Also, judging by the presence of ThreadHelper.ThrowIfNotOnUIThread(), it seems pretty clear that the body of the Execute method will indeed be running on the UI thread, which means it would be a bad idea to have any blocking synchronous operations running in the body of my custom command. Or do anything very long running in the body of that Execute() handler.

So I'd like to use async/await to decouple any long-running operations in my custom command implementation from the UI thread, but I'm not sure how to correctly fit that into the VSIX MPF framework scaffolding.

If I change the signature of the Execute method to async void Execute(...), VS tells me that there's a problem with the ThreadHelper.ThrowIfNotOnUIThread() call: "Avoid throwing when not on the main thread while in an async or Task-returning method. Switch to the thread required instead."

I'm not sure how to "switch to the thread required instead". Is that what the await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(package.DisposalToken) code in the InitializeAsync method is doing? Should I just copy that?

What about exception handling? If I allow the synchronous void Execute() handler to throw an exception, VS will catch it and show a generic error messagebox. But if I change it to async void Execute() then uncaught exceptions won't be raised on the thread which invoked the Execute, and may cause a more serious problem elsewhere. What's the correct thing to do here? Synchronously accessing Task.Result to rethrow exceptions in the correct context seems like a canonical example of the well-known deadlock. Should I just catch all exceptions in my implementation and display my own generic message boxes for anything which can't be handled more gracefully?

EDIT to ask more specific question

Here's a fake synchronous custom command implementation:

internal sealed class GenerateConfigSetterCommand
{
    [...snip the rest of the class...]

    /// <summary>
    /// This function is the callback used to execute the command when the menu item is clicked.
    /// See the constructor to see how the menu item is associated with this function using
    /// OleMenuCommandService service and MenuCommand class.
    /// </summary>
    /// <param name="sender">Event sender.</param>
    /// <param name="e">Event args.</param>
    private void Execute(object sender, EventArgs e)
    {
        ThreadHelper.ThrowIfNotOnUIThread();

        // Command implementation goes here
        WidgetFrobulator.DoIt();
    }
}

class WidgetFrobulator
{
    public static void DoIt()
    {
        Thread.Sleep(1000);
        throw new NotImplementedException("Synchronous exception");
    }


    public static async Task DoItAsync()
    {
        await Task.Delay(1000);
        throw new NotImplementedException("Asynchronous exception");
    }
}

When the custom command button is clicked, VS has some basic error handling which shows a simple message box:

basic error message box for synchronously-thrown exception

Clicking Ok dismisses the message box and VS continues working, undisturbed by the "buggy" custom command.

Now let's say I change the custom command's Execute event handler to a naïve async implementation:

    private async void Execute(object sender, EventArgs e)
    {
        // Cargo cult attempt to ensure that the continuation runs on the correct thread, copied from the scaffolding code's InitializeAsync() method.
        await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(package.DisposalToken);

        // Command implementation goes here
        await WidgetFrobulator.DoItAsync();
    }

Now, when I click the command button, Visual Studio terminates, due to the unhandled exception.

My question is: What is the best practice way to handle exceptions arising from an async VSIX Custom Command implementation, which leads to VS treating unhandled exceptions in async code the same way it treats unhandled exceptions in synchronous code, without risking a deadlock of the main thread?

2

There are 2 answers

0
Rich N On BEST ANSWER

The previously-accepted answer generates a compiler warning, VSTHRD100 'Avoid async void methods', which is some indication that it may not be fully correct. In fact the Microsoft threading documentation has a rule to never define async void methods.

I think the correct answer here is to use the JoinableTaskFactory's RunAsync method. This would look as in the code below. Andrew Arnott of Microsoft says 'This is preferable [to async void] both because exceptions won't crash the app and (more particularly) the app won't close in the middle of an async event handler (that might be saving a file, for example).'

There are a couple of points to note. Although exceptions won't crash the app they just get swallowed, so if you want to display a message box, for example, you'll still need a try..catch block inside the RunAsync. Also this code is reentrant. I've shown this in the code below: if you click the menu item twice quickly, after 5 seconds you get two messageboxes both claiming they came from the second call.

    // Click the menu item twice quickly to show reentrancy
    private int callCounter = 0;
    private void Execute(object sender, EventArgs e)
    {
        ThreadHelper.ThrowIfNotOnUIThread();
        package.JoinableTaskFactory.RunAsync(async () =>
        {
            callCounter++;
            await Task.Delay(5000);
            string message = $"This message is from call number {callCounter}";
            VsShellUtilities.ShowMessageBox(package, message, "", 
                OLEMSGICON.OLEMSGICON_INFO, OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
        });
    }
0
Hydrargyrum On

The documentation that describes the correct usage of the ThreadHelper.JoinableTaskFactory APIs is here.

In the end, I did the following:

private async void Execute(object sender, EventArgs e)
{
    try
    {
         await CommandBody();
    }
    catch (Exception ex)
    {
        // Generic last-chance MessageBox display 
        // to ensure the async exception can't kill Visual Studio.
        // Note that software for end-users (as opposed to internal tools)
        // should usually log these details instead of displaying them directly to the user.
        await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

        VsShellUtilities.ShowMessageBox(
            this._package,
            ex.ToString(),
            "Command failed",
            OLEMSGICON.OLEMSGICON_CRITICAL,
            OLEMSGBUTTON.OLEMSGBUTTON_OK,
            OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
    }
}

private async Task CommandBody()
{
    // Actual implementation logic in here
}