Find non-awaited async method calls

Question

I've just stumbled across a rather dangerous scenario while migrating an ASP.NET application to the async/await model.

The situation is that I made a method async: async Task DoWhateverAsync(), changed the declaration in the interface to Task DoWhateverAsync() and hoped that the compiler would tell me where the code now is wrong, via that Warning. Well, tough luck. Wherever that object gets injected via the interface, the warning doesn't happen. :-(

This is dangerous. Is there any way to check automatically for non-awaited methods that return tasks? I don't mind a few warnings too many, but I wouldn't want to miss one.

Here's an example:

using System.Threading.Tasks;
namespace AsyncAwaitGames
{
    // In my real case, that method just returns Task.
    public interface ICallee { Task<int> DoSomethingAsync(); }

    public class Callee: ICallee
    {
        public async Task<int> DoSomethingAsync() => await Task.FromResult(0);
    }
    public class Caller
    {
        public void DoCall()
        {
            ICallee xxx = new Callee();

            // In my real case, the method just returns Task,
            // so there is no type mismatch when assigning a result 
            // either.
            xxx.DoSomethingAsync(); // This is where I had hoped for a warning.
        }
    }
}

It may not be as universal as a compiler warning, but I've generally relied on the return type. If I expect something to be a `string` and attempt to use it as a `string`, but really it's a `Task`, the compiler *will* tell me about that. (Doesn't help with `void/Task` ones though, I guess.) — David, Jan 18 '17 at 14:06
What was the declaration before? Are you changing a `void` method to one returning a `Task`? — Lee, Jan 18 '17 at 14:08
I think the logic behind the warning being based on it both being as `async` method *and* returning a `Task` type was to avoid introducing lots of new warnings for existing code bases that already used `Task`. The problem now is that `async` isn't really part of the signature/external contract and so (as you've probably observed) it's not valid in the interface. — Damien_The_Unbeliever, Jan 18 '17 at 14:15
"What was the declaration before?" was void, unfortunately, and changed to Task. — Volker, Jan 18 '17 at 14:27
I'm not understanding your scenario here. You say "hoped that the compiler would tell me where the code now is wrong". An `await` is a point in the program where the continuation of the await logically requires the completion of the task. The code before your change did not logically require the completion of the task, so why should it now after your change? What wrongness was introduced? — Eric Lippert, Jan 18 '17 at 16:38
The code before my change /assumed/ the completion of the task because DoSomething() was a synchronous method returning void. Then I changed the method to be async and return Task. If I had used the class directly, visual studio would have given me a warning. Since I cannot add "async" to a method signature in ICallee above, the code now looks like a normal method returning Task. Apparently this is not enough to trigger a warning for a) making DoCall() async and b) awaiting xxx.DoSomethingAsync(). This is understandable. For me the root cause is that I can't specify "async" in ICallee. — Volker, Jan 18 '17 at 18:42
"What wrongness was introduced?" Before: `public void DoCall() { ICallee xxx = new Callee(); xxx.DoSomething();}` (good). After: `public void DoCall() { ICallee xxx = new Callee(); xxx.DoSomethingAsync();}` (bad). Good: `public void DoCall() { ICallee xxx = new Callee(); await xxx.DoSomethingAsync();}` — Volker, Jan 18 '17 at 18:49
(In fact, the good code is supposed to be `public async Task DoCallAsync() { ICallee xxx = new Callee(); await xxx.DoSomethingAsync();}` of course, but once I got that await in the method body, the compiler then nags me in the proper way. It's being reminded of that missing await, that doesn't work right now.) — Volker, Jan 18 '17 at 19:37
The main reason this isn't warned is because it's ok to start an async method and not wait on it. However; if you did plan on awaiting it you would mark the method as async and then the compiler would say, hey you intended to await and didn't. — Michael Puckett II, Nov 20 '17 at 18:19

score 32 · Answer 1 · answered Jan 06 '18 at 16:12

32

After quite some difficulties with this problem I decided to create an Analyzer with code fix to solve it.

The code is available here: https://github.com/ykoksen/unused-task-warning

It is also as a NuGet package that can be used as an analyzer for a project (when it is build): https://www.nuget.org/packages/Lindhart.Analyser.MissingAwaitWarning/#

Furthermore it is also available as a Visual Studio Extension (for 2017). However this only analyses currently open files, so I'd recommend using the NuGet package. The extension is available here (or search for it in Visual Studio): https://marketplace.visualstudio.com/items?itemName=Lindhart.missingAwaitWarning#overview

