dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.06k stars 4.04k forks source link

Improve design of partner component hosting in Roslyn ServiceHub process #44326

Closed tmat closed 3 years ago

tmat commented 4 years ago

Currently we allow select partners to run their services in Roslyn's ServiceHub process: Live Unit Testing, Source Based Test Discovery, Razor and IntelliCode. In future we'll need to host XAML LSP.

These components are already required to use External Access layer to access RemoteHostClient (client) and ServiceBase (server) APIs.

Proposal

We will keep using RemoteHostClient on the client but stop using ServiceBase as a base type for service implementation.

Roslyn will expose ExternalAccess API for accessing Solution snapshot in ServiceHub process. For example, for Razor:

namespace Microsoft.CodeAnalysis.ExternalAccess.Razor.Api
{
    internal static class RazorRemoteUtilities
    {
        // Custom Roslyn formatters required for serialization of some Roslyn types such as DocumentId, ProjectId, etc.
        public static ImmutableArray<IMessagePackFormatter> Formatters
            => MessagePackFormatters.Formatters;

        public static ValueTask<Solution> GetSolutionAsync(ServiceBrokerClient serviceBroker, object solutionInfo, CancellationToken cancellationToken)
            => RemoteWorkspaceSolutionProvider.GetSolutionAsync(serviceBroker, solutionInfo, cancellationToken);
    }
}

The partner team will implement a ServiceHub service in their own repo using IServiceHubServiceFactory pattern. They will need a single Microsoft.VisualStudio.{component-name}.{service-name}.servicehub.service.json file that describes their service (the name of the service must start with Microsoft.VisualStudio. in order for the service to be considered first-party service):

{
  "host": "desktopClr",
  "hostId": "RoslynCodeAnalysisService",
  "hostGroupAllowed": true,
  "entryPoint": {
    "assemblyPath": "My.dll",
    "fullClassName": "MyServiceFactory",
    "appBasePath": "%VSAPPIDDIR%",
    "configPath": "%PkgDefApplicationConfigFile%"
  }
}
public class MyServiceFactory : IServiceHubServiceFactory
{
    public Task<object> CreateAsync(
        Stream stream,
        IServiceProvider hostProvidedServices,
        ServiceActivationOptions serviceActivationOptions,
        IServiceBroker serviceBroker,
        AuthorizationServiceClient authorizationServiceClient)
    {
        authorizationServiceClient.Dispose();

        // create new service instance
        return Task.FromResult(new MyRemoteService(serviceBroker));
    }
}

The service should create and hold onto ServiceBrokerClient and pass it to *RemoteUtilities when retrieving Solution from object solutionInfo.

The service also needs to be registered by adding an attribute on VS Package type:

[ProvideBrokeredService("Microsoft.VisualStudio.{component-name}.{service-name}", Audience = Shell.ServiceBroker.ServiceAudience.Local)]

See https://github.com/dotnet/aspnetcore-tooling/pull/2500 for example.

Roslyn implementation of External Access layer for Unit Testing: https://github.com/dotnet/roslyn/pull/48776

tmat commented 4 years ago

@shyamnamboodiripad @Cosifne @NTaylorMullen @mgoertz-msft @CyrusNajmabadi

shyamnamboodiripad commented 4 years ago

@tmat it may be nice to also call out how callbacks will work and how the new client side interaction with RemoteHostClient (and any proposed changes for Session objects) looks like...

mgoertz-msft commented 4 years ago

@tmat Looks great, so in theory if we were to make ISolutionProvider and PinnedSolutionInfo public the server would no longer need any IVT access to Roslyn for anything. Are there any parts of the remoting infrastructure in ServiceBase/EndPoint worth sharing though?

tmat commented 4 years ago

Looks great, so in theory if we were to make ISolutionProvider and PinnedSolutionInfo public the server would no longer need any IVT access to Roslyn for anything.

Yes, but the service might potentially also need other internal Roslyn APIs depending on what the service is doing.

Are there any parts of the remoting infrastructure in ServiceBase/EndPoint worth sharing though?

Not on the server side. The client will still need to use Roslyn's internal RemoteHostClient class to invoke remote services that require the solution snapshot. A major problem right now is that the remote solution snapshot is mirrored from devenv. So, any remote service that needs to access a solution snapshot must make sure that the snapshot is present in devenv process while invoking GetSolutionAsync in the remote process. This is currently handled by RemoteHostClient that is only available in devenv process.

For example, our LSP server must currently be running in devenv process. The handlers of LSP requests use RemoteHostClient to invoke remote service, passing it the current solution that exists in the VisualStudioWorkspace devenv process and holding on the snapshot until the remote invocation is completed.

tmat commented 4 years ago

Updated to reflect the latest design that is currently being implemented.