Async Await causing UI to be blocked

Question

I have an issue where I have a normal function, that calls an async function, that calls another async function in a loop. Problem is, there are calls to my hardware interface (such as Scan which performs motor movements) and for some reason, this seems to block the UI thread continuously until we actually finish executing the entire "Start" function.

I'm not sure what I'm doing wrong, but I feel maybe certain parts shouldn't be done on the UI thread and maybe done on another thread... I'm not entirely sure how to do this. Below is an example of the code. With each scan, the UI does get updated where the "button" is highlighted per scan.

    public void StartProcess(ObservableCollection<ObjModel> objs)
    {
        // Cancel
        if (cancellationTokenSource != null)
        {
            // Cancel tasks
            cancellationTokenSource.Cancel();
            cancellationTokenSource.Dispose();
            return;
        }

        cancellationTokenSource = new CancellationTokenSource();

        if (MachineController.Instance.InitHardware())
        {
            StartScan(objs);
        }
        else
        {
            Clean();
        }
    }

    private async void StartScan(ObservableCollection<ObjModel> objs)
    {
        // We are now running
        InfoModel.IsRunning = true;

        for (int index = 1; index < Size; index++)
        {
            MachineController.Instance.MoveToIndex(index - 1);
            if ((index - 1) % 2 == 0)
            {
                for (int iAIndex = 1; iAIndex < Size; iAIndex++)
                {
                    var obj = objs.Where(x => x.R == index && x.C == iAIndex).FirstOrDefault();

                    await DoScan(obj);
                }
            }
        }

        Clean();
    }

    private async Task DoScan(ObjModel obj)
    {
        MachineController.Instance.MoveToCIndex(obj.C - 1);

        // Set the task to only take a couple of seconds
        bool check = MachineController.Instance.Scan();

        if (check)
        {
            await Task.Delay(5, cancellationTokenSource.Token);

            // Plot  
            UpdateGraph(obj.R, obj.C);
        }
    }

Do not use `async void` methods, except for event handlers. Besides that, `MachineController.Instance.Scan()` is obviously called in the UI thread and blocks it. You may perhaps wrap it in `await Task.Run(() => MachineController.Instance.Scan());` — Clemens, Oct 29 '19 at 22:14
Hi Clemens. Thanks. Should I remove the async from the StartScan function then? Can DoScan still then have the await?... — uaswpff, Oct 29 '19 at 22:24
No, it should be declared as `private async Task StartScan(...)` and be awaited when called. Just like `DoScan`. The same then applies to `StartProcess`. — Clemens, Oct 29 '19 at 22:25
The only code that won't run on the calling thread will be the `Task.Delay()` (actually it will use a timer anyway I believe). The fact that you `await` it is the `only` reason why your code is asynchronous but not to the level you are expecting. The rest runs on the calling thread which is of course the UI thread, hence the blocking. Did you forget to use async I/O? `Task.Run()`? — , Oct 29 '19 at 22:48

Clemens · Answer 1 · 2019-10-29T23:15:11.137

3

The MachineController.Instance methods are obviously called in the UI thread and may block it.

To avoid blocking the UI thread, execute those methods in a Task:

