Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RoslynServiceExtensions should be removed in lieu of IAsyncServiceProvider3 #73636

Open
davkean opened this issue May 22, 2024 · 1 comment
Open

Comments

@davkean
Copy link
Member

davkean commented May 22, 2024

To reduce the number of unnecessarily thread switches, we have introduced a new way to asynchronous retrieve services; IAsyncServiceProvider3.

    /// <summary>
    /// Provides a mechanism for retrieving proffered services asynchronously. This is the async equivalent of <see cref="IServiceProvider"/>
    /// and should be used in asynchronous contexts to avoid blocking calling threads.
    /// </summary>
    /// <remarks>
    /// <para>This interface is safe to access from any thread.</para>
    /// <para>Prefer this interface and <see cref="t:Microsoft.VisualStudio.Shell.ServiceExtensions" /> over using <see cref="IAsyncServiceProvider"/> and <see cref="IAsyncServiceProvider2"/> directly
    /// to avoid unexpected blocking RPC calls when casting the resulting service to a specific interface. The former also provide more consistent throwing behavior.</para>
    /// </remarks>
    public interface IAsyncServiceProvider3 : IAsyncServiceProvider2
    {
        /// <summary>
        /// Retrieves a proffered service asynchronously, specifying whether to throw an exception if it could not be retrieved.
        /// </summary>
        /// <typeparam name="TService">The service identity of the proffered service to retrieve.</typeparam>
        /// <typeparam name="TInterface">The interface used to interact with the proffered service. If <typeparamref name="TService"/> is not registered as async or free-threaded, then this cast will be performed on the main thread.</typeparam>
        /// <param name="throwOnFailure"><see langword="true"/> to throw an exception if the service could not be retrieved; otherwise, <see langword="false"/> to return <see langword="null"/> on failure.</param>
        /// <param name="cancellationToken">A token whose cancellation indicates that the caller no longer is interested in the result. This will not cancel the in-progress loading of packages and/or creation of services as a result of the service retrieval, but this token will result in an expediant cancellation of the returned Task.</param>
        /// <returns>A task representing the service retrieval whose result is the service or <see langword="null"/> if there was a failure and <paramref name="throwOnFailure"/> is <see langword="false"/>.</returns>
        /// <exception cref="OperationCanceledException">
        /// <paramref name="cancellationToken"/> has been canceled.
        /// </exception>
        /// <exception cref="ServiceUnavailableException">
        /// <paramref name="throwOnFailure"/> is <see langword="true"/> and there was a failure retrieving the service due to one of the following conditions:
        /// <list type="bullet">
        ///     <item>
        ///         <description>The service threw an exception during activation. <see cref="Exception.InnerException"/> may include details about the underlying failure.</description>
        ///     </item>
        ///     <item>
        ///         <description>The associated package failed to load. <see cref="Exception.InnerException"/> may include details about the underlying failure.</description>
        ///     </item>
        ///     <item>
        ///         <description>The associated package could not be found, or the package did not correctly proffer the service.</description>
        ///     </item>
        ///     <item>
        ///         <description>The associated package proffered <see langword="null"/>.</description>
        ///     </item>
        ///     <item>
        ///         <description>The service does not support the requested interface specified by <typeparamref name="TInterface"/>.</description>
        ///     </item>
        ///     <item>
        ///         <description>The environment has starting shutting down and the retrieval would have resulted in a package load.</description>
        ///     </item>
        /// </list>
        /// </exception>
        /// <remarks>
        /// <para>Prefer this method over an explicit cast to <typeparamref name="TInterface"/> to avoid a blocking RPC call if the underlying service is STA-bound,
        /// which can hang if the UI thread is blocked in a <see cref="JoinableTaskFactory.Run(Func{Task})"/> or <see cref="JoinableTask.Join"/>.</para>
        /// <para>Note the difference in behavior this method has around exceptions to its non-generic equivalents <see cref="IAsyncServiceProvider.GetServiceAsync"/> and <see cref="IAsyncServiceProvider2.GetServiceAsync"/>.</para>
        /// <para>This method is safe to access from any thread.</para>
        /// </remarks>
        Task<TInterface?> GetServiceAsync<TService, TInterface>(bool throwOnFailure, CancellationToken cancellationToken) where TInterface : class;

This interface takes the throwing policy from ServiceExtensions.GetServiceAsync extensions methods, and brings into the global service provider and AsyncPackage. Unlike the extensions however, it only introduces a thread switch when absolutely required. Many operations are now free-threaded, and let's consumes cancel their observation of a package load and/or service creation.

var appId = await GetServiceAsync<SVsAppId, IVsAppId>(); // Free-threaded and synchronous call
var service = await GetServiceAsync<SManagedService, ManagedService>();  // Free-threaded and synchronous call once service has been created.

var service = await GetServiecAsync<SSynchService, SyncService>(this.DisposalToken); // Not free-threaded (we will go to UI thread to grab SyncService), but we can now load `SyncService` package asynchronously, and let the user cancel its observation of it.

The intention of this interface is not to replace existing usage of IAsyncServiceProvider, in fact, almost all code inside Visual Studio immediately benefits from the very fact that all core APIs (ServiceExtensions, IVsService<TService, TInterface>, AsyncPackage.GetServiceAsync, SAsyncServiceProvider, now recognize and handle IAsyncServiceProvider3 without users changing code.

That is, except for Roslyn. Roslyn provides their own versions of the extensions so that they can plug in their own JoinableTaskFactory into the extensions, which was needed because they could not override ThreadHelper.JoinableTaskFactory.

This is now unneeded, simple implement IAsyncServiceProvider3 in your tests and you can control which JoinableTaskFactory to use to thread switch. ServiceExtensions will delegate to it if the IAsyncServiceProvider passed to is a IAsyncServiceProvider3.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 22, 2024
@CyrusNajmabadi CyrusNajmabadi added Concept-Continuous Improvement and removed untriaged Issues and PRs which have not yet been triaged by a lead labels May 22, 2024
@CyrusNajmabadi CyrusNajmabadi added this to the 17.11 milestone May 22, 2024
@CyrusNajmabadi
Copy link
Member

@sharwell @ToddGrun about this. Dave/platform wouldl def love to see some movement on this soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants