Keith Barrows
Keith Barrows

Reputation: 25308

Cancelling thread in async/await

I am trying to test a small app that is using async/await to process transactions and using an await/async polling feature.

My test setup:

    [TestMethod]
    public void TestProcessTimerOnly()
    {
        // this initializes and kicks off the polling
        var tp = new TransactionProcessor();

        try
        {
            Thread.Sleep(5000);
            tp.CancelProcessing();
        }
        catch (Exception ex)
        {
            LogErrors(ref tp, ex);
        }
        finally
        {
            DisplayLog(tp);
        }
    }

    [TestMethod]
    public void TestProcessTimerOnlyForcedCancellation()
    {
        // this initializes and kicks off the polling
        var tp = new TransactionProcessor(1);

        try
        {
            Thread.Sleep(5000);
            tp.CancelProcessing();
        }
        catch (Exception ex)
        {
            LogErrors(ref tp, ex);
        }
        finally
        {
            DisplayLog(tp);
        }
    }

My code (all in one class):

    // Constructor
    public TransactionProcessor(int? debugForcedCancellationDelay = null)
    {
// >>>>>>>> Setup Cancellation <<<<<<<<
        if(debugForcedCancellationDelay.IsEmpty() || debugForcedCancellationDelay.IsZeroOrLess())
            _cancellationToken = new CancellationTokenSource();
        else
            _cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(debugForcedCancellationDelay.Value));
// >>>>>>>> End <<<<<<<<

// was:
        // RepeatActionEvery(() => TestingLog.Add("Repeat Action Every 1 Second"), TimeSpan.FromSeconds(1), _cancellationToken.Token).Wait();
// corrected:
        // _processTask is defined as a global field of type Task...
        _processTask = RepeatActionEvery(() => TestingLog.Add("Repeat Action Every 1 Second"), TimeSpan.FromSeconds(1), _cancellationToken.Token);    //.Wait();
    }

// was:
    //public void CancelProcessing()
// corrected:
    public async Task CancelProcessing()
    {
        _cancellationToken.Cancel();
        await _processTask;
    }
    public static async Task RepeatActionEvery(Action action, TimeSpan interval, CancellationToken cancellationToken)
    {
        while (true)
        {
            action();
            var task = Task.Delay(interval, cancellationToken);

            try { await task; }
            catch (TaskCanceledException) { return; }
        }
    }

When I run the TestProcessTimerOnly() test, it will just sit there until I finally cancel the test rig.

When I run the TestProcessTimerOnlyForcedCancellation() test, it behaves as expected.

So the questions boils down to: Am I using the _cancellationToken variable properly? In one instance, I initialize it with a timeout param. In the other instance I initialize it with no params. What am I doing wrong here?

Upvotes: 1

Views: 321

Answers (2)

i3arnon
i3arnon

Reputation: 116636

You're using "sync over async" which is highly discouraged.

What you are doing

You are getting a task out of RepeatActionEvery that will end only when the cancellation token is canceled. But you are waiting synchronously (blocking) on that task, which means that you would never get out of the constructor and reach the line cancelling the token (tp.CancelProcessing();).

Of course when you create the CancellationTokenSource with a timeout, it will cancel itself eventually without you having to invoke it, so the task will end, the thread waiting on it will be free to finish the constructor and call tp.CancelProcessing();

What you want to be doing

What you should probably do (IIUC) is store the transaction task without waiting for it and await only when you cancel (or finish) the transaction:

public TransactionProcessor(int? debugForcedCancellationDelay = null)
{
    // ...
    Task = RepeatActionEvery(
        () => TestingLog.Add("Repeat Action Every 1 Second"), 
        TimeSpan.FromSeconds(1), 
        _cancellationToken.Token);
}

public async Task CancelProcessingAsync()
{
    _cancellationToken.Cancel();
    await Task;
}

Upvotes: 6

Earl G Elliott III
Earl G Elliott III

Reputation: 425

The reason is because in the first example you have blocked all threads with the .Wait() so nothing actually happens because it is stuck waiting. The reason it works in the second example is because you have set the cancellationToken to cancel after 1 second. This "wait" on the cancellation token's part is on a different thread so it can still run.

Using .Wait or Thread.Sleep will block ALL threads. Since all of your logic is on the same thread, you are stuck. Using Task.Run().Wait() around your RepeatEveryAction(() =>) call (without the .Wait()) should clear it up because the logic will be put onto another thread to run.

Upvotes: -2

Related Questions