2

I was looking over some old code and I found an async method doing some async stuff and at the end calling await Task.CompletedTask; like below:

    public async Task SomeMethod()
    {
        //some code
        //...

        await _someService.DoStuffAsync();
        await Task.CompletedTask;
    }

From what I read it seems safe to remove the last line, as it does not have a functional impact over the method execution, since the Task.CompletedTask will return a task that is already completed so it will not await it, but I have second thoughts about this.

Is it safe to delete the last line? Is it redundant?

meJustAndrew
  • 6,011
  • 8
  • 50
  • 76
  • You could possibly debate about having `return Task.CompletedTask` there. But `await Task.CompletedTask` doesn't make any sense. Of course `return Task.CompletedTask` would mean you'd have to change the signature (remove `async`). – Wiktor Zychla May 28 '20 at 19:18
  • @WiktorZychla `return Task.CompletedTask` will result in `Task` return type for this method. – Guru Stron May 28 '20 at 19:19
  • 1
    `await Task.CompletedTask;` will complete synchronously: it's safe to remove that line. It was probably added before doing any actual async invocation within the method. – Tommaso Bertoni May 28 '20 at 19:19
  • @WiktorZychla completely agree, I would not care so much if it would have been a return, but it is just awaiting something that is completed. I wonder if I am missing something... – meJustAndrew May 28 '20 at 19:20
  • 1
    @meJustAndrew it should be safe to remove that code. – Guru Stron May 28 '20 at 19:20
  • @meJustAndrew: have you seen this pattern somewhere? What's the original cause of your doubts? – Wiktor Zychla May 28 '20 at 19:20
  • @WiktorZychla it is the first time I have seen this async, but I wonder if it will keep waiting for the current Task to change its state or something like this. I have seen [this question](https://stackoverflow.com/questions/59867966/does-await-task-completedtask-do-anything) (which is similar to mine, I know) and it made me more confused than I was before, so I don't know... – meJustAndrew May 28 '20 at 19:23
  • 1
    @meJustAndrew: you should not be confused. An async function *yields* when it returns and lets the calling context execute. As Stephen points out, `Task.Yield()` should be used. But you really don't need this. – Wiktor Zychla May 28 '20 at 19:26

2 Answers2

5

Yes, it's perfectly safe to remove that. This call doesn't do anything there other than checking that the task is already completed.

Years ago, when async was not yet available but Task was, we had to have

public Task Something()
{
    ....

    return Task.FromResult(0);
}

People adopted this when Task.CompletedTask was added and you will often see

public Task Something()
{
    ....

    return Task.CompletedTask;
}

This is something that makes sense if .... doesn't do anything async but you still have to return a Task.

When async is available and the method body awaits, the method doesn't have to return anything:

public async Task Something()
{
    await SomethingElse();        
}

But awaiting the Task.CompletedTask - I don't think there's a scenario where this would be useful.

Wiktor Zychla
  • 47,367
  • 6
  • 74
  • 106
1

Adding the line await Task.CompletedTask; is an easy way to suppress the compiler warning CS1998:

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Other than that it does nothing. So it is quite possible to add it while designing the API of a class, and then forget to remove it after writting the implementation. Feel free to remove it. Removing it will not cause any side-effects at all.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104