CODEWITHSUNDEEP
c#async-awaittask-parallel-librarytask
Paul Devenney
Paul Devenney

Reputation: 1329

C# Task Ignoring Cancellation timeout

I'm trying to write a wrapper for arbitrary code that will cancel (or at least stop waiting for) the code after a given timeout period.

I have the following test and implementation

[Test]
public void Policy_TimeoutExpires_DoStuff_TaskShouldNotContinue()
{
    var cts = new CancellationTokenSource();
    var fakeService = new Mock<IFakeService>();
    IExecutionPolicy policy = new TimeoutPolicy(new ExecutionTimeout(20), new DefaultExecutionPolicy());
    Assert.Throws<TimeoutException>(async () => await policy.ExecuteAsync(() => DoStuff(3000, fakeService.Object), cts.Token));

    fakeService.Verify(f=>f.DoStuff(),Times.Never);
}

and the "DoStuff" method

private static async Task DoStuff(int sleepTime, IFakeService fakeService)
{

    await Task.Delay(sleepTime).ConfigureAwait(false);
    var result = await Task.FromResult("bob");
    var test = result + "test";
    fakeService.DoStuff();
}

And the implementation of IExecutionPolicy.ExecuteAsync

public async Task ExecuteAsync(Action action, CancellationToken token)
{
    var cts = new CancellationTokenSource();//TODO: resolve ignoring the token we were given!

    var task = _decoratedPolicy.ExecuteAsync(action, cts.Token);
    cts.CancelAfter(_timeout);

    try
    {
        await task.ConfigureAwait(false);
    }
    catch(OperationCanceledException err)
    {
        throw new TimeoutException("The task did not complete within the TimeoutExecutionPolicy window of" + _timeout + "ms", err);
    }
}

What should happen is that that the test method attempts to take >3000ms and the timeout should occur at 20ms, but this is not happening. Why does my code not timeout as expected?

EDIT:

As requested - the decoratedPolicy is as follows

public async Task ExecuteAsync(Action action, CancellationToken token)
{
    token.ThrowIfCancellationRequested();
    await Task.Factory.StartNew(action.Invoke, token);  
}

Upvotes: 2

Views: 2619

Answers (4)

Paul Devenney
Paul Devenney

Reputation: 1329

I've decided to answer my own question here, as while each of the listed answers solved something that I needed to do, they did not identify the root cause of this issue. Many, many thanks to: Scott Chamberlain, Yuval Itzchakov,Sriram Sakthivel, Jeff Cyr. All advice gratefully received.

Root cause/Solution:

await Task.Factory.StartNew(action.Invoke, token);

which you see above in my "decorated policy" returns a Task and await waits only for the outer task. Replacing it with

await Task.Run(async () => await action.Invoke());

gets the correct result.

My code was suffering from a combination of Gotcha #4 and Gotcha #5 from an excellent article on C# async gotchas

The entire article (as well as answers posted to this question) has really improved my overall understanding.

Upvotes: 0

Jeff Cyr
Jeff Cyr

Reputation: 4864

You are calling Assert.Throws(Action action) and your anonymous async method is casted to async void. The method will be called asynchronously with Fire&Forget semantics without throwing exception.

However the process would likely crash shortly after due to the uncatched exception in an async void method.

You should call ExecuteAsync synchronously:

[Test]
public void Policy_TimeoutExpires_DoStuff_TaskShouldNotContinue()
{
    var cts = new CancellationTokenSource();
    var fakeService = new Mock<IFakeService>();
    IExecutionPolicy policy = new TimeoutPolicy(new ExecutionTimeout(20), new DefaultExecutionPolicy());
    Assert.Throws<AggregateException>(() => policy.ExecuteAsync(() => DoStuff(3000, fakeService.Object), cts.Token).Wait());

    fakeService.Verify(f=>f.DoStuff(),Times.Never);
}

or use an async test method:

[Test]
public async Task Policy_TimeoutExpires_DoStuff_TaskShouldNotContinue()
{
    var cts = new CancellationTokenSource();
    var fakeService = new Mock<IFakeService>();
    IExecutionPolicy policy = new TimeoutPolicy(new ExecutionTimeout(20), new DefaultExecutionPolicy());

    try
    {
        await policy.ExecuteAsync(() => DoStuff(3000, fakeService.Object), cts.Token);
        Assert.Fail("Method did not timeout.");
    }
    catch (TimeoutException)
    { }

    fakeService.Verify(f=>f.DoStuff(),Times.Never);
}

Upvotes: 1

Sriram Sakthivel
Sriram Sakthivel

Reputation: 73502

If I understand correctly, you're trying to support timeout for a method which doesn't supports timeout / cancellation.

Typically this is done with a starting a timer with required timeout value. If the timer fires first, then you can throw exception. With TPL, you can use Task.Delay(_timeout) instead of a timer.

public async Task ExecuteAsync(Action action, CancellationToken token)
{
    var task = _decoratedPolicy.ExecuteAsync(action, token);

    var completed = await Task.WhenAny(task, Task.Delay(_timeout));
    if (completed != task)
    {
        throw new TimeoutException("The task did not complete within the TimeoutExecutionPolicy window of" + _timeout + "ms");
    }
}

Note: This doesn't stops the execution of _decoratedPolicy.ExecuteAsync method rather it ignores it.

If your method do support cancellation(but not in a timely manner) then it is better to cancel the Task after the timeout. You can do it by creating a Linked Token.

public async Task ExecuteAsync(Action action, CancellationToken token)
{
    using(var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(token))
    {
        var task = _decoratedPolicy.ExecuteAsync(action, linkedTokenSource.Token);

        var completed = await Task.WhenAny(task, Task.Delay(_timeout));
        if (completed != task)
        {
            linkedTokenSource.Cancel();//Try to cancel the method
            throw new TimeoutException("The task did not complete within the TimeoutExecutionPolicy window of" + _timeout + "ms");
        }
    }
}

Upvotes: 5

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149628

Using CancellationToken means that you're doing cooperative cancellation. Setting CancellationTokenSource.CancelAfter will transition the underlying token to a canceled state after the specified amount of time, but if that token isn't monitored by the calling async method, then nothing will happen.

In order for this to actually generate a OperationCanceledException, you need to call cts.Token.ThrowIfCancellationRequested inside _decoratedPolicy.ExecuteAsync.

For example:

// Assuming this is _decoratedPolicy.ExecuteAsync
public async Task ExecuteAsync(Action action, CancellationToken token)
{
     // This is what creates and throws the OperationCanceledException
     token.ThrowIfCancellationRequested();

     // Simulate some work
     await Task.Delay(20);
}

Edit:

In order to actually cancel the token, you need to monitor it at all points where work is executed and execution may timeout. If you cannot make that guarantee, then defer to @SriramSakthivel answer where the actual Task is being discarded, rather than being actually canceled.

Upvotes: 2

Related Questions