Race condition with async/await, how to resolve

Question

I have a problem with async/await in C#, i need it to get some object called Trades, after i get it, it needs to SAVE it. Problem is, with async/await, it is doing the SAVE first, and then go and get my trade objects. How do i ensure i get the objects first, and then does the saving.... here is my code...

    private async void OnRefresh()
    {
        try
        {
            var trades = await ExchangeServiceInstance.GetTrades("");
            mmTrades = new ObservableCollection<EEtrade>(trades);
            tradeListView.ItemsSource = mmTrades;
        }
        catch { }
    }

    public async void OnSignalReceived()
    {
        // THIS NEEDS TO FINISH FIRST, BUT IT DOESN'T
        await tradeListView.Dispatcher.InvokeAsync((Action)async delegate
        {
            if (ExchangeServiceInstance.SelectedTabIndex == CURRENT_TAB_INDEX_ITEM)
            {
                await Task.Delay(MMConfig.DELAYMILLISEC);
                OnRefresh();
            }
        });

        // SOMEHOW THIS GETS CALLED FIRST BEFORE THE ABOVE GETS TO FINISH!
        await OnSaveTrades();
    }

    public async Task<int> OnSaveTrades()
    {
        foreach (var trade in mmTrades)
        {
            await ExchangeServiceInstance.OnInsertDoneTrade(trade);
        }
        return mmTrades.Count;
    }

Any ideas guys? Thanks!

TheTanic · Accepted Answer · 2022-10-19T10:09:48.583

The problem is your OnRefresh method. Because the return type is void the method is not awaited [Check out this answer]. In addition you dont even try to await for the method inside your delegate

Changing the method to the following:

private async Task OnRefresh()
    {
        try
        {
            var trades = await ExchangeServiceInstance.GetTrades("");
            mmTrades = new ObservableCollection<EEtrade>(trades);
            tradeListView.ItemsSource = mmTrades;
        }
        catch { }
    }

And await this method inside your delegate, should solve your problem:

public async void OnSignalReceived()
    {
        // THIS NEEDS TO FINISH FIRST, BUT IT DOESN'T
        await tradeListView.Dispatcher.InvokeAsync((Action)async delegate
        {
            if (ExchangeServiceInstance.SelectedTabIndex == CURRENT_TAB_INDEX_ITEM)
            {
                await Task.Delay(MMConfig.DELAYMILLISEC);
                await OnRefresh();
            }
        });

        // SOMEHOW THIS GETS CALLED FIRST BEFORE THE ABOVE GETS TO FINISH!
        await OnSaveTrades();
    }

score 1 · Answer 2 · answered Oct 19 '22 at 10:07

The use of (Action)async is basically the same as async void, and async void is almost always a mistake. Specifically, the consumer cannot know the outcome (unless it faults synchronously). The dispatcher here isn't really thinking of async.

If we assume that you must use the dispatcher here, perhaps a workaround might be to use something like a SemaphoreSlim (or maybe a TaskCompletionSource<something>) that you signal at the end of your async work (even in the exception case), and then await that; untested, but:

var tcs = new TaskCompletionSource<bool>();
await tradeListView.Dispatcher.InvokeAsync((Action)async delegate
{
    try {
        if (ExchangeServiceInstance.SelectedTabIndex == CURRENT_TAB_INDEX_ITEM)
        {
            await Task.Delay(MMConfig.DELAYMILLISEC);
            OnRefresh();
        }
        tcs.TrySetResult(true);
    } catch (Exception ex) {
        tcs.TrySetException(ex);
    }
});
await tcs.Task; // ensure the async work is complete

await OnSaveTrades();

score 0 · Answer 3 · answered Oct 19 '22 at 10:07

First of all, you are using the async void pattern a lot. This is really bad practice for a number of reasons. You should stop doing that.

The problem here is that OnRefresh is again an async void method that can't be awaited but should be:

private async Task OnRefresh()
{
    try
    {
        var trades = await ExchangeServiceInstance.GetTrades("");
        mmTrades = new ObservableCollection<EEtrade>(trades);
        tradeListView.ItemsSource = mmTrades;
    }
    catch { }
}

In your OnSignalReceived method change the call to OnRefresh(); to await OnRefresh();

Race condition with async/await, how to resolve

3 Answers3