Treat missing .ConfigureAwait(false) as errors (and fix) (#633)

I enabled the new .NET Analyzer for the project and then used an Editor Config rule to treat CA2007 as an error. This diagnostic is for any awaited task that does not have an explicit trailing `.ConfigureAwait(false)` call, an annoying but very necessary configuration for asynchronous .NET library code (such as OmniSharp). The bugs caused by these missing calls are strange. In the case of PowerShell Editor Services, an LSP server using OmniSharp and powering the PowerShell Extension for VS Code, it showed up as a hang when the server executed user code that used objects from `System.Windows.Forms`. This is because that .NET library sets its own synchronization context, which is exactly where `ConfigureAwait(continueOnCapturedContext: false)` comes into play. The default value is `true` which roughly means that the awaited tasks will only run on their own context. So when the context is changed (because of `System.Windows.Forms` or other code introducing a new synchronization context) the task will never be continued, resulting in this hang. By configuring this to `false` we allow the tasks to continue regardless of the new context. See this .NET blog post for more details: https://devblogs.microsoft.com/dotnet/configureawait-faq/ Note that elsewhere in the codebase we've been very careful to set it to false, but we've not been perfect. Treating this diagnostic as an error will allow us to be as perfect as possible about it.
OmniSharp · Aug 10, 2021 · 5281160 · 5281160
1 parent 1969872
commit 5281160
Show file tree

Hide file tree

Showing 11 changed files with 32 additions and 25 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -128,7 +128,11 @@ resharper_web_config_module_not_resolved_highlighting=warning
 resharper_web_config_type_not_resolved_highlighting=warning
 resharper_web_config_wrong_module_highlighting=warning
 
+# .NET Analzyer settings
+# VSTHRD200: Use "Async" suffix for awaitable methods
 dotnet_diagnostic.VSTHRD200.severity = none
+# CA2007: Do not directly await a Task
+dotnet_diagnostic.CA2007.severity = error
 
 [*.{cs,cshtml}]
 charset=utf-8

diff --git a/Directory.Build.props b/Directory.Build.props
@@ -13,6 +13,8 @@
         <PackageProjectUrl>https://github.com/OmniSharp/csharp-language-server-protocol</PackageProjectUrl>
         <PackageTags>lsp;language server;language server protocol;language client;language server client</PackageTags>
         <AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)\lsp.snk</AssemblyOriginatorKeyFile>
+        <!-- See: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview -->
+        <EnableNETAnalyzers>true</EnableNETAnalyzers>
     </PropertyGroup>
     <PropertyGroup>
         <EmbedUntrackedSources>true</EmbedUntrackedSources>

diff --git a/sample/SampleServer/Program.cs b/sample/SampleServer/Program.cs
@@ -83,7 +83,7 @@ private static async Task MainAsync(string[] args)
                                 );
                                 workDone = manager;
 
-                                await Task.Delay(2000);
+                                await Task.Delay(2000).ConfigureAwait(false);
 
                                 manager.OnNext(
                                     new WorkDoneProgressReport {
@@ -102,7 +102,7 @@ private static async Task MainAsync(string[] args)
                                     }
                                 );
 
-                                await Task.Delay(2000);
+                                await Task.Delay(2000).ConfigureAwait(false);
 
                                 workDone.OnNext(
                                     new WorkDoneProgressReport {
@@ -115,12 +115,12 @@ private static async Task MainAsync(string[] args)
                         )
                        .OnStarted(
                             async (languageServer, token) => {
-                                using var manager = await languageServer.WorkDoneManager.Create(new WorkDoneProgressBegin { Title = "Doing some work..." });
+                                using var manager = await languageServer.WorkDoneManager.Create(new WorkDoneProgressBegin { Title = "Doing some work..." }).ConfigureAwait(false);
 
                                 manager.OnNext(new WorkDoneProgressReport { Message = "doing things..." });
-                                await Task.Delay(10000);
+                                await Task.Delay(10000).ConfigureAwait(false);
                                 manager.OnNext(new WorkDoneProgressReport { Message = "doing things... 1234" });
-                                await Task.Delay(10000);
+                                await Task.Delay(10000).ConfigureAwait(false);
                                 manager.OnNext(new WorkDoneProgressReport { Message = "doing things... 56789" });
 
                                 var logger = languageServer.Services.GetService<ILogger<Foo>>();
@@ -130,7 +130,7 @@ private static async Task MainAsync(string[] args)
                                     }, new ConfigurationItem {
                                         Section = "terminal",
                                     }
-                                );
+                                ).ConfigureAwait(false);
 
                                 var baseConfig = new JObject();
                                 foreach (var config in languageServer.Configuration.AsEnumerable())
