2
votes

I can't seem to wrap my head around how to properly start a task and cancel it. I've changed my code to be more simple to understand my issue and pasted it below (I've also added some comments). The SubscribeToReport method is called first - it contains an await runReportTask which from my understanding will just start the runReportTask method, wait for it to complete execution, then resume executing the code that appear after the await.

Inside runReportTask I create a CancellationTokenSource and start a new task using Task.Factory.StartNew(). Inside the task, there is a while loop that performs processing as long as the task hasn't been canceled. From my understanding, if cancellationToken.IsCancellationRequested is ever true, the else within my while loop will run and the task will stop. After that, I expect SubscribeToReport() to continue execution - this does not happen, and it doesn't ever seem like my code does anything when I call cancellationTokenSource.Cancel() in StopReportTask() . What am I doing wrong? I've been at this for a while and I just can't seem to wrap my head around how this is supposed to work.

private async static void SubscribeToReport()
{
    Console.WriteLine("Waiting for task to finish or cancel...");

    await runReportTask();

    Console.WriteLine("Task has finished or was canceled."); // THIS LINE NEVER EXECUTES
}

private static CancellationTokenSource cancellationTokenSource;
private static CancellationToken cancellationToken;
private bool moreToDo = true;
private static Task runReportTask()
{
    cancellationTokenSource = new CancellationTokenSource();
    cancellationToken = cancellationTokenSource.Token;

    return Task.Factory.StartNew(() =>
    {
        // Were we already canceled?
        cancellationToken.ThrowIfCancellationRequested();

        while(moreToDo == true)
        {
            if(!cancellationToken.IsCancellationRequested)
            {
                // Do important code here
            }
            else{
                Console.WriteLine("Task canceled."); // THIS LINE NEVER EXECUTES WHEN CALLING StopReportTask() below
                cancellationToken.ThrowIfCancellationRequested();
                break;
            }
        }

    }, cancellationToken);
}

// This method is called from another source file
private void StopReportTask()
{
    Console.WriteLine("Stopping report task...");

    cancellationTokenSource.Cancel();
}
2
Can you confirm the cancellationTokenSource.Cancel() is called on the same object? - CodingYoshi
The class the contains this code is a singleton, so yes. - Roka545
I'm starting to think this may be an issue related to a nanomsg method I am using. I'm using nanomsg to receive data via a publisher/subscriber system - inside my while loop I have a method that listens for messages - it waits until it receives one - during this time I don't think the while loop continues running which would explain why the cancellation token isn't working. I've debugged my StopReportTask method and the cancellation token is definitely returning true (as in it wants to cancel the task). - Roka545
"The class the contains this code is a singleton" -- the relevant code and class field are static, so they are not part of a singleton. And given the code you posted, it is theoretically possible, if you call runReportTask() again before the first task finishes, that the first task will never be canceled. It is also true, as you observe, that if the code is blocked, it can never get to a point of detecting the cancellation. If you want a real answer, you need to post a real minimal reproducible example that reliably reproduces the problem. There is not enough context here to know for sure what the issue is. - Peter Duniho

2 Answers

6
votes

it contains an await runReportTask which from my understanding will just start the runReportTask method, wait for it to complete execution, then resume executing the code that appear after the await.

The code:

await runReportTask();

is like this code:

var task = runReportTask();
await task;

So, to be clear, it is the method call that starts the task running. await just (asynchronously) waits for it to complete.

It's also important to note that await is an asynchronous wait. This means it pauses the method's execution and returns. So, as far as your calling code knows, SubscribeToReport has completed executing.

(This is one of the problems of async void; it's not easily consumable - calling code doesn't know when it completes).

Inside runReportTask I create a CancellationTokenSource and start a new task using Task.Factory.StartNew().

As a side note, you should use Task.Run. Task.Factory.StartNew is a low-level API with dangerous default settings.

it doesn't ever seem like my code does anything when I call cancellationTokenSource.Cancel() in StopReportTask()

It's probably a different cancellationTokenSource instance.

You could try this, as a test - I'm not sure if it's your desired semantics, though (it will cause runReportTask to cancel the previous attempt):

private static CancellationTokenSource cancellationTokenSource;
private static readonly object _mutex = new object();
private bool moreToDo = true;
private static Task runReportTask()
{
  CancellationTokenSource oldCts, currentCts;
  lock (_mutex)
  {
    oldCts = cancellationTokenSource;
    currentCts = cancellationTokenSource = new CancellationTokenSource();
  }
  if (oldCts != null)
    oldCts.Cancel();
  var cancellationToken = currentCts.Token;
  ...
}
3
votes

It might be a typing error, but moreToDo is not static. Your static function RunReportTask can't access it. Besides it seems that you want that others outside your runReportTask can change moreToDo to stop this task. Are you sure you want this?

If you want others to stop your task, why don't you let them use the CancellationTokenSource?

On the other hand, if you only want that your RunReportTask is the only one who changes moreToDo, why did you make it accessible by others?

Having said that, I'm really not sure if it is wise to make your procedures static. Do you really plan to let someone start your process and let someone else stop this process?

Another problem is that if several starters start your task the CancellationTokenSource is replaced by a new one, while already running tasks still check the token created by your old CancellationTokenSource, and no one could stop the old running tasks. Are you sure this is what you intended?

These problems would not arise if you forced the task starter to provide the CancellationTokenSource, which is much more like MSDN proposes in the article Cancellation in managed threads.

If you'd let the starter of a task create the CancellationTokenSource, you would have an owner of the running task who could decide whether is should share the ownership or not. Others can't stop this task without consent of the task owner.

Because your SubscribeToReport returns void and not Task I assume it is a simplified version of an event handler. If not, I advise you to let it return Task.

The non-static version of your SubscribeToReport would be like this:

public async Task SubscribeToReport(CancellationToken token)
{
    Console.WriteLine("Waiting for task to finish or cancel...");
    await Task.Run(runReportTask(token), token);
    Console.WriteLine("Task has finished or was canceled.");
}

private Task runReportTask(CancellationToken token)
{
    bool moreToDo = false;
    token.ThrowIfCancellationRequested();
    while(moreToDo && !token.IsCancellatinRequested)
    {
        // Do important code here, 
        // if this takes some time, make sure that 
        // you reqularly check IsCancellationRequested
        // this part runs until !moreToDo
     }

     if (token.IsCancellationRequested)
     {
         Console.WriteLine("Task canceled."); 
         // do cleanup here
     }
}

As an alternative: don't check for token.IsCancellationRequested, but call token.ThrowIfCancellationRequested. Be sure to do cleanup in finally statements.

usage:

using (var cancellationTokenSource = new CancellationTokenSource())
{
    var token = CancellationTokenSource.Token;
    var task = SubscribeToReport(token);
    // here you are free to do other things,
    // if you decide to cancel:
    cancellationTokenSource.Cancel();
    // or:
    cancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(5));
    
    // if you have nothing else to do but wait for the task to be completed
    // or cancelled:
    await task;
    ProcessTaskResult(task);
}        
     

Note that the CancellationTokenSource is neatly disposed when your task is finished. The designer of the CancellationTokenSource implemented IDisposable to encourage you to free the resources it takes as soon as you don't need them anymore before the garbage collector does this.