Async code smells and how to track them down with analyzers - Part I



Roslyn analyzers are great. Not only do they detect different issues in our code, but they are also able to propose solutions, thanks to accompanying code fixes. There’s one more, less-advertised aspect of analyzers: besides improving the quality of our codebase, they also improve the state of language knowledge in our teams. This is a real time-saver during the code review because the technical, language-related remarks are reported automatically in design/build time. There are many existing analyzer packages provided by the community (mostly free and open-source) with hundreds of different rules. This abundance raises the following question: Which analyzer packages should I use and which rules should be reported as errors? To help answer this question, I decided to start a blog post series that describes different code smells together with analyzers that can detect them. I will start with analyzers related to asynchronous programming. However, this area is full of traps, so this article is the first of two episodes devoted to async/await.

Analyzers for asynchronous programming 🔗︎

If you are looking for analyzers that can help you detect different issues with asynchronous code, you can find them in the following packages:

Most of those packages are not exclusively devoted to asynchronous programming, so I made an exercise by going through the complete list of offered rules and listed only those related to async code in the following sections:

Id Description Default Severity CodeFix
CA2007 Consider calling ConfigureAwait on the awaited task Warning Yes
CA2008 Do not create tasks without passing a TaskScheduler Warning No
CA2012 Use ValueTasks correctly Warning No
CA2247 Argument passed to TaskCompletionSource constructor should be TaskCreationOptions enum instead of TaskContinuationOptions enum Warning Yes

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
VSTHRD001 Avoid legacy thread switching methods Warning Yes
VSTHRD002 Avoid problematic synchronous waits Warning Yes
VSTHRD003 Avoid awaiting foreign Tasks Warning No
VSTHRD004 Await SwitchToMainThreadAsync Error No
VSTHRD010 Invoke single-threaded types on Main thread Warning No
VSTHRD011 Use AsyncLazy<T> Error No
VSTHRD012 Provide JoinableTaskFactory where allowed Warning No
VSTHRD100 Avoid async void methods Warning Yes
VSTHRD101 Avoid unsupported async delegates Warning No
VSTHRD102 Implement internal logic asynchronously Info No
VSTHRD103 Call async methods when in an async method Warning Yes
VSTHRD104 Offer async option Info No
VSTHRD105 Avoid method overloads that assume TaskScheduler.Current Warning No
VSTHRD106 Use InvokeAsync to raise async events Warning No
VSTHRD107 Await Task within using expression Error Yes
VSTHRD108 Assert thread affinity unconditionally Warning No
VSTHRD109 Switch instead of assert in async methods Error Yes
VSTHRD110 Observe result of async calls Warning No
VSTHRD111 Use .ConfigureAwait(bool) Hidden Yes
VSTHRD112 Implement System.IAsyncDisposable Info Yes
VSTHRD113 Check for System.IAsyncDisposable Info No
VSTHRD114 Avoid returning null from a Task-returning method. Warning Yes
VSTHRD200 Use Async naming convention Warning Yes

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
AsyncFixer01 Unnecessary async/await usage Warning Yes
AsyncFixer02 Long-running or blocking operations inside an async method Warning Yes
AsyncFixer03 Fire & forget async void methods Warning Yes
AsyncFixer04 Fire & forget async call inside a using block Warning No
AsyncFixer05 Downcasting from a nested task to an outer task Warning No

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
ASYNC0001 Asynchronous method names should end with Async Warning Yes
ASYNC0002 Non asynchronous method names shouldn’t end with Async Warning Yes
ASYNC0003 Avoid void returning asynchronous method Warning Yes
ASYNC0004 Use ConfigureAwait(false) on await expression Warning Yes

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
MA0004 Use .ConfigureAwait(false) Warning Yes
MA0022 Return Task.FromResult instead of returning null Info No
MA0032 Use an overload that have cancellation token Info No
MA0040 Flow the cancellation token when available Info No
MA0042 Do not use blocking cal Info No
MA0045 Do not use blocking call (make method async) Info No
MA0079 Flow a cancellation token using .WithCancellation() Info No
MA0080 Specify a cancellation token using .WithCancellation() Info No

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
RCS1046 Asynchronous method name should end with ‘Async’ None No
RCS1047 Non-asynchronous method name should not end with ‘Async’ Info No
RCS1090 Call ‘ConfigureAwait(false)’ Info Yes
RCS1174 Remove redundant async/await None Yes
RCS1210 Return Task.FromResult instead of returning null Warning Yes
RCS1229 Use async/await when necessary Info Yes

