5

I have a WPF application that initializes the state of the UI via methods in the constructor. However, it's never returning from the Wait(); in the constructor.

Here's what I am currently doing via a fairly-contrived sample:

public class SomeViewModel
{
    public ICommand AddDataCommand { get { return RelayCommand(AddDataExecute); } }
    public ObservableCollection<int> UIData { /* Property with INotifyPropertyChanged */}   

    public SomeViewModel() 
    {
        //Load synch. here
        LoadData().Wait();      
    }

    public async Task LoadData()
    {
        UIData = await Task.Run(() => SomeService.SelectAllUIData());
    }

    public async void AddDataExecute()
    {
        //Add new data item to database on separate thread to keep UI responsive
        await Task.Run(() => SomeService.AddNewData(0));

        //Reload data from database and update UI, has to happen after AddNewData completes
        await LoadData();
    }
}

I believe it's hanging because I am never actually returning a Task. However, I don't know of a different way to assign UIData asynchronously that works both in the constructor and the Commands that call it.

Killnine
  • 5,728
  • 8
  • 39
  • 66
  • You normally don't do loading in constructor. You do it in another method that is called when needed (for example: when view triggers Loaded) – Aleksandar Toplek Jan 29 '14 at 17:01
  • I'd concur with @AleksandarToplek, but with the additional detail that you offload your call to start loading the UI on the dispatcher. You can configure the call to start operating when the UI has fully loaded, which gives you the opportunity to show some loading UI. –  Jan 29 '14 at 17:45

2 Answers2

10

Don't construct the object through a constructor, if you require construction to be asynchronous. Use a static factory method:

public class SomeViewModel
{
    private SomeViewModel() 
    { }

    public static async Task<SomeViewModel> Create()
    {
        SomeViewModel model = new SomeViewModel();
        await model.LoadData();
        return model;
    }

    public async Task LoadData()
    {
        UIData = await Task.Run(() => SomeService.SelectAllUIData());
    }

    //...
}

As for why your code isn't working, you're getting the standard await deadlock in which you're blocking on an asynchronous operation, that blocking is preventing continuations from being called on the UI thread, and with two different operations each waiting on the other to continue, you get a deadlock. This is why it's important to "async all the way up" rather than synchronously blocking on an asynchronous operation.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • 1
    I have never seen this before. Do you have any links with further examples? I am using Ninject to generate my ViewModels and set them as the DataContext in my Views. Would ninject, then, be doing this? Seems like a lot of work to just new up your VM. – Killnine Jan 29 '14 at 17:11
  • @Killnine I'm not sure what else you want to know; this is pretty simple and straightforward, and there isn't really "more to it". You would then use this `Create` method to construct the object. If that's not an option, then you'll need to defer the loading of the data until after construction has finished, in some way, while still ensuring the object isn't used before it is "ready". – Servy Jan 29 '14 at 17:15
  • +1 for the explanation of the deadlock. That's very helpful. My main contention with this solution is that I don't actually **need** the VM to be built async. I think perhaps just separating the functionality into a `LoadData()` and `LoadDataAsync` (which the commands would call, and just wraps LoadData() in a Task.Run) would work, no? – Killnine Jan 29 '14 at 17:23
  • @Killnine `LoadData` is *already* asynchronous, as it stands. I would personally suggest constructing the entire object asynchronously as it tends to be the easiest option in most cases. If you want to just construct an "empty shell" and asynchronously load it with data, that's up to you. It can work, but it's generally more confusing and inconvenient. – Servy Jan 29 '14 at 17:25
10

You're seeing the classic deadlock situation that I describe on my blog.

To solve it, use async all the way, as I describe in my MSDN article on async best practices.

This can be difficult in some scenarios. I have a blog post describing a few approaches for async constructors. However, since you're talking about a ViewModel and data for the UI, you'll probably find my blog post on async properties more helpful - in particular, the section on data binding.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Perhaps it doesn't matter as much, but this same issue is what got me into my question above: http://stackoverflow.com/questions/14205590/await-a-async-void-method-call-for-unit-testing. My unit tests all broke when I started retrieving data asynchronously. Eventually got me to this question. Wish there was a DelegateCommand(Func Execute)... – Killnine Jan 29 '14 at 19:06
  • 1
    I recommend you follow my [answer here](http://stackoverflow.com/a/14207615/263693). The `async Task` method is fully unit-testable, and you create an extremely simple `async void` wrapper around it. There's a very similar technique in [this MSDN article](http://msdn.microsoft.com/en-us/magazine/dn237302.aspx). – Stephen Cleary Jan 29 '14 at 19:23
  • Yep, that's what I am doing. But it would be nice is DelegateCommand/RelayCommand accepted Func as arguments. Then there'd be no need for that wrapper. thanks again! – Killnine Jan 29 '14 at 20:29
  • @Killnine: You could write your own [like this](http://stackoverflow.com/a/15743118/263693). – Stephen Cleary Jan 30 '14 at 02:14
  • @Killnine but there is option where you have parameters in execute. Take a look at this: http://stackoverflow.com/questions/5298910/mvvm-light-relaycommand-parameters – Aleksandar Toplek Jan 30 '14 at 05:18