c# - 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?

Answer 1

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