1

What are the differences? What problems I am introducing with these each approach? Is there any better way to 'wrap' sync call?

async Task Method()
{
    await Task.CompletedTask;
    externalService.syncCall();
}

async Task Method()
{
    externalService.syncCall();
    await Task.CompletedTask;
}

Task Method()
{
    externalService.syncCall();
    return Task.CompletedTask;
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Adrian Bartosz
  • 11
  • 1
  • 1
    The bottom (third) example, the one _without_ the `async` modifier, is the best one, imo. The first and second examples are silly because there is zero reason to `await Task.CompletedTask`. – Dai Feb 17 '22 at 10:21
  • 2
    @Dai `await Task.CompletedTask` is one way to suppress the CS1998 warning. As for the 3rd being "best", if `syncCall` throws then it breaks the expected exception semantics of an asynchronous function. – Johnathan Barclay Feb 17 '22 at 10:25
  • In what type of application do you intend to use the `Method` method? WinForms? ASP.NET? Console app? – Theodor Zoulias Feb 17 '22 at 10:33
  • @JohnathanBarclay If you're getting CS1998 then just remove the `await` modifier. I am aware that those kinds of methods [_should_ have their own `try/catch` and use `Task.FromException`](https://stackoverflow.com/questions/55542514/how-do-i-handle-exceptions-when-resolving-warning-cs1998-this-async-method-lack) but in-practice I haven't had any problem with not doing that, but I'm curious what your experiences are. – Dai Feb 17 '22 at 10:36
  • @Dai It's a nuanced scenario, but I wouldn't recommend a quick and dirty solution without having more context than this question currently contains. – Johnathan Barclay Feb 17 '22 at 10:52
  • @JohnathanBarclay Very correct! – Dai Feb 17 '22 at 10:57
  • You might enjoy this blog post by Stephen Cleary: [Eliding Async and Await](https://blog.stephencleary.com/2016/12/eliding-async-await.html) – Theodor Zoulias Feb 17 '22 at 11:09

2 Answers2

2

On the assumption that this method needs to return a Task due to interface constraints:

The first 2 are loosely equivalent and will behave in the same way.

The third will be slightly more efficient because it bypasses the state machine generated by the async keyword, however, eliding async makes you responsible for exception handling, and the current implementation is likely incomplete.

The consumer of an asynchronous (Task-returning) method expects exceptions to be thrown only at the point the Task is awaited.

For example:

var task = SomeMethodAsync();

// Do some other stuff..

await task; // Exception here if thrown in SomeMethodAsync()

Now if externalService.syncCall() throws in your Method(), this will happen:

var task = Method(); // Exception thrown here

// Do some other stuff..

await task;

To ensure Method() behaves as per the first example, you could do something like this:

Task Method()
{
    try { externalService.syncCall(); }
    catch (Exception e) { return Task.FromException(e); }
    return Task.CompletedTask;
}
Johnathan Barclay
  • 18,599
  • 1
  • 22
  • 35
  • 2
    To imitate completely the behavior of the `async` method, you also have to add this: `catch (OperationCanceledException oce) { return Task.FromCanceled(oce.CancellationToken); }`. Which is kind of ridiculous. If you have time to spend for this kind of micro-optimization, most probably you could spend it on other areas of your project, and reap much bigger benefits. This one might give you a few nanoseconds per operation at best. – Theodor Zoulias Feb 17 '22 at 11:02
  • @TheodorZoulias That would be ridiculous, unless `syncCall` is likely to throw an `OperationCanceledException`, and the consumer of `Method` needs to be able to differentiate between cancelled and other faulted tasks. A cancelled `Task` still throws when awaited which is the key consideration. – Johnathan Barclay Feb 17 '22 at 11:27
  • Johnathan if you are not going to imitate completely the behavior of an `async` method, then you might as well not `catch` any exception at all, and just assume that the caller of the method will invoke and then `await` it in one step: `await SomeMethodAsync();`. This is by far the most common usage pattern. – Theodor Zoulias Feb 17 '22 at 11:35
1

What are the differences?

The first two are essentially the same, although they're both doing unnecessary work with the await Task.CompletedTask;. I would say await Task.CompletedTask is a code smell, since it's really just adding code to do a noop in order to make a compiler warning go away. The compiler warning is there for a reason, and adding code just to silence it is missing the point of the warning.

The third one is quite different as soon as exceptions come into play. With Task-returning methods, callers expect exceptions to be placed on the returned task, but this one will throw them directly before returning the task.

Is there any better way to 'wrap' sync call?

The best way is to not:

void Method() => externalService.syncCall();

But sometimes you may need to. E.g., if you're implementing an interface that has a Task-returning Method. In that case, I recommend using a #pragma to disable the compiler warning. This has two advantages over await Task.CompletedTask: first, it explicitly notes in the code "I have considered this warning and evaluated it as spurious in this situation"; second, it doesn't add actual code just to do a noop.

#pragma warning disable 1998 // This async method lacks 'await' operators and will run synchronously.
async Task Method()
#pragma warning restore 1998 // This async method lacks 'await' operators and will run synchronously.
{
    externalService.syncCall();
}

If you need to do this often, collecting the #pragmas into a helper type would be cleaner.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810