Skip to content

Commit

Permalink
Merge pull request dotnet#74300 from dibarbet/fix_lsp_language_2
Browse files Browse the repository at this point in the history 
Move determination of language for an LSP request into the serialized queue to avoid race
  • Loading branch information
@dibarbet
dibarbet authored Jul 10, 2024
2 parents 6d1db01 + 235d31b commit 7e28266
Show file tree
Hide file tree
Showing 20 changed files with 532 additions and 367 deletions.
76 changes: 40 additions & 36 deletions ...er/Microsoft.CommonLanguageServerProtocol.Framework.UnitTests/Mocks/TestMethodHandlers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,47 +9,50 @@

namespace Microsoft.CommonLanguageServerProtocol.Framework.UnitTests;

internal record class MockRequest(int Param);
internal record class MockResponse(string Response);

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class TestMethodHandler : IRequestHandler<int, string, TestRequestContext>
internal class TestMethodHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "Method";
public static readonly IMethodHandler Instance = new TestMethodHandler();

public bool MutatesSolutionState => true;
public static TypeRef RequestType = TypeRef.Of<int>();
public static TypeRef ResponseType = TypeRef.Of<string>();
public static TypeRef RequestType = TypeRef.Of<MockRequest>();
public static TypeRef ResponseType = TypeRef.Of<MockResponse>();
public static RequestHandlerMetadata Metadata = new(Name, RequestType, ResponseType, LanguageServerConstants.DefaultLanguageName);

public Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult("stuff");
public Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult<MockResponse>(new("stuff"));
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class TestParameterlessMethodHandler : IRequestHandler<bool, TestRequestContext>
internal class TestParameterlessMethodHandler : IRequestHandler<MockResponse, TestRequestContext>
{
public const string Name = "ParameterlessMethod";
public static readonly IMethodHandler Instance = new TestParameterlessMethodHandler();

public bool MutatesSolutionState => true;

public static TypeRef ResponseTypeRef = TypeRef.Of<bool>();
public static TypeRef ResponseTypeRef = TypeRef.Of<MockResponse>();
public static RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: null, ResponseTypeRef, LanguageServerConstants.DefaultLanguageName);

public Task<bool> HandleRequestAsync(TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(true);
public Task<MockResponse> HandleRequestAsync(TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(new MockResponse("true"));
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class TestNotificationHandler : INotificationHandler<bool, TestRequestContext>
internal class TestNotificationHandler : INotificationHandler<MockRequest, TestRequestContext>
{
public const string Name = "Notification";
public static readonly IMethodHandler Instance = new TestNotificationHandler();

public bool MutatesSolutionState => true;
public static TypeRef RequestTypeRef = TypeRef.Of<bool>();
public static TypeRef RequestTypeRef = TypeRef.Of<MockRequest>();
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef, ResponseTypeRef: null, LanguageServerConstants.DefaultLanguageName);

public Task HandleNotificationAsync(bool request, TestRequestContext context, CancellationToken cancellationToken)
public Task HandleNotificationAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(true);
}

Expand Down Expand Up @@ -77,56 +80,57 @@ public Task HandleNotificationAsync(TestRequestContext requestContext, Cancellat
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class MutatingHandler : IRequestHandler<int, string, TestRequestContext>
internal class MutatingHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "MutatingMethod";
public static readonly IMethodHandler Instance = new MutatingHandler();
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<int>(), ResponseTypeRef: TypeRef.Of<string>(), LanguageServerConstants.DefaultLanguageName);

public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<MockRequest>(), ResponseTypeRef: TypeRef.Of<MockResponse>(), LanguageServerConstants.DefaultLanguageName);

public MutatingHandler()
{
}

public bool MutatesSolutionState => true;

public Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
public Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
{
return Task.FromResult(string.Empty);
return Task.FromResult(new MockResponse(string.Empty));
}
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class CompletingHandler : IRequestHandler<int, string, TestRequestContext>
internal class CompletingHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "CompletingMethod";
public static readonly IMethodHandler Instance = new CompletingHandler();
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<int>(), ResponseTypeRef: TypeRef.Of<string>(), LanguageServerConstants.DefaultLanguageName);
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<MockRequest>(), ResponseTypeRef: TypeRef.Of<MockResponse>(), LanguageServerConstants.DefaultLanguageName);

public bool MutatesSolutionState => false;

