Cancel a loop in a task from outside

Question

On a WinForm, I need to continuously read data from a source (USB device in my case) and display the data in labels. The reading should started on command (button_click) and stopped on another button click or in the form_closing method. I meanwhile found out that I need to use Task.Factory for this since I can create a CancellationToken there. Here is my (example) code so far:

public partial class Form1 : Form
{
    CancellationTokenSource m_CancellationSource;
    Task m_USBReaderTask;

    public Form1()
    {
        InitializeComponent();
    }

    private void button1_Click(object sender, EventArgs e)
    {
        m_CancellationSource = new CancellationTokenSource();
        m_USBReaderTask = Task.Factory.StartNew(() => doAsync(m_CancellationSource.Token), m_CancellationSource.Token);
    }

    private void doAsync(CancellationToken ct)
    {
        InitUSBReader();
        while (!ct.IsCancellationRequested)
        {
            int[] data=ReadUSB();
            this.Invoke((MethodInvoker)delegate
            {
                lbOut1.Text = data[0].ToString();
                lbOut2.Text = data[1].ToString();
                lbOut3.Text = data[2].ToString();
                //... and so on...
            });
        }
        CleanupUSBReader(); //this is never happening
    }

    private void Form1_FormClosing(object sender, FormClosingEventArgs e)
    {
        if (null != m_CancellationSource)
        {
            m_CancellationSource.Cancel();
            m_USBReaderTask.Wait(); // this always hangs.
        }
    }

}

Obviously I am facing two problems:

1. When the CancellationToken is set the task is aborted but I need to clean up, I just want to end the 'while' loop. (or does it crash and no error message?)
2. In the FormClosing event, I need to wait until the cleaning up has finished but it blocks endlessly.

Beside my two problems, is my approach correct at all or is there a more elegant way to achieve my goals? Thanks

Answer 1

Regarding your first issue. When the token is canceled the While-loop will end, and the CleanupUSBReader() method should be run. This is assuming ReadUSB returns regularly, if not you will need some way to cancel the read. If you only cancel the task when the form is closing, the issue is probably a deadlock, see second paragraph. If the ReadUSB returns, you do not deadlock, and you still do not reach the cleanup method, there has to be some other issue, like an exception somewhere.

Regarding your second issue. The problem is that you call this.Invoke, this is synchronous, i.e. it will run the code on the main thread, and wait for it to complete. So when the form is closing the main thread asks the task to be cancelled and waits for it to complete, but the task is waiting for the main thread to update the UI. This result in a classing deadlock. One solution should be to use this.BeginInvoke, since this asks the main thread to update the UI, but does not wait for the result. Read more in Invoke vs BeginInvoke

There is a general recommendation to avoid using task.Wait() since this very easily results in deadlocks like this. It might be a good idea to skip waiting for the task if the form is closing. Or to cancel the closing, await the task, and close the form after the await..

Answer 2

Here are two ways you could refactor your code, in order to avoid using the ugly this.Invoke((MethodInvoker)delegate technique. If the USB reader is free threaded, you can invoke each of its methods in different ThreadPool threads, like this:

async Task LoopAsync(CancellationToken ct)
{
    await Task.Run(() => InitUSBReader(), ct);
    while (!ct.IsCancellationRequested)
    {
        int[] data = await Task.Run(() => ReadUSB(), ct);
        lbOut1.Text = data[0].ToString();
        lbOut2.Text = data[1].ToString();
        lbOut3.Text = data[2].ToString();
        await Task.Delay(100);
    }
    await Task.Run(() => CleanupUSBReader(), ct);
}

But if the USB reader requires thread affinity, then the above method will not work. To make it run in a single thread, you could use the technique bellow, which also offers the advantage of decoupling the UI code from the USB reading code:

Task LoopAsync(IProgress<int[]> progress, CancellationToken ct)
{
    return Task.Factory.StartNew(() =>
    {
        InitUSBReader();
        while (!ct.IsCancellationRequested)
        {
            int[] data = ReadUSB();
            progress.Report(data);
            Thread.Sleep(100);
        }
        CleanupUSBReader();
    }, TaskCreationOptions.LongRunning);
}

...and start the task like this:

m_USBReaderTask = LoopAsync(new Progress<int[]>(data =>
{
    lbOut1.Text = data[0].ToString();
    lbOut2.Text = data[1].ToString();
    lbOut3.Text = data[2].ToString();
}), m_CancellationSource.Token);

The Progress class is normally used for reporting progress, but it is capable of reporting any kind of data.

For cleaning up when the form closes, you can handle the FormClosing event like this:

private async void Form_FormClosing(object sender, FormClosingEventArgs e)
{
    if (!m_USBReaderTask.IsCompleted)
    {
        e.Cancel = true;
        this.Enabled = false;
        m_CancellationSource.Cancel();
        await m_USBReaderTask;
        await Task.Yield(); // Ensure the asynchronous completion before Close
        this.Close();
    }
}