The code for the analyzer:

    public override void Initialize(AnalysisContext context)
    {
        context.RegisterSyntaxNodeAction(AnalyseSymbolNode, SyntaxKind.InvocationExpression);
    }

    private void AnalyseSymbolNode(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext)
    {
        if (syntaxNodeAnalysisContext.Node is InvocationExpressionSyntax node)
        {
            if (syntaxNodeAnalysisContext
                    .SemanticModel
                    .GetSymbolInfo(node.Expression, syntaxNodeAnalysisContext.CancellationToken)
                    .Symbol is IMethodSymbol methodSymbol)
            {
                if (node.Parent is ExpressionStatementSyntax)
                {
                    // Only checks for the two most common awaitable types. In principle this should instead check all types that are awaitable
                    if (EqualsType(methodSymbol.ReturnType, typeof(Task), typeof(ConfiguredTaskAwaitable)))
                    {
                        var diagnostic = Diagnostic.Create(Rule, node.GetLocation(), methodSymbol.ToDisplayString());

                        syntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
                    }
                }
            }
        }
    }

    /// <summary>
    /// Checks if the <paramref name="typeSymbol"/> is one of the types specified
    /// </summary>
    /// <param name="typeSymbol"></param>
    /// <param name="type"></param>
    /// <returns></returns>
    /// <remarks>This method should probably be rewritten so it doesn't merely compare the names, but instead the actual type.</remarks>
    private static bool EqualsType(ITypeSymbol typeSymbol, params Type[] type)
    {
        var fullSymbolNameWithoutGeneric = $"{typeSymbol.ContainingNamespace.ToDisplayString()}.{typeSymbol.Name}";
        return type.Any(x => fullSymbolNameWithoutGeneric.Equals(x.FullName));
    }

answered Jan 06 '18 at 16:12

Ykok

1,313
13
15

1

Does this catch cases where I've put `Task t = GetSomethingAsync()` or `var t = GetSometingAsync()` which obscures the compiler warning. – Simon_Weaver Sep 12 '18 at 05:05
1

Where do I see the errors. I added the package and I have a bad call to a Task, but I don't see any additional errors beyond the compiler's standard messages. – Simon_Weaver Sep 12 '18 at 05:11
@Simon_Weaver it does not catch the cases where you assign the task to a variable (ex. `Task t = GetSomethingAsync()`). In these cases it makes the assumption that you actively made a choice - and anyway you have the `Task` visibly there. – Ykok Sep 13 '18 at 18:09
1

@Simon_Weaver to see the warnings you'll have to be using Visual Studio 2017 (I believe, it might work in 2015). Furthermore to get it as compiler warning you'll have to add the NuGet package to your project. If you use the Visual Studio extension you will only see it as a warning when you have the file open. – Ykok Sep 13 '18 at 18:11
1

Thanks for the info. Problem is I must have done this somewhere assuming it wasn’t going to throw an exception - and now it does - and I have no clue where. Very very tricky. I have it sending me a text message every time it happens to look for a pattern. I may have done var T or Task t but I searched and couldn’t find any. – Simon_Weaver Sep 13 '18 at 18:27
Also when I added the nuget I got some error about System.Runtime that I didn’t know how to fix. But I guess it won’t find the problem so I uninstalled it for now. If you can think of a clever way to see everywhere I assigned a task to a variable (I’ve probably only ever done it 10-20 times) I’d love to hear it :) – Simon_Weaver Sep 13 '18 at 18:29
@Simon_Weaver You might be able to find the exception using an unhandled exception handler: https://stackoverflow.com/questions/406385/handling-unhandled-exceptions-problem. Not sure though. – Ykok Sep 14 '18 at 20:01
@Ykok unfortunately even when I listen for TaskScheduler.UnobservedTaskException and try to handle the exception from the AggregateException it returns I only get this weird Null Reference exception that's nothing to do with my code :-( – Simon_Weaver Sep 14 '18 at 20:57
@Ykok In a solution with 23 projects, do I need to add the NuGet package to each of them, if I want all the projects to be analyzed? – Miguel Hughes Nov 21 '19 at 12:14
@MiguelHughes Yes - I believe so – Ykok Nov 24 '19 at 14:08
1

Thank you for this plugin! Does exactly what I need. We had too many unawaited calls in void methods which where not detected. Most of those voids should have been Tasks – Thomas Eyde Mar 29 '22 at 06:38

Major · Answer 2 · 2019-09-18T09:25:42.027

You have a few options:

This is the simplest "Caveman" solution use the built in VS search functionality (CTRL + SHIFT + F) search in Entire solution, also under Find Options click on the checkbox Use Regular expression and use this regex: (?<!await|task(.*))\s([_a-zA-Z0-9\.])*Async\( It assumes you post fixed all your async method with the Async keyword and the method call is in one line. If it is not true then do not use it (or add the missing validations to the expression).
Use some 3rd party code analyzer tool, Nuget package. ReSharper is very popular and I believe it able to detect this problems or you can create your own rules.
My choice would be to use Roslyn (@Volker provided one solution). You can create your own rule set with code fix solutions (light bulb icon will show your code fix) so this is the best.
UPDATE: VS 2019 checks for this problem by default and gives warnings.

How to use Roslyn:

You have to install .NET Compiler Platform SDK: from here
Use VS 2017 Version 15.2 (or higher)
Create a new project File -> New -> Project, under the Extensibility group select: Analyzer with Code Fix (Nuget + VSIX) You have to target .NET Framework 4.6.2 to create this project.

You can copy paste the previous solution. Create

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AsyncAwaitAnalyzer : DiagnosticAnalyzer
{ ...
}

class with logic, to detect the issue. And create

[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AsyncAwaitCodeFixProvider)), Shared]
public class AsyncAwaitCodeFixProvider : CodeFixProvider
{ ...
}