public async Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
public async Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
{
while (true)
{
if (cancellationToken.IsCancellationRequested)
{
return "I completed!";
return new("I completed!");
}
await Task.Delay(100, cancellationToken).NoThrowAwaitable();
}
}
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class CancellingHandler : IRequestHandler<int, string, TestRequestContext>
internal class CancellingHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "CancellingMethod";
public static readonly IMethodHandler Instance = new CancellingHandler();
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<int>(), ResponseTypeRef: TypeRef.Of<string>(), LanguageServerConstants.DefaultLanguageName);
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<MockRequest>(), ResponseTypeRef: TypeRef.Of<MockResponse>(), LanguageServerConstants.DefaultLanguageName);

public bool MutatesSolutionState => false;

public async Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
public async Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
{
while (true)
{
Expand All @@ -137,48 +141,48 @@ public async Task<string> HandleRequestAsync(int request, TestRequestContext con
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class ThrowingHandler : IRequestHandler<int, string, TestRequestContext>
internal class ThrowingHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "ThrowingMethod";
public static readonly IMethodHandler Instance = new ThrowingHandler();
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<int>(), ResponseTypeRef: TypeRef.Of<string>(), LanguageServerConstants.DefaultLanguageName);
public static readonly RequestHandlerMetadata Metadata = new(Name, RequestTypeRef: TypeRef.Of<MockRequest>(), ResponseTypeRef: TypeRef.Of<MockResponse>(), LanguageServerConstants.DefaultLanguageName);

public bool MutatesSolutionState => false;

public Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
public Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
{
throw new NotImplementedException();
}
}

[LanguageServerEndpoint(Name, LanguageServerConstants.DefaultLanguageName)]
internal class TestDefaultLanguageHandler : IRequestHandler<int, string, TestRequestContext>
internal class TestDefaultLanguageHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = "Language";
public static readonly IMethodHandler Instance = new TestDefaultLanguageHandler();

public bool MutatesSolutionState => true;
public static TypeRef RequestType = TypeRef.Of<int>();
public static TypeRef ResponseType = TypeRef.Of<string>();
public static TypeRef RequestType = TypeRef.Of<MockRequest>();
public static TypeRef ResponseType = TypeRef.Of<MockResponse>();
public static RequestHandlerMetadata Metadata = new(Name, RequestType, ResponseType, LanguageServerConstants.DefaultLanguageName);

public Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(string.Empty);
public Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(new MockResponse(string.Empty));
}

[LanguageServerEndpoint(Name, Language)]
internal class TestXamlLanguageHandler : IRequestHandler<int, string, TestRequestContext>
internal class TestXamlLanguageHandler : IRequestHandler<MockRequest, MockResponse, TestRequestContext>
{
public const string Name = TestDefaultLanguageHandler.Name;
public const string Language = "xaml";
public static readonly IMethodHandler Instance = new TestXamlLanguageHandler();

public bool MutatesSolutionState => true;
public static TypeRef RequestType = TypeRef.Of<int>();
public static TypeRef ResponseType = TypeRef.Of<string>();
public static TypeRef RequestType = TypeRef.Of<MockRequest>();
public static TypeRef ResponseType = TypeRef.Of<MockResponse>();
public static RequestHandlerMetadata Metadata = new(Name, RequestType, ResponseType, Language);

public Task<string> HandleRequestAsync(int request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult("xaml");
public Task<MockResponse> HandleRequestAsync(MockRequest request, TestRequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(new MockResponse("xaml"));
}

27 changes: 14 additions & 13 deletions .../Microsoft.CommonLanguageServerProtocol.Framework.UnitTests/RequestExecutionQueueTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using Nerdbank.Streams;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using StreamJsonRpc;
using Xunit;

Expand Down Expand Up @@ -51,7 +52,7 @@ public async Task ExecuteAsync_ThrowCompletes()
var lspServices = GetLspServices();

// Act & Assert
await Assert.ThrowsAsync<NotImplementedException>(() => requestExecutionQueue.ExecuteAsync<int, string>(1, ThrowingHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None));
await Assert.ThrowsAsync<NotImplementedException>(() => requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), ThrowingHandler.Name, lspServices, CancellationToken.None));
}

[Fact]
Expand All @@ -72,12 +73,12 @@ public async Task ExecuteAsync_WithCancelInProgressWork_CancelsInProgressWorkWhe
var cancellingRequestCancellationToken = new CancellationToken();
var completingRequestCancellationToken = new CancellationToken();

var _ = requestExecutionQueue.ExecuteAsync<int, string>(1, CancellingHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, cancellingRequestCancellationToken);
var _1 = requestExecutionQueue.ExecuteAsync<int, string>(1, CompletingHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, completingRequestCancellationToken);
var _ = requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), CancellingHandler.Name, lspServices, cancellingRequestCancellationToken);
var _1 = requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), CompletingHandler.Name, lspServices, completingRequestCancellationToken);

