Merge pull request #47951 from sharwell/dispose-language-services

Dispose per-instance services created by factories

Author: sharwell

src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt

@@ -1 +1,2 @@
-
+virtual Microsoft.CodeAnalysis.Host.HostLanguageServices.Dispose() -> void
+virtual Microsoft.CodeAnalysis.Host.HostWorkspaceServices.Dispose() -> void

src/Workspaces/Core/Portable/Workspace/Host/HostLanguageServices.cs

@@ -3,13 +3,23 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Host.Mef;
 
 namespace Microsoft.CodeAnalysis.Host;
 
 /// <summary>
-/// Per language services provided by the host environment.
+/// Per-language services provided by the host environment.
 /// </summary>
-public abstract class HostLanguageServices
+/// <remarks>
+/// <para>Language services which implement <see cref="IDisposable"/> are considered ownable, in which case the
+/// owner is responsible for disposing of owned instances when they are no longer in use. The ownership rules are
+/// described in detail for <see cref="HostWorkspaceServices"/>. Instances of <see cref="ILanguageService"/> have
+/// the same ownership rules as <see cref="IWorkspaceService"/>, and instances of
+/// <see cref="ILanguageServiceFactory"/> have the same ownership rules as
+/// <see cref="IWorkspaceServiceFactory"/>.</para>
+/// </remarks>
+public abstract class HostLanguageServices : IDisposable
 {
     /// <summary>
     /// The <see cref="HostWorkspaceServices"/> that originated this language service.
@@ -40,6 +50,11 @@ protected HostLanguageServices()
     /// </summary>
     public abstract TLanguageService? GetService<TLanguageService>() where TLanguageService : ILanguageService;
 
+    [SuppressMessage("Usage", "CA1816:Dispose methods should call SuppressFinalize", Justification = "Derived types are not allowed to include a finalizer for this pattern.")]
+    public virtual void Dispose()
+    {
+    }
+
     /// <summary>
     /// Gets a language specific service provided by the host identified by the service type. 
     /// If the host does not provide the service, this method returns throws <see cref="InvalidOperationException"/>.

src/Workspaces/Core/Portable/Workspace/Host/HostWorkspaceServices.cs

@@ -5,14 +5,44 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.Host;
 
 /// <summary>
 /// Per workspace services provided by the host environment.
 /// </summary>
-public abstract class HostWorkspaceServices
+/// <remarks>
+/// <para>Workspace services which implement <see cref="IDisposable"/> are considered ownable, in which case the
+/// owner is responsible for disposing of owned instances when they are no longer in use. When
+/// <see cref="IWorkspaceService"/> or <see cref="IWorkspaceServiceFactory"/> instances are provided directly to the
+/// <see cref="HostWorkspaceServices"/>, the owner of the instances is the type or container (e.g. a MEF export
+/// provider) which created the instances. For the specific case of ownable workspace services created by a factory
+/// (i.e. instances returned by <see cref="IWorkspaceServiceFactory.CreateService"/>), the <see cref="Workspace"/>
+/// is considered the owner of the resulting instance and is expected to be disposed during the call to
+/// <see cref="Dispose"/>.</para>
+///
+/// <para><strong>Summary of lifetime rules</strong></para>
+///
+/// <list type="bullet">
+/// <item><description>
+///   <strong><see cref="IWorkspaceService"/> instance constructed externally (e.g. MEF):</strong> Owned by the
+///   external source, and will not be automatically disposed when <see cref="Workspace"/> is disposed.
+/// </description></item>
+/// <item><description>
+///   <strong><see cref="IWorkspaceServiceFactory"/> instance constructed externally (e.g. MEF):</strong> Owned by
+///   the external source, and will not be automatically disposed when <see cref="Workspace"/> is disposed.
+/// </description></item>
+/// <item><description>
+///   <strong><see cref="IWorkspaceService"/> instance constructed by <see cref="IWorkspaceServiceFactory"/> within
+///   the context of <see cref="HostWorkspaceServices"/>:</strong> Owned by <see cref="Workspace"/>, and
+///   <strong>will</strong> be automatically disposed when <see cref="Workspace"/> is disposed.
+/// </description></item>
+/// </list>
+/// </remarks>
+public abstract class HostWorkspaceServices : IDisposable
 {
     /// <summary>
     /// The host services this workspace services originated from.
@@ -40,6 +70,11 @@ protected HostWorkspaceServices()
 #pragma warning restore
     }
 
+    [SuppressMessage("Usage", "CA1816:Dispose methods should call SuppressFinalize", Justification = "Derived types are not allowed to include a finalizer for this pattern.")]
+    public virtual void Dispose()
+    {
+    }
+
     /// <summary>
     /// Gets a workspace specific service provided by the host identified by the service type. 
     /// If the host does not provide the service, this method throws <see cref="InvalidOperationException"/>.

src/Workspaces/Core/Portable/Workspace/Workspace.cs

@@ -17,9 +17,9 @@
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.PooledObjects;
-using Microsoft.CodeAnalysis.Remote;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
+using Microsoft.CodeAnalysis.Shared.Utilities;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -692,13 +692,9 @@ protected virtual void Dispose(bool finalize)
 
         _legacyOptions.UnregisterWorkspace(this);
 
-        // Directly dispose IRemoteHostClientProvider if necessary. This is a test hook to ensure RemoteWorkspace
-        // gets disposed in unit tests as soon as TestWorkspace gets disposed. This would be superseded by direct
-        // support for IDisposable in https://github.com/dotnet/roslyn/pull/47951.
-        if (Services.GetService<IRemoteHostClientProvider>() is IDisposable disposableService)
-        {
-            disposableService.Dispose();
-        }
+        // Dispose per-instance services created for this workspace (direct MEF exports, including factories, will
+        // be disposed when the MEF catalog is disposed).
+        Services.Dispose();
 
         // We're disposing this workspace.  Stop any work to update SG docs in the background.
         _updateSourceGeneratorsQueueTokenSource.Cancel();

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensionsResources.resx

@@ -273,4 +273,7 @@
   <data name="New_line_preferences" xml:space="preserve">
     <value>New line preferences</value>
   </data>
+  <data name="Instantiated_parts_threw_exceptions_from_IDisposable_Dispose" xml:space="preserve">
+    <value>Instantiated part(s) threw exception(s) from IDisposable.Dispose().</value>
+  </data>
 </root>