👉 Full list of supported rules

👉 Nuget package

👉 Official website

Id Description Default Severity CodeFix
AsyncifyInvocation This invocation could benefit from the use of Task async. Warning Yes
AsyncifyVariable This variable access could benefit from the use of Task async. Warning Yes

👉 Nuget package

👉 Official website

Async Code Smells 🔗︎

Here’s my list of the first seven most common issues related to asynchronous programming. For every issue, I provide entries for .editorconfig that configure analyzers that can detect it.

1. Redundant async/await 🔗︎

Using the async/await keywords results in implicit memory allocation required for the state machine responsible for orchestrating asynchronous invocations. When the awaited expression is the only one or the last statement in the function, it might be skipped. However, there’s a couple of concerns around this code optimization of which you should be aware. Before you start applying it, I highly recommend reading an excellent blog post about it from Stephen Cleary Eliding Async and Await.

❌ Wrong

async Task DoSomethingAsync()
{
    await Task.Yield(); //Reported diagnostics: AsyncFixer01, RCS1174
}

✔️ Correct

Task DoSomethingAsync()
{
    return Task.Yield();
}

🛠️ Configuration

# AsyncFixer01: Unnecessary async/await usage
dotnet_diagnostic.AsyncFixer01.severity = error

# RCS1174: Remove redundant async/await.
dotnet_diagnostic.RCS1174.severity = error

2. Calling synchronous method inside the async method 🔗︎

If we are writing asynchronous code then we should always prefer calling asynchronous methods if they exist. Many IO related APIs offer asynchronous counterparts of their well known synchronous methods. They should be our first choice.

❌ Wrong

async Task DoAsync(Stream file, byte[] buffer)
{
    file.Read(buffer, 0, 10); // VSTHRD103 is reported to use ReadAsync
}

✔️ Correct

async Task DoAsync(Stream file, byte[] buffer)
{
    await file.ReadAsync(buffer, 0, 10); // VSTHRD103 is reported to use ReadAsync
}

🛠️ Configuration

# AsyncFixer02: Long-running or blocking operations inside an async method
dotnet_diagnostic.AsyncFixer02.severity = error

# VSTHRD103: Call async methods when in an async method
dotnet_diagnostic.VSTHRD103.severity = error

3. Async Void method 🔗︎

There are two reasons why Async Void methods are harmful:

  • A caller of the method is not able to await asynchronous operation.
  • There’s no way to handle exception thrown by the method. If the exception occurs, your process crashes!!!

You should always use async Task instead of async void, unless it’s an event handler, but then you should guarantee yourself that the method can’t throw an exception.

❌ Wrong

async void DoSomethingAsync() // Reported diagnostics: VSTHRD100, AsyncFixer03, ASYNC0003
{
    await Task.Yield();
}

✔️ Correct

async Task DoSomethingAsync()
{
    await Task.Yield();
}

🛠️ Configuration

# AsyncFixer03: Fire & forget async void methods
dotnet_diagnostic.AsyncFixer03.severity = error

# VSTHRD100: Avoid async void methods
dotnet_diagnostic.VSTHRD100.severity = error

# ASYNC0003: Avoid void returning asynchronous method
dotnet_diagnostic.ASYNC0003.severity = error

4. Unsupported async delegates 🔗︎

If we pass the asynchronous lambda function the Action argument, the compiler generates async void method which downsides were described in the previous code smell. There are two solutions for this problem: if it’s possible, we should change the parameter type from Action to Func<Task>, otherwise we need to implement that callback delegate synchronously. It’s worth mentioning that some APIs already provide counterparts of their methods that accept Func<Task> for the callback parameters.

❌ Wrong

void DoSomething()
{ 
    Callback(async () => // Reported diagnostics: VSTHRD101
    {
        await Task.Yield();
    });
}

void Callback(Action action)
{
}

✔️ Correct

void DoSomething()
{  
    CallbackAsync(async () =>
    {
        await Task.Yield();
    });
}

void CallbackAsync(Func<Task> action)
{
}

🛠️ Configuration

# VSTHRD101: Avoid unsupported async delegates
dotnet_diagnostic.VSTHRD101.severity = error

