WPF async command blocking UI

Question

I am new the using Task.Run() along with async and await to make UI more responsive, so likely I have not implemented something correctly.

I have reviewed the great articles from Stephen Cleary about using AsyncCommands and have used his code from Patterns for Asynchronous MVVM Applications: Commands as a basis for having a responsive UI but when I run the code it still seems to freeze up (I am not able to move the window or interact with other buttons until the function has fully finished.

I am trying to perform a search which usually takes 5-10 seconds to return. Below is the code that creates the AsyncCommand along with the what the function does.

Code:

    public ICommand SearchCommand
    {
        get
        {
            if (_SearchCommand == null)
            {
                _SearchCommand = AsyncCommand.Create(() => Search());
            }
            return _SearchCommand;
        }
    }
    private async Task Search()
    {
        IEnumerable<PIPoint> points = await SearchAsync(_CurrentPIServer, NameSearch, PointSourceSearch).ConfigureAwait(false);
        SearchResults.Clear();
        foreach (PIPoint point in points)
        {
            SearchResults.Add(point.Name);
        }
    }
    private async Task<IEnumerable<PIPoint>> SearchAsync(string Server, string NameSearch, string PointSourceSearch)
    {
        {
            PIServers KnownServers = new PIServers();
            PIServer server = KnownServers[Server];
            server.Connect();
            return await Task.Run<IEnumerable<PIPoint>>(()=>PIPoint.FindPIPoints(server, NameSearch, PointSourceSearch)).ConfigureAwait(false);
        }
    }

I am thinking that the issue is somewhere in how I am pushing the long running function onto a thread and its not getting off of the UI thread or my understanding of how Tasks and async/await are completely off.

EDIT 1: Following Stephen's answer I updated the functions, but I am not seeing any change in the UI responsiveness. I created a second command that performs the same actions and I get the same response from UI in either case. The code now looks like the following

CODE:

    public ICommand SearchCommand
    {
        get
        {
            if (_SearchCommand == null)
            {
                _SearchCommand = AsyncCommand.Create(async () =>
                {
                    var results = await Task.Run(()=>Search(_CurrentPIServer, NameSearch, PointSourceSearch));
                    SearchResults = new ObservableCollection<string>(results.Select(x => x.Name));
                });
            }
            return _SearchCommand;
        }
    }
    public ICommand SearchCommand2
    {
        get
        {
            if (_SearchCommand2 == null)
            {
                _SearchCommand2 = new RelayCommand(() =>
                {
                    var results = Search(_CurrentPIServer, NameSearch, PointSourceSearch);
                    SearchResults = new ObservableCollection<string>(results.Select(x => x.Name));
                }
                ,()=> true);
            }
            return _SearchCommand2;
        }
    }
    private IEnumerable<PIPoint> Search(string Server, string NameSearch, string PointSourceSearch)
    {
        PIServers KnownServers = new PIServers();
        PIServer server = KnownServers[Server];
        server.Connect();
        return PIPoint.FindPIPoints(server, NameSearch, PointSourceSearch);
    }

I must be missing something but I am not sure what at this point.

EDIT 2: After more investigation on what was taking so long it turns out the iterating of the list after the results are found is what was hanging the process. By simply changing what the Search function was returning and having it already iterated over the list of objects allows for the UI to remain responsive. I marked Stephen's answer as correct as it handled my main problem of properly moving work off of the UI thread I just didnt move the actual time consuming work off.

Answer 1

My first guess is that the work queued to Task.Run is quite fast, and the delay is caused by other code (e.g., PIServer.Connect).

Another thing of note is that you are using ConfigureAwait(false) in Search which updates SearchResults - which I suspect is wrong. If SearchResults is bound to the UI, then you should be in the UI context when updating it, so ConfigureAwait(false) should not be used.

That said, there's a Task.Run principle that's good to keep in mind: push Task.Run as far up your call stack as possible. I explain this in more detail on my blog. The general idea is that Task.Run should be used to invoke synchronous methods; it shouldn't be used in the implementation of an asynchronous method (at least, not one that is intended to be reused).

As a final note, async is functional in nature. So it's more natural to return results than update collections as a side effect.

Combining these recommendations, the resulting code would look like:

private IEnumerable<PIPoint> Search(string Server, string NameSearch, string PointSourceSearch)
{
  PIServers KnownServers = new PIServers();
  PIServer server = KnownServers[Server];
  // TODO: If "Connect" or "FindPIPoints" are naturally asynchronous,
  //  then this method should be converted back to an asynchronous method.
  server.Connect();
  return PIPoint.FindPIPoints(server, NameSearch, PointSourceSearch);
}

public ICommand SearchCommand
{
  get
  {
    if (_SearchCommand == null)
    {
      _SearchCommand = AsyncCommand.Create(async () =>
      {
        var results = await Task.Run(() =>
            Search(_CurrentPIServer, NameSearch, PointSourceSearch));
        SearchResults = new ObservableCollection<string>(
            results.Select(x => x.Name));
      });
    }
    return _SearchCommand;
  }
}