Use combined cancellation token throughout AnalyzeDocument.

- Apply suggestions.
OmniSharp · Sep 25, 2023 · 3c3bc93 · 3c3bc93
1 parent ddbaef8
commit 3c3bc93
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 17 deletions.
diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AsyncAnalyzerWorkQueue.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AsyncAnalyzerWorkQueue.cs
@@ -13,7 +13,7 @@ namespace OmniSharp.Roslyn.CSharp.Workers.Diagnostics
     public class AsyncAnalyzerWorkQueue
     {
         private readonly object _lock = new();
-        private readonly Queue _forground = new();
+        private readonly Queue _foreground = new();
         private readonly Queue _background = new();
         private readonly ILogger<AnalyzerWorkQueue> _logger;
         private TaskCompletionSource<object?> _takeWorkWaiter = new(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -28,7 +28,7 @@ public int PendingCount
             get
             {
                 lock (_lock)
-                    return _forground.PendingCount + _background.PendingCount;
+                    return _foreground.PendingCount + _background.PendingCount;
             }
         }
 
@@ -38,15 +38,16 @@ public void PutWork(IReadOnlyCollection<DocumentId> documentIds, AnalyzerWorkTyp
             {
                 foreach (var documentId in documentIds)
                 {
-                    _forground.RequestCancellationIfActive(documentId);
+                    _foreground.RequestCancellationIfActive(documentId);
                     _background.RequestCancellationIfActive(documentId);
 
                     if (workType == AnalyzerWorkType.Foreground)
-                        _forground.Enqueue(documentId);
+                        _foreground.Enqueue(documentId);
                     else if (workType == AnalyzerWorkType.Background)
                         _background.Enqueue(documentId);
                 }
 
+                // Complete the work waiter task to allow work to be taken from the queue.
                 if (!_takeWorkWaiter.Task.IsCompleted)
                     _takeWorkWaiter.SetResult(null);
             }
@@ -62,15 +63,15 @@ public async Task<QueueItem> TakeWorkAsync(CancellationToken cancellationToken =
 
                 lock (_lock)
                 {
-                    if (_forground.TryDequeue(out var documentId, out var cancellationTokenSource))
+                    if (_foreground.TryDequeue(out var documentId, out var cancellationTokenSource))
                     {
                         return new QueueItem
                         (
                             DocumentId: documentId,
                             CancellationToken: cancellationTokenSource.Token,
                             AnalyzerWorkType: AnalyzerWorkType.Foreground,
-                            DocumentCount: _forground.MaximumPendingCount,
-                            DocumentCountRemaining: _forground.PendingCount
+                            DocumentCount: _foreground.MaximumPendingCount,
+                            DocumentCountRemaining: _foreground.PendingCount
                         );
                     }
                     else if (_background.TryDequeue(out documentId, out cancellationTokenSource))
@@ -85,12 +86,15 @@ public async Task<QueueItem> TakeWorkAsync(CancellationToken cancellationToken =
                         );
                     }
 
-                    if (_forground.PendingCount == 0 && _background.PendingCount == 0 && _takeWorkWaiter.Task.IsCompleted)
+                    if (_foreground.PendingCount == 0 && _background.PendingCount == 0 && _takeWorkWaiter.Task.IsCompleted)
                         _takeWorkWaiter = new TaskCompletionSource<object?>(TaskCreationOptions.RunContinuationsAsynchronously);
 
                     awaitTask = _takeWorkWaiter.Task;
                 }
 
+                // There is no chance of the default cancellation token being cancelled, so we can
+                // simply wait for work to be queued. Otherwise, we need to handle the case that the
+                // token is cancelled before we have work to return.
                 if (cancellationToken == default)
                 {
                     await awaitTask.ConfigureAwait(false);
@@ -103,9 +107,7 @@ public async Task<QueueItem> TakeWorkAsync(CancellationToken cancellationToken =
                     {
                         await Task.WhenAny(awaitTask, tcs.Task).ConfigureAwait(false);
                     }
-
                 }
-
             }
         }
 
@@ -114,7 +116,7 @@ public void WorkComplete(QueueItem item)
             lock (_lock)
             {
                 if (item.AnalyzerWorkType == AnalyzerWorkType.Foreground)
-                    _forground.WorkComplete(item.DocumentId, item.CancellationToken);
+                    _foreground.WorkComplete(item.DocumentId, item.CancellationToken);
                 else if (item.AnalyzerWorkType == AnalyzerWorkType.Background)
                     _background.WorkComplete(item.DocumentId, item.CancellationToken);
             }
@@ -128,7 +130,7 @@ public async Task WaitForegroundWorkComplete(CancellationToken cancellationToken
             Task waitForgroundTask;
 
             lock (_lock)
-                waitForgroundTask = _forground.GetWaiter();
+                waitForgroundTask = _foreground.GetWaiter();
 
             if (waitForgroundTask.IsCompleted)
                 return;

diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs
@@ -316,12 +316,12 @@ private async Task<ImmutableArray<Diagnostic>> AnalyzeDocument(Project project,
                     reportSuppressedDiagnostics: false));
 
                 Task<ImmutableArray<Diagnostic>> semanticDiagnosticsWithAnalyzers = compilationWithAnalyzers
-                    .GetAnalyzerSemanticDiagnosticsAsync(documentSemanticModel, filterSpan: null, cancellationToken);
+                    .GetAnalyzerSemanticDiagnosticsAsync(documentSemanticModel, filterSpan: null, combinedCancellation.Token);
 
                 Task<ImmutableArray<Diagnostic>> syntaxDiagnosticsWithAnalyzers = compilationWithAnalyzers
-                    .GetAnalyzerSyntaxDiagnosticsAsync(syntaxTree, cancellationToken);
+                    .GetAnalyzerSyntaxDiagnosticsAsync(syntaxTree, combinedCancellation.Token);
 
-                ImmutableArray<Diagnostic> documentSemanticDiagnostics = documentSemanticModel.GetDiagnostics(null, cancellationToken);
+                ImmutableArray<Diagnostic> documentSemanticDiagnostics = documentSemanticModel.GetDiagnostics(null, combinedCancellation.Token);
 
                 await Task.WhenAll(syntaxDiagnosticsWithAnalyzers, semanticDiagnosticsWithAnalyzers);
 
@@ -331,7 +331,7 @@ private async Task<ImmutableArray<Diagnostic>> AnalyzeDocument(Project project,
                     .Concat(documentSemanticDiagnostics)
                     .ToImmutableArray();
             }
-            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+            catch (OperationCanceledException) when (combinedCancellation.Token.IsCancellationRequested)
             {
                 throw;
             }

diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/AsyncAnalyzerWorkerQueueFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/AsyncAnalyzerWorkerQueueFacts.cs
@@ -136,7 +136,7 @@ public async Task WhenMultipleThreadsAreConsumingAnalyzerWorkerQueueItWorksAsExp
         }
 
         [Fact]
-        public async Task WheNewnWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnotherOneToGetReady()
+        public async Task WhenNewWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnotherOneToGetReady()
         {
             var queue = new AsyncAnalyzerWorkQueue(new LoggerFactory());
             var document1 = CreateTestDocumentId();