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:
- Microsoft.CodeAnalysis.FxCopAnalyzers
- Microsoft.VisualStudio.Threading.Analyzers
- AsyncFixer
- Roslyn.Analyzers
- Meziantou.Analyzer
- Roslynator
- Asyncify
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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