Is this way of using Task cancellation before closing port correct?

Question

I have a C# GUI application where when one clicks on a button and then MyMethod starts running async way and it continuously(inside a do while loop) calls MyTask. MyTask writes and reads data from a port. And these data is passed to MyProgressMethod for further proccess.

I want to implement a button which would first cancel/stop MyTask and then close the port.

Being new with this async way, I relied on some online examples which I stumbled upon and the rest were a difficult to grasp. Based on what I read, I came up with the following to achieve cancellation with a button. But I don't quite understand the mechanism and wondering whether the following way is correct:

Declaring a CancellationTokenSource object at the very beginning of class:

CancellationTokenSource my_cancelationTokenSource = null;

The button click event. Button event calls MyMethod:

 private void Button_Click(object sender, RoutedEventArgs e)
 {
    //Some code
    MyMethod();
 }

MyMethod calls MyTask each second and passes data to MyProgressMethod:

private async void MyMethod()
{

    my_cancelationTokenSource = new CancellationTokenSource();

    do
        {
            await Task.Delay(1000);
            //Process some code
            byte[] my_received_data = await MyTask(my_sent_data, my_cancelationTokenSource.Token);
            MyProgressMethod(my_received_data, my_sent_data);
        }
        while (true);
}

MyTask read and writes to the port(Needs to be cenceledbefore the port is closed):

private async Task<byte[]> MyTask(byte[] my_sent_data, CancellationToken cancelToken)
{

    await Task.Delay(200, cancelToken);//??? What should happen here?
    //Some code
    
}

Button event for canceling task and then closing the port:

private  void Button_Disconnect_Click(object sender, RoutedEventArgs e)
{

    my_cancelationTokenSource.Cancel();

    if (my_port.IsOpen)
    {
        my_port.Close();
    }

}

How can this code be optimized for stability?(i.e. port should only be closed after task is cancelled)

You need to catch the cancellation exception which gets thrown. And you could place the port closure in the catch. — Charlieface, Mar 21 '21 at 20:05
You mean in the cancel button event? How about await Task.Delay(200, cancelToken)? What does it do? Could you show these as an answer? Thanks in advance. — pnatk, Mar 21 '21 at 20:08
As a side note, [avoid async void](https://learn.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming#avoid-async-void). It is OK to make the `Button_Click` handler `async void`, because that's the purpose of `async void`'s existence. It's not OK for the `MyMethod` to be `async void` though, because this method is not an event handler. — Theodor Zoulias, Mar 21 '21 at 20:10
@TheodorZoulias The reason I needed to use async void in MyMethod because I wanted to avoid Thread.Sleep(1000) not to block GUI thread. To be able to use await Task.Delay(1000) I introduced acync void. Is that very bad? — pnatk, Mar 21 '21 at 20:13
The correct return type for the `MyMethod` is `Task`, not `void`. This way you'll get back something that you can store in a variable and `await` later, and observe it's possible errors. An `async void` method runs unattended and unobserved in the background, unless it fails, in which case it will cause the crashing of the process. In general this is not what you want. — Theodor Zoulias, Mar 21 '21 at 20:23
@TheodorZoulias What could I do in my scenario? Is there an easy remedy? — pnatk, Mar 21 '21 at 20:27
Would simply changing it to private async Task MyMethod() do the job? — pnatk, Mar 21 '21 at 20:29
Does this answer your question? [Check if CancellationToken has been cancelled](https://stackoverflow.com/questions/58285559/check-if-cancellationtoken-has-been-cancelled) — Charlieface, Mar 21 '21 at 21:19
They are very different answers from each other. Can you help me with my code scenario? In my case I need to be sure task is done before port is closed. — pnatk, Mar 21 '21 at 21:23

Charlieface · Accepted Answer · 2021-03-22T09:11:32.223

The solution here is to not close the port directly from the Disconnect button. Instead, cancel the token, and catch OperationCanceledException in MyMethod:

private CancellationTokenSource my_cancelationTokenSource;

private async void MyMethod()
{
    my_cancelationTokenSource = new CancellationTokenSource();
    try
    {
        while (true)
        {
            await Task.Delay(1000, my_cancelationTokenSource.Token);
            //Process some code
            byte[] my_received_data = await MyTask(my_sent_data, my_cancelationTokenSource.Token);
            MyProgressMethod(my_received_data, my_sent_data);
        }
    }
    catch (OperationCanceledException)
    {
        try
        {
            my_cancelationTokenSource.Dispose();
            my_cancelationTokenSource = null;
            my_port.Dispose();
        }
        catch {  }
    }
}

private void Button_Disconnect_Click(object sender, RoutedEventArgs e)
{
    my_cancelationTokenSource?.Cancel();
}

Notes:

my_cancelationTokenSource becomes a field rather than a local variable.
Pass the token to the Task.Delay functions also. (It's unclear why you need the delays, normally you just wait for a response on the port).
I don't know what exactly you want done on cancellation, I'll leave that to you.
try/catch the closure of the port, which you should do via Dispose, just in case it throws.

I would suggest to nullify the `my_cancelationTokenSource` field after disposing it, otherwise the `my_cancelationTokenSource?.Cancel();` line could throw an `ObjectDisposedException`. — Theodor Zoulias, Mar 22 '21 at 05:16
Thank you for the answer I use it now with some minor changes. I use my_port.Close(); instead of Dispose. I also changed to "async Task MyMethod()" and "async void Button_Click" and call it with await MyMethod(). I did these changes to avoid async void for non-event handlers. I hope its fine — pnatk, Mar 22 '21 at 21:03

Is this way of using Task cancellation before closing port correct?

1 Answers1