5. Not awaited Task within using expression 🔗︎

System.Threading.Tasks.Task implements IDisposable interface. Calling a method returning task directly in using expressions results in Task disposal at the end of using block which is never an expected behavior.

❌ Wrong

using (CreateDisposableAsync()) //Reported diagnostics: VSTHRD107
{
    
}

✔️ Correct

using (await CreateDisposableAsync())
{
    
}

🛠️ Configuration

# VSTHRD107: Await Task within using expression
dotnet_diagnostic.VSTHRD107.severity = error

6. Not awaited Task inside the using block 🔗︎

If we skip the await keyword for asynchronous operation inside the using block, then the disposable object could be disposed before the asynchronous invocation finishes. This might result in incorrect behavior and very often ends with a runtime exception notifying that we are trying to operate on the object that is already disposed. This issue has two root causes: either it’s done by accident when somebody simply forgot about adding async/await keywords or it’s a result of incorrectly applied code optimization described in Redundant async/await code smell.

❌ Wrong

private Task<int> DoSomething() // Reported diagnostics: RCS1229
{
    using (var service = CreateService())
    {
        return service.GetAsync(); // Reported diagnostics: AsyncFixer04
    }
}

✔️ Correct

private async Task<int> DoSomething()
{
    using (var service = CreateService())
    {
        return await service.GetAsync();
    }
}

🛠️ Configuration

# AsyncFixer04: Fire & forget async call inside a using block
dotnet_diagnostic.AsyncFixer04.severity = error

# RCS1229: Use async/await when necessary.
dotnet_diagnostic.RCS1229.severity = error

There is a small difference between those two analyzers. RCS1229 is reported on the method level and AsyncFixer04 is reported in the return statement line which is IMHO more intuitive. I also observed that those diagnostics are not able to report the issue while using a new using var syntax:

private Task<int> DoSomething(CancellationToken cancellationToken)
{
    using var service = CreateService();
    return service.GetAsync(cancellationToken); // THIS SHOULD BE REPORTED!!!
}

In Roslynator this problem has been recently fixed issues/726 (it’s not released yet at the moment of publishing this article).

7. Unobserved result of asynchronous method 🔗︎

Missing await keyword before asynchronous operation will result in method completing before given async operation finishes. The method will behave non-deterministically and the outcome will be different from the expectations. What is worse, if the un-awaited expression throws an exception, it goes unnoticed and it doesn’t cause the process to crash, which makes it even harder to spot. You should always await asynchronous expression or assign a returned task to a variable and ensure that finally something awaits it.

❌ Wrong

async Task DoSomethingAsync()
{
    await DoSomethingAsync1(); 
    DoSomethingAsync2(); // Reported diagnostics: CS4014, VSTHRD110
    await DoSomethingAsync3(); 
}

✔️ Correct

async Task DoSomethingAsync()
{
    await DoSomethingAsync1(); 
    await DoSomethingAsync2();
    await DoSomethingAsync3();
}

🛠️ Configuration

# VSTHRD110: Observe result of async calls
dotnet_diagnostic.VSTHRD110.severity = error

# CS4014: Because this call is not awaited, execution of the current method continues before the call is completed
dotnet_diagnostic.CS4014.severity = error

There’s also a standard compiler warning CS4014 for that issue but it’s only able to report un-awaited expressions inside the async methods.

Summary 🔗︎

# Code smell AsyncFixer VS-Threading Roslyn.Analyzers Roslynator
1. Unnecessary async/await usage 🔎🛠️ AsyncFixer01 🔎🛠️ RCS1174
2. Call sync methods inside async method 🔎🛠️ AsyncFixer02 🔎🛠️ VSTHRD103
3. Async void methods 🔎🛠️ AsyncFixer03 🔎🛠️ VSTHRD100 🔎🛠️ ASYNC0003
4. Unsupported async delegates 🔎 VSTHRD101
5. Not awaited Task within using expression 🔎🛠️ VSTHRD107
6. Not awaited Task inside the using block 🔎 AsyncFixer04 🔎🛠️ RCS1229
7. Unobserved result of asynchronous method 🔎 VSTHRD110

🔎 - Analyzer

🛠️ - CodeFix


If you find this blog post useful and you think it's worth to share this idea with others, please don't hesitate to use these buttons below 👇

See Also


comments powered by Disqus