// Act & Assert
// A Debug.Assert would throw if the tasks hadn't completed when the mutating request is called.
await requestExecutionQueue.ExecuteAsync<int, string>(1, MutatingHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
await requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), MutatingHandler.Name, lspServices, CancellationToken.None);
}
}

Expand All @@ -100,8 +101,8 @@ public async Task ExecuteAsync_CompletesTask()
var requestExecutionQueue = GetRequestExecutionQueue(false, (TestMethodHandler.Metadata, TestMethodHandler.Instance));
var lspServices = GetLspServices();

var response = await requestExecutionQueue.ExecuteAsync<int, string>(request: 1, TestMethodHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
Assert.Equal("stuff", response);
var response = (MockResponse?)await requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), TestMethodHandler.Name, lspServices, CancellationToken.None);
Assert.Equal("stuff", response?.Response);
}

[Fact]
Expand All @@ -110,8 +111,8 @@ public async Task ExecuteAsync_CompletesTask_Parameterless()
var requestExecutionQueue = GetRequestExecutionQueue(false, (TestParameterlessMethodHandler.Metadata, TestParameterlessMethodHandler.Instance));
var lspServices = GetLspServices();

var response = await requestExecutionQueue.ExecuteAsync<NoValue, bool>(request: NoValue.Instance, TestParameterlessMethodHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
Assert.True(response);
var response = (MockResponse?)await requestExecutionQueue.ExecuteAsync(serializedRequest: null, TestParameterlessMethodHandler.Name, lspServices, CancellationToken.None);
Assert.Equal("true", response?.Response);
}

[Fact]
Expand All @@ -120,7 +121,7 @@ public async Task ExecuteAsync_CompletesTask_Notification()
var requestExecutionQueue = GetRequestExecutionQueue(false, (TestNotificationHandler.Metadata, TestNotificationHandler.Instance));
var lspServices = GetLspServices();

var response = await requestExecutionQueue.ExecuteAsync<bool, NoValue>(request: true, TestNotificationHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
var response = await requestExecutionQueue.ExecuteAsync(JToken.FromObject(new MockRequest(1)), TestNotificationHandler.Name, lspServices, CancellationToken.None);
Assert.Same(NoValue.Instance, response);
}

Expand All @@ -130,19 +131,19 @@ public async Task ExecuteAsync_CompletesTask_Notification_Parameterless()
var requestExecutionQueue = GetRequestExecutionQueue(false, (TestParameterlessNotificationHandler.Metadata, TestParameterlessNotificationHandler.Instance));
var lspServices = GetLspServices();

var response = await requestExecutionQueue.ExecuteAsync<NoValue, NoValue>(request: NoValue.Instance, TestParameterlessNotificationHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
var response = await requestExecutionQueue.ExecuteAsync(serializedRequest: null, TestParameterlessNotificationHandler.Name, lspServices, CancellationToken.None);
Assert.Same(NoValue.Instance, response);
}

[Fact]
public async Task Queue_DrainsOnShutdown()
{
var requestExecutionQueue = GetRequestExecutionQueue(false, (TestMethodHandler.Metadata, TestMethodHandler.Instance));
var request = 1;
var request = JToken.FromObject(new MockRequest(1));
var lspServices = GetLspServices();

var task1 = requestExecutionQueue.ExecuteAsync<int, string>(request, TestMethodHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
var task2 = requestExecutionQueue.ExecuteAsync<int, string>(request, TestMethodHandler.Name, LanguageServerConstants.DefaultLanguageName, lspServices, CancellationToken.None);
var task1 = requestExecutionQueue.ExecuteAsync(request, TestMethodHandler.Name, lspServices, CancellationToken.None);
var task2 = requestExecutionQueue.ExecuteAsync(request, TestMethodHandler.Name, lspServices, CancellationToken.None);

await requestExecutionQueue.DisposeAsync();

Expand Down
4 changes: 2 additions & 2 deletions ...anguageServer/Microsoft.CommonLanguageServerProtocol.Framework/AbstractHandlerProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ internal abstract class AbstractHandlerProvider
/// <param name="language">The language for the request.</param>
/// <returns>The handler for the request.</returns>
/// <remarks>
/// If the handler for the given language is not found, the default handler is returned.
/// If the default handler is not found, an exception is thrown.
/// If the handler for the given language is not found, an exception is thrown.
/// Callers are expected to only request handlers for methods and languages that exist.
/// </remarks>
public abstract IMethodHandler GetMethodHandler(string method, TypeRef? requestTypeRef, TypeRef? responseTypeRef, string language);
}
Loading

0 comments on commit 7e28266

Please sign in to comment.