class to provide fixing suggestions (add await) to the problem.

After a success build you will get your own .wsix package you can install it to your VS instance and after a VS restart is should start pick up the problems.

Sorry, never created these before...which sections of code in "previous solution" are you referring to explicitly? — crichavin, Sep 17 '19 at 23:43

Volker · Accepted Answer · 2017-01-20T10:09:17.173

8

In the end, we used roslyn to find all instances where a return value of Task or Task<> was ignored:

if (methodSymbol.ReturnType.Equals(syntaxNodeAnalysisContext.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Task).FullName)))
{
    // For all such symbols, produce a diagnostic.
    var diagnostic = Diagnostic.Create(Rule, node.GetLocation(), methodSymbol.ToDisplayString());

    syntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
}
if (((INamedTypeSymbol) methodSymbol.ReturnType).IsGenericType && ((INamedTypeSymbol) methodSymbol.ReturnType).BaseType.Equals(syntaxNodeAnalysisContext.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Task).FullName)))
{
    // For all such symbols, produce a diagnostic.
    var diagnostic = Diagnostic.Create(Rule, node.GetLocation(), methodSymbol.ToDisplayString());

    syntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
}

edited Jan 20 '17 at 10:09

answered Jan 19 '17 at 14:42

Volker

1,753
2
18
28

13

I request you to please add some context/commentary around the suggestion you have shared. It will help the asker and other future readers to understand your post better. – RBT Jan 20 '17 at 00:35
1

Could you please add the full implementation you did with the Roslyn extension? I've tried to make it work, but am quite new with these extensions. – Ykok Nov 10 '17 at 15:23
This is what i was gonna tell u. Thanx to Roslyn :) – DLL_Whisperer Nov 17 '17 at 12:31
@JoshMouch - check my post below :-) – Ykok Jan 07 '18 at 14:58
1

I tried implementing this, but it simply gives warnings on too much. I used it before I implemented my own solution (see my answer), and as I remember it simply found all places where a method returns a `Task`. – Ykok Jan 07 '18 at 15:02
2

@Volker to make your answer more complete, elaborate on where and how to add the above – Korayem Jun 26 '18 at 09:23

Martin Liversage · Answer 4 · 2017-01-18T17:56:53.477

The compiler will emit warning CS4014 but it is only emitted if the calling method is async.

No warning:

Task CallingMethod() {
    DoWhateverAsync();
    // More code that eventually returns a task.
}

Warning CS4014: Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.

async Task CallingMethod() {
    DoWhateverAsync();
}

This is not terrible useful in your specific case because you have to find all the places where DoWhateverAsync is called and change them to get the warning and then fix the code. But you wanted to use the compiler warning to find these calls in the first place.

I suggest that you use Visual Studio to find all usages of DoWhateverAsync. You will have to modify the surrounding code anyway either by going through compiler warnings or by working through a list of usages.

@David No, in the OP's situation *the calling method isn't `async`* because he's in the process of transitioning from a synchronous to an asynchronous model. — Servy, Jan 18 '17 at 16:00
Also no warning if you do `Task t = DoWhateverAsync()` which may have been deliberately used to hide the compiler warning in the first place. eek — Simon_Weaver, Sep 12 '18 at 05:08

score 1 · Answer 5 · answered Nov 18 '17 at 06:43

1

You can add a specific warning in VS Project properties as one that throws a compilation error, like described here

You can add a semi-colon separated list of warning codes e.g. CS4014, and the compiler will fail to compile if you're not awaiting an async method.

Here's a screenshot with VS2017: VS2017 configuration to throw compiler errors for warning CS4014

answered Nov 18 '17 at 06:43

theDurbari

29
3

The problem is that absolutely no warnings are produced in the described scenario. – Ykok Nov 18 '17 at 21:49
3

Actually, as @Martin Liversage mentioned, the calling function (in this case `DoCall`) has to be an `async` function in order for that compiler warning to be thrown. So I stand corrected, since this doesn't apply to your case. I think the way to handle this would be by some code analyzer, either Roslyn or ReSharper etc. – theDurbari Nov 20 '17 at 02:42

score 0 · Answer 6 · answered Jun 22 '21 at 21:32

0

I found a great Visual Studio extension/NuGet package for this. Saves you having to create your own like other answers suggest.

https://github.com/ykoksen/unused-task-warning

Once I installed the Visual Studio extension I went to Analyze/Run Code Analysis/On Solution and it found several places where I had this issue.

answered Jun 22 '21 at 21:32

phillhutt

183
1
5

1

This *is* the solution from the accepted answer, which includes the same link you have posted, along with more information. – Ergwun Oct 04 '21 at 10:02
@Ergwun, the accepted answer does not reference the extension. But I see the highest rated answer does. Somehow I missed that. – phillhutt Oct 04 '21 at 20:23
Yeah, you are right - it is the top voted one, not the accepted one. I think they recently changed it so the accepted answer is not necessarily top, so the currently top-voted answer would not have been as prominent before. – Ergwun Oct 04 '21 at 23:38

Find non-awaited async method calls

6 Answers6

Linked

Related