Displaying a wait cursor while we're waiting

I come from VB6 where everything is single threaded, so I've never written a lick of multi-threaded code before. I just added a wait cursor to one of our GUI's by stumbling through the docs, but I'm unsure I've done it the best way. Did I?

Control is a Winform view and the code below is located in the presenter class. Full file can be found on GitHub.

private void RefreshToDoList(object sender, EventArgs e)
    {
        try
        {
            Cursor.Current = Cursors.WaitCursor;
            var getItems = new Task>(() => GetItems());
            getItems.Start();

            Control.TodoItems = getItems.Result;
        }
        finally
        {
            Cursor.Current = Cursors.Default;
        }
    }

private IOrderedEnumerable GetItems()
{
    var items = new ConcurrentBag();
    var projects = VBE.VBProjects.Cast();
    Parallel.ForEach(projects,
        project =>
        {
            var modules = _parser.Parse(project);
            foreach (var module in modules)
            {
                var markers = module.Comments.AsParallel().SelectMany(GetToDoMarkers);
                foreach (var marker in markers)
                {
                    items.Add(marker);
                }
            }
        });

    var sortedItems = items.OrderBy(item => item.ProjectName)
                            .ThenBy(item => item.ModuleName)
                            .ThenByDescending(item => item.Priority)
                            .ThenBy(item => item.LineNumber);

    return sortedItems;
}

Your intuition was correct, you're not doing asynchronicity the right way.

Take a look at this bit of code:

var getItems = new Task>(() => GetItems());
getItems.Start();
Control.TodoItems = getItems.Result;

You're creating a new task, starting it and then you synchronously block the current thread by calling .Result. A new thread isn't created just by creating a Task object, that's what async/await is for.

Instead, change your method as such (omitting cursor and try/catch for clarity):

private async void RefreshToDoList(object sender, EventArgs e)
{
    Control.TodoItems = await Task.Factory.StartNew(() => GetItems());
}

private IOrderedEnumerable GetItems()
{
    // Everything else is the same here
}

This incorporates a few changes. First of all: I've made the event handler async void. This is the only situation where it is advised to do this, simply because there's no alternative: event handlers have the requirement to have a void return type whereas asynchronous methods work with Task.

In essence this means that you won't be able to await the event handler so if the caller finishes while the event's effects are still running, your program will exit without a regard of this still running code.

Another change is that I used Task.Factory.StartNew(delegate);. You can choose between this and Task.Run(delegate) but both are more fluent than creating a new Task object and then starting it.

You should always avoid calling .Result since it synchronously blocks the thread to wait for the result (which is the opposite of what you typically want in an asynchronous environment).

As shown in the docs:

Accessing the property's get accessor blocks the calling thread until the asynchronous operation is complete; it is equivalent to calling the Wait method.

The only reason I'm creating a new task from the caller is because GetItems() doesn't return one. Typically you want asynchronicity throughout the entire hierarchy but I didn't see where it would've been used in GetItems().

However, if you ever call an asynchronous method then it returns Task or Task and you can simply use await MyMethodAsync().

StackExchange Code Review Q#82160, answer score: 5