private async Task DoScan(ObjModel obj)
{
    bool check = await Task.Run(() =>
    {
        MachineController.Instance.MoveToCIndex(obj.C - 1);

        return MachineController.Instance.Scan();
    }

    ...
}

edited Oct 29 '19 at 23:15

answered Oct 29 '19 at 22:20

Clemens

123,504
12
155
268

Thanks Clemens, I changed it so that I am doing what you're doing above. I couldn't change the original async void method because when I call await on the StartScan function, it requires the original calling function to be async in order to use await... and I feel I'm getting into a loop of async awaits.... – uaswpff Oct 30 '19 at 09:37
@uaswpff You know you can do `Task someTask = Task.Run(() => { /* your code here */ }); someTask.Wait();` in a sync routine, right? It is preferable to have your entire call chain async - and even UI event handlers can be async - but in very special cases when you can't do that (like in constructors), this is the solution you are looking for. – mg30rg Oct 30 '19 at 09:42
1

@mg30rg That's a bad advice. Never call Task.Wait in the UI thread: https://stackoverflow.com/q/13411339/1136211. Even better, never use it at all: https://stackoverflow.com/q/33351092/1136211 – Clemens Oct 30 '19 at 09:49
@Clemens I **strongly** suggest you reading the entire comment before answering. I worte _It is preferable to have your entire call chain async - and even UI event handlers can be async_ – mg30rg Oct 30 '19 at 10:33
1

@mg30rg I did that of course, but still, it's a bad advice, and irrelevant here, because there is no indication at all for any *special case*. There is nothing in the question that would prevent OP from making the whole call chain async. – Clemens Oct 30 '19 at 11:02

score -2 · Answer 2 · answered Oct 29 '19 at 22:42

The first options is that you are not actually doing Multitasking.

However there is an alternative: That you are swamping the GUI Thread with updates, making it look like the GUI thread is blocked (when it is just really, really taxed).

Writing a GUI is a expensive operation. If you only do it once per user-triggered event, you will never notice. But do it from a loop - especially loops running in Multitasking - and you can notice it quickly. Doing my first steps into Multithreading, I actually run into this precise issue. I updated the GUI too often, locking it up.

I did write some example code to showcase it:

using System;
using System.Windows.Forms;

namespace UIWriteOverhead
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        int[] getNumbers(int upperLimit)
        {
            int[] ReturnValue = new int[upperLimit];

            for (int i = 0; i < ReturnValue.Length; i++)
                ReturnValue[i] = i;

            return ReturnValue;
        }

        void printWithBuffer(int[] Values)
        {
            textBox1.Text = "";
            string buffer = "";

            foreach (int Number in Values)
                buffer += Number.ToString() + Environment.NewLine;
            textBox1.Text = buffer;
        }

        void printDirectly(int[] Values){
            textBox1.Text = "";

            foreach (int Number in Values)
                textBox1.Text += Number.ToString() + Environment.NewLine;
        }

        private void btnPrintBuffer_Click(object sender, EventArgs e)
        {
            MessageBox.Show("Generating Numbers");
            int[] temp = getNumbers(10000);
            MessageBox.Show("Printing with buffer");
            printWithBuffer(temp);
            MessageBox.Show("Printing done");
        }

        private void btnPrintDirect_Click(object sender, EventArgs e)
        {
            MessageBox.Show("Generating Numbers");
            int[] temp = getNumbers(1000);
            MessageBox.Show("Printing directly");
            printDirectly(temp);
            MessageBox.Show("Printing done");
        }
    }
}

Not doing the string concation on the Text Property improoves performance by orders of magnitude. I am not sure how intelligent the Draw behavior is in this case: Will it queue a redraw for every +=, or only for the 1st one. But with Multitasking you usually will get a draw every time.

This answer is all kinds of irrelevant. Biggest problem: it doesn't involve async/await at all. Next biggest problem: the OP's question doesn't involve high-frequency UI updates. Third biggest problem: the code above doesn't actually _update_ the UI at all in either approach until the whole operation is done. Fourth biggest problem: the example uses string concatenation in a loop instead of `StringBuilder`. — Peter Duniho, Oct 29 '19 at 23:19
I mean, if one is going to answer a question that shouldn't have been answered at all in the first place (this particular question has literally dozens if not hundreds of duplicates on the site already), it would make sense to at least provide a _correct_ and useful answer. — Peter Duniho, Oct 29 '19 at 23:21
@PeterDuniho **HOW** you do the multitasking does not mater for this. There could be nothing *less* relevant to this then this minor detail. — Christopher, Oct 30 '19 at 20:45
_"HOW you do the multitasking does not mater for this"_ -- it really does. But let's assume you're right. A problem is still that your answer doesn't provide for _multitasking_ at all. There's no multitasking in your example code at all. In addition to that critical point, there's also the issue that your answer addresses a concern that may exist in other scenarios, but which has absolutely nothing to do with the question being asked. The OP's issue has nothing to do with frequency of UI updates (your claim), but rather that they never actually put long-running ops in a background thread — Peter Duniho, Oct 30 '19 at 21:00
@PeterDuniho "here's no multitasking in your example code at all." Perhaps because it is about demosntrating that writing the GUI is expensive? Wich is why it is called "UIWriteOverhead". And why that is written as my 3rd Paragraph. When writing that example, I was honestly surprised it worked without even needing multitasking. That makes teh code a whole lot more readable while still showing what maters - the incredible overhead of writing to a UI class in a loop. — Christopher, Oct 30 '19 at 21:12

Async Await causing UI to be blocked

2 Answers2