[LSP] Fix bug where request contexts were not created serially

src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs

@@ -671,7 +671,7 @@ public async ValueTask DisposeAsync()
                 // Some tests manually call shutdown, so avoid calling shutdown twice if already called.
                 if (!LanguageServer.HasShutdownStarted)
                 {
-                    await LanguageServer.GetTestAccessor().ShutdownServerAsync();
+                    await LanguageServer.GetTestAccessor().ShutdownServerAsync("Disposing of test lsp server");
                 }
 
                 await LanguageServer.GetTestAccessor().ExitServerAsync();

src/Features/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/AbstractLanguageServer.cs

@@ -293,9 +293,9 @@ internal TestAccessor(AbstractLanguageServer<TRequestContext> server)
 
         internal bool HasShutdownStarted() => _server.HasShutdownStarted;
 
-        internal Task ShutdownServerAsync()
+        internal Task ShutdownServerAsync(string message = "Shutting down")
         {
-            return _server.ShutdownAsync();
+            return _server.ShutdownAsync(message);
         }
 
         internal Task ExitServerAsync()

src/Features/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/IQueueItem.cs

@@ -17,9 +17,18 @@ public interface IQueueItem<TRequestContext>
     /// <summary>
     /// Executes the work specified by this queue item.
     /// </summary>
+    /// <param name="requestContext">the context created by <see cref="CreateRequestContextAsync(CancellationToken)"/></param>
     /// <param name="cancellationToken" />
     /// <returns>A <see cref="Task "/> which completes when the request has finished.</returns>
-    Task StartRequestAsync(CancellationToken cancellationToken);
+    Task StartRequestAsync(TRequestContext? requestContext, CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Creates the context that is sent to the handler for this queue item.
+    /// Note - this method is always called serially inside the queue before
+    /// running the actual request in <see cref="StartRequestAsync(TRequestContext?, CancellationToken)"/>
+    /// Throwing in this method will cause the server to shutdown.
+    /// </summary>
+    Task<TRequestContext?> CreateRequestContextAsync(CancellationToken cancellationToken);
 
     ILspServices LspServices { get; }
 

src/Features/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/IRequestContextFactory.cs

@@ -15,6 +15,7 @@ public interface IRequestContextFactory<TRequestContext>
 {
     /// <summary>
     /// Create a <typeparamref name="TRequestContext"/> object from the given <see cref="IQueueItem{RequestContextType}"/>.
+    /// Note - throwing in the implementation of this method will cause the server to shutdown.
     /// </summary>
     /// <param name="queueItem">The <see cref="IQueueItem{RequestContextType}"/> from which to create a request.</param>
     /// <param name="requestParam">The request parameters.</param>

src/Features/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/QueueItem.cs

@@ -85,23 +85,28 @@ public static (IQueueItem<TRequestContext>, Task<TResponse>) Create(
         return (queueItem, queueItem._completionSource.Task);
     }
 
+    public async Task<TRequestContext?> CreateRequestContextAsync(CancellationToken cancellationToken)
+    {
+        cancellationToken.ThrowIfCancellationRequested();
+        var requestContextFactory = LspServices.GetRequiredService<IRequestContextFactory<TRequestContext>>();
+        var context = await requestContextFactory.CreateRequestContextAsync(this, _request, cancellationToken).ConfigureAwait(false);
+        return context;
+    }
+
     /// <summary>
     /// Processes the queued request. Exceptions will be sent to the task completion source
     /// representing the task that the client is waiting for, then re-thrown so that
     /// the queue can correctly handle them depending on the type of request.
     /// </summary>
     /// <param name="cancellationToken"></param>
     /// <returns>The result of the request.</returns>
-    public async Task StartRequestAsync(CancellationToken cancellationToken)
+    public async Task StartRequestAsync(TRequestContext? context, CancellationToken cancellationToken)
     {
         _logger.LogStartContext($"{MethodName}");
         try
         {
             cancellationToken.ThrowIfCancellationRequested();
 
-            var requestContextFactory = LspServices.GetRequiredService<IRequestContextFactory<TRequestContext>>();
-            var context = await requestContextFactory.CreateRequestContextAsync(this, _request, cancellationToken).ConfigureAwait(false);
-
             if (context is null)
             {
                 // If we weren't able to get a corresponding context for this request (for example, we

src/Features/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/RequestExecutionQueue.cs

@@ -188,11 +188,14 @@ private async Task ProcessQueueAsync()
 
                     var cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(CancellationToken, cancellationToken);
 
+                    // The request context must be created serially inside the queue to so that requests always run
+                    // on the correct snapshot as of the last request.
+                    var context = await work.CreateRequestContextAsync(cancellationToken).ConfigureAwait(false);
                     if (work.MutatesServerState)
                     {
                         // Mutating requests block other requests from starting to ensure an up to date snapshot is used.
                         // Since we're explicitly awaiting exceptions to mutating requests will bubble up here.
-                        await work.StartRequestAsync(cancellationToken).ConfigureAwait(false);
+                        await work.StartRequestAsync(context, cancellationToken).ConfigureAwait(false);
                     }
                     else
                     {
@@ -201,7 +204,7 @@ private async Task ProcessQueueAsync()
                         // via NFW, though these errors don't put us into a bad state as far as the rest of the queue goes.
                         // Furthermore we use Task.Run here to protect ourselves against synchronous execution of work
                         // blocking the request queue for longer periods of time (it enforces parallelizabilty).
-                        _ = Task.Run(() => work.StartRequestAsync(cancellationToken), cancellationToken);
+                        _ = Task.Run(() => work.StartRequestAsync(context, cancellationToken), cancellationToken);
                     }
                 }
                 catch (OperationCanceledException ex) when (ex.CancellationToken == queueItem.cancellationToken)

src/Features/LanguageServer/ProtocolUnitTests/Ordering/RequestOrderingTests.cs

@@ -7,7 +7,6 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using Roslyn.Test.Utilities;