awaiting task with timeout

Question

I'm trying to write a helper method which allows me to pass in an arbitrary task and a timeout. If the task completes before the timeout, a success delegate is called, otherwise an error delegate is called. The method looks like this:

    public static async Task AwaitWithTimeout(Task task, int timeout, Action success, Action error)
    {
        if (await Task.WhenAny(task, Task.Delay(timeout)) == task)
        {
            if (success != null)
            {
                success();
            }
        }
        else
        {
            if (error != null)
            {
                error();
            }
        }
    }

Now this seems to work most of the time, but i wanted to write some tests as well to make sure. This test, to my surprise fails, and calls the error delegate instead of the success:

        var taskToAwait = Task.Delay(1);
        var successCalled = false;

        await TaskHelper.AwaitWithTimeout(taskToAwait, 10, () => successCalled = true, null);

        Assert.IsTrue(successCalled);

This test, however, is green:

        var taskToAwait = Task.Run(async () =>
        {
            await Task.Delay(1);
        });

        var successCalled = false;

        await TaskHelper.AwaitWithTimeout(taskToAwait, 10, () => successCalled = true, null);

        Assert.IsTrue(successCalled);

How do i make both tests green? Is my usage of Task.WhenAny incorrect?

`Delay(1)` is very short (especially if you are debugging) - starting it on separate thread make it sufficiently long for your test to pass most of the time. Try to use longer delay or better yet task that allows to manually complete it synchronously in test. — Alexei Levenkov, Feb 26 '16 at 07:39

score 17 · Accepted Answer · edited May 23 '17 at 12:10

Timers are inaccurate. By default their accuracy is around 15 ms. Anything lower than that will trigger in 15ms interval. Refer related answer.

Given that you have 1ms timer and 10ms timer; both are roughly equal so you get inconsistent results there.

The code you wrapped in Task.Run and claims to be working is just a coincidence. When I tried several times, results are inconsistent. It fails sometimes for the same reason mentioned.

You're better off increasing the timeout or just pass in a already completed task.

For example following test should consistently pass. Remember that your test should be consistent not brittle.

[Test]
public async Task AwaitWithTimeout_Calls_SuccessDelegate_On_Success()
{
    var taskToAwait = Task.FromResult(0);

    var successCalled = false;

    await TaskHelper.AwaitWithTimeout(taskToAwait, 10, () => successCalled = true, ()=>{ });

    Assert.IsTrue(successCalled);
}

For never ending task use TaskCompletionSource and don't set its result.

[Test]
public async Task AwaitWithTimeout_Calls_ErrorDelegate_On_NeverEndingTask()
{
    var taskToAwait = new TaskCompletionSource<object>().Task;

    var errorCalled = false;

    await TaskHelper.AwaitWithTimeout(taskToAwait, 10, () => { }, ()=> errorCalled = true);

    Assert.IsTrue(errorCalled);
}

Also I recommend you to avoid using null. You can just pass the empty delegate as a parameter. Then you don't want to have null checks scattered all over your codebase.

I'd write the helper method as:

public static async Task AwaitWithTimeout(this Task task, int timeout, Action success, Action error)
{
    if (await Task.WhenAny(task, Task.Delay(timeout)) == task)
    {
        success();
    }
    else
    {
        error();
    }
}

Note that above method is an extension method; so you can call it with task instance.

await taskToAwait.AwaitWithTimeout(10, () => { }, ()=> errorCalled = true);//No nulls, just empty delegate

Thank you. Do you have any suggestions as how i should test the error scenario(s)? That is, input a task that never completes. For now, im still using Task.Delay() for this (just setting the delay very high, and the timeout somewhat low) — user1202032, Feb 26 '16 at 08:19
@user1202032 Sure. Updated my answer to address never ending task. — Sriram Sakthivel, Feb 26 '16 at 09:13
While the core concept (using `Task.WhenAny` to await on two tasks) is spot-on, the API around it feels dirty. `Action` for success is messy if you need to wire up another `Task` from it - you'd end up with ugly captures. I'd simply throw a `TimeoutException` in the timeout scenario and treat successful completion as, well, successful completion. In performance-critical cases a `Task` return type would also work (where `true` means successful completion and `false` means timeout). — Kirill Shlenskiy, Feb 26 '16 at 09:51
Then there's also the handling of the eventual completion of the timed-out `Task` (discussed here: http://blogs.msdn.com/b/pfxteam/archive/2012/10/05/how-do-i-cancel-non-cancelable-async-operations.aspx). And, of course, don't forget `ConfigureAwait(false)` - there's absolutely no reason not to have it in helper or library methods. — Kirill Shlenskiy, Feb 26 '16 at 09:53
@KirillShlenskiy Yes exactly my thoughts. `Task` makes more sense than delegates. Even better TimeoutExceptiion. I leave it to OP. — Sriram Sakthivel, Feb 26 '16 at 09:55

awaiting task with timeout

1 Answers1

Linked