5

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?

Hydrargyrum
  • 3,378
  • 4
  • 27
  • 41
  • 2
    There are a lot of questions here. have you checked out this example https://learn.microsoft.com/en-us/visualstudio/extensibility/how-to-provide-an-asynchronous-visual-studio-service?view=vs-2019 – TheGeneral Jul 28 '20 at 07:51
  • Just as a side comment for context, I will be using the MySqlConnector package in my custom command. The authors strongly recommend using its async methods rather than synchronous. (https://mysqlconnector.net/tutorials/best-practices/) That makes me keen to write the custom command using async/await rather than sync style. – Hydrargyrum Jul 28 '20 at 08:13
  • 1
    Ah, I hadn't seen that the "Use an asynchronous service in a command handler" subheading in the article. I'd glossed over the article because I didn't think I needed to write a service. Will check it out. – Hydrargyrum Jul 28 '20 at 08:20
  • I've read it, but it's very confusing! The step-by-step instructions talk about adapting synchronous Packages into AsyncPackages, but in VS2017 15.9.25, the synchronous Package template no longer seems to exist; it's AsyncPackage only. And the instructions for adapting a Custom Command to call an async service refer to methods which don't exist in the modern Custom Command template such as `MenuItemCallback()` which I _think_ is replaced by `Execute()` in the current template. – Hydrargyrum Jul 28 '20 at 08:50
  • I've left feedback on the article to suggest it be updated to work with the new item templates that MS is now distributing. Hopefully I can scrape enough clues out of it to figure out the right way to proceed. – Hydrargyrum Jul 28 '20 at 08:51
  • The article linked by @TheGeneral has sample code where the command's `MenuItemCallback()` method, equivalent to `Execute()`, does a fire-and-forget invocation of the async service. It calls the Task-returning service without `await`ing the result and does nothing to handle exceptions. VS survives the exception without terminating, but it seems that the exception goes completely unobserved in this case. I don't accept that as best practice. – Hydrargyrum Jul 29 '20 at 04:10
  • You can always raise your own exception dialog, or rethrow the exception on the UI thread, i believe there is an example of that in your original question (you could try it, it may work in a catch). If you are satisfied with your results, might be a good idea to write up an answer to this and mark it correct – TheGeneral Jul 29 '20 at 04:13
  • Would you mind clarifying how to rethrow an awaited exception on the UI thread? I'm not sure how that works. That's what I thought my "naive async implementation" would do, but instead it just kills VS. – Hydrargyrum Jul 29 '20 at 04:19
  • have you tried `try { .. } catch(..) { SwitchToMainThreadAsync(..) throw}` i am not sure this will work, but it might be worth a try – TheGeneral Jul 29 '20 at 04:21
  • That behaves the same as my naive implementation: kills VS. – Hydrargyrum Jul 29 '20 at 04:24

2 Answers2

5

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);
        });
    }
Hydrargyrum
  • 3,378
  • 4
  • 27
  • 41
Rich N
  • 8,939
  • 3
  • 26
  • 33
0

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
}
Hydrargyrum
  • 3,378
  • 4
  • 27
  • 41