@@ -149,9 +149,9 @@ private static async Task MainAsync(string[] args)
                                 logger.LogInformation("Scoped Config: {Config}", scopedConfig);
                             }
                         )
-            );
+            ).ConfigureAwait(false);
 
-            await server.WaitForExit;
+            await server.WaitForExit.ConfigureAwait(false);
         }
     }
 

diff --git a/sample/SampleServer/SemanticTokensHandler.cs b/sample/SampleServer/SemanticTokensHandler.cs
@@ -25,15 +25,15 @@ public SemanticTokensHandler(ILogger<SemanticTokensHandler> logger) =>
             SemanticTokensParams request, CancellationToken cancellationToken
         )
         {
-            var result = await base.Handle(request, cancellationToken);
+            var result = await base.Handle(request, cancellationToken).ConfigureAwait(false);
             return result;
         }
 
         public override async Task<SemanticTokens?> Handle(
             SemanticTokensRangeParams request, CancellationToken cancellationToken
         )
         {
-            var result = await base.Handle(request, cancellationToken);
+            var result = await base.Handle(request, cancellationToken).ConfigureAwait(false);
             return result;
         }
 
@@ -42,7 +42,7 @@ public SemanticTokensHandler(ILogger<SemanticTokensHandler> logger) =>
             CancellationToken cancellationToken
         )
         {
-            var result = await base.Handle(request, cancellationToken);
+            var result = await base.Handle(request, cancellationToken).ConfigureAwait(false);
             return result;
         }
 
@@ -54,7 +54,7 @@ CancellationToken cancellationToken
             using var typesEnumerator = RotateEnum(SemanticTokenType.Defaults).GetEnumerator();
             using var modifiersEnumerator = RotateEnum(SemanticTokenModifier.Defaults).GetEnumerator();
             // you would normally get this from a common source that is managed by current open editor, current active editor, etc.
-            var content = await File.ReadAllTextAsync(DocumentUri.GetFileSystemPath(identifier), cancellationToken);
+            var content = await File.ReadAllTextAsync(DocumentUri.GetFileSystemPath(identifier), cancellationToken).ConfigureAwait(false);
             await Task.Yield();
 
             foreach (var (line, text) in content.Split('\n').Select((text, line) => (line, text)))

diff --git a/sample/SampleServer/TextDocumentHandler.cs b/sample/SampleServer/TextDocumentHandler.cs
@@ -51,7 +51,7 @@ public override async Task<Unit> Handle(DidOpenTextDocumentParams notification,
         {
             await Task.Yield();
             _logger.LogInformation("Hello world!");
-            await _configuration.GetScopedConfiguration(notification.TextDocument.Uri, token);
+            await _configuration.GetScopedConfiguration(notification.TextDocument.Uri, token).ConfigureAwait(false);
             return Unit.Value;
         }
 
@@ -84,7 +84,7 @@ CancellationToken cancellationToken
         )
         {
             // you would normally get this from a common source that is managed by current open editor, current active editor, etc.
-            var content = await File.ReadAllTextAsync(DocumentUri.GetFileSystemPath(request), cancellationToken);
+            var content = await File.ReadAllTextAsync(DocumentUri.GetFileSystemPath(request), cancellationToken).ConfigureAwait(false);
             var lines = content.Split('\n');
             var symbols = new List<SymbolInformationOrDocumentSymbol>();
             for (var lineIndex = 0; lineIndex < lines.Length; lineIndex++)
@@ -160,31 +160,31 @@ CancellationToken cancellationToken
             using var partialResults = _progressManager.For(request, cancellationToken);
             if (partialResults != null)
             {
-                await Task.Delay(2000, cancellationToken);
+                await Task.Delay(2000, cancellationToken).ConfigureAwait(false);
 
                 reporter.OnNext(
                     new WorkDoneProgressReport {
                         Cancellable = true,
                         Percentage = 20
                     }
                 );
-                await Task.Delay(500, cancellationToken);
+                await Task.Delay(500, cancellationToken).ConfigureAwait(false);
 
                 reporter.OnNext(
                     new WorkDoneProgressReport {
                         Cancellable = true,
                         Percentage = 40
                     }
                 );
-                await Task.Delay(500, cancellationToken);
+                await Task.Delay(500, cancellationToken).ConfigureAwait(false);
 
                 reporter.OnNext(
                     new WorkDoneProgressReport {
                         Cancellable = true,
                         Percentage = 50
                     }
                 );
-                await Task.Delay(500, cancellationToken);
+                await Task.Delay(500, cancellationToken).ConfigureAwait(false);
 
                 partialResults.OnNext(
                     new[] {
@@ -209,7 +209,7 @@ CancellationToken cancellationToken
                         Percentage = 70
                     }
                 );
-                await Task.Delay(500, cancellationToken);
+                await Task.Delay(500, cancellationToken).ConfigureAwait(false);
 
                 reporter.OnNext(
                     new WorkDoneProgressReport {

diff --git a/src/Dap.Client/DebugAdapterClient.cs b/src/Dap.Client/DebugAdapterClient.cs
@@ -154,7 +154,7 @@ await DebugAdapterEventingHelper.Run(
                 token
             ).ConfigureAwait(false);
 
-            await _initializedComplete.ToTask(token, _scheduler);
+            await _initializedComplete.ToTask(token, _scheduler).ConfigureAwait(false);
 
             await DebugAdapterEventingHelper.Run(
                 _startedDelegates,

diff --git a/src/Dap.Testing/DebugAdapterProtocolTestBase.cs b/src/Dap.Testing/DebugAdapterProtocolTestBase.cs
@@ -79,7 +79,7 @@ Action<DebugAdapterServerOptions> serverOptionsAction
 
             return await Observable.FromAsync(_client.Initialize)
                                    .ForkJoin(Observable.FromAsync(_server.Initialize), (_, _) => ( _client, _server ))
-                                   .ToTask(CancellationToken);
+                                   .ToTask(CancellationToken).ConfigureAwait(false);
         }
     }
 }
diff --git a/src/JsonRpc/OutputHandler.cs b/src/JsonRpc/OutputHandler.cs
@@ -132,7 +132,7 @@ private async Task ProcessOutputStream(CancellationToken cancellationToken)
             {
                 do
                 {
-                    var value = await _queue.ReadAsync(cancellationToken);
+                    var value = await _queue.ReadAsync(cancellationToken).ConfigureAwait(false);
                     if (value is ITraceData traceData)
                     {
                         _activityTracingStrategy?.ApplyOutgoing(traceData);

diff --git a/src/Protocol/Features/Document/CallHierarchyFeature.cs b/src/Protocol/Features/Document/CallHierarchyFeature.cs
@@ -393,7 +393,7 @@ protected CallHierarchyHandlerBase() : this(Guid.NewGuid())
 
             public sealed override async Task<Container<CallHierarchyItem>?> Handle(CallHierarchyPrepareParams request, CancellationToken cancellationToken)
             {
-                var response = await HandlePrepare(request, cancellationToken);
+                var response = await HandlePrepare(request, cancellationToken).ConfigureAwait(false);
                 return Container<CallHierarchyItem>.From(response?.Select(CallHierarchyItem.From)!);
             }
 

diff --git a/src/Server/LanguageServer.Shutdown.cs b/src/Server/LanguageServer.Shutdown.cs
@@ -24,8 +24,8 @@ public partial class LanguageServer : IExitHandler, IShutdownHandler
         #pragma warning disable VSTHRD100
         public async void ForcefulShutdown()
         {
-            await ( (IShutdownHandler) this ).Handle(ShutdownParams.Instance, CancellationToken.None);
-            await ( (IExitHandler) this ).Handle(ExitParams.Instance, CancellationToken.None);
+            await ( (IShutdownHandler) this ).Handle(ShutdownParams.Instance, CancellationToken.None).ConfigureAwait(false);
+            await ( (IExitHandler) this ).Handle(ExitParams.Instance, CancellationToken.None).ConfigureAwait(false);
         }
 
         async Task<Unit> IRequestHandler<ExitParams, Unit>.Handle(ExitParams request, CancellationToken token)

diff --git a/test/.editorconfig b/test/.editorconfig
@@ -1,4 +1,5 @@
 [*]
+dotnet_diagnostic.CA2007.severity = none
 dotnet_diagnostic.CS0618.severity = none
 dotnet_diagnostic.CS4014.severity = none
 dotnet_diagnostic.CS1998.severity = none