diff --git a/src/Features/Core/Portable/Extensions/ExtensionMessageHandlerService.cs b/src/Features/Core/Portable/Extensions/ExtensionMessageHandlerService.cs index 0d0882e73b60e..f13a988e017e1 100644 --- a/src/Features/Core/Portable/Extensions/ExtensionMessageHandlerService.cs +++ b/src/Features/Core/Portable/Extensions/ExtensionMessageHandlerService.cs @@ -11,12 +11,14 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Threading; using Roslyn.Utilities; @@ -40,26 +42,29 @@ internal sealed class ExtensionMessageHandlerService( IExtensionMessageHandlerFactory customMessageHandlerFactory) : IExtensionMessageHandlerService { + private static readonly ConditionalWeakTable s_disabledExtensionHandlers = new(); + private readonly SolutionServices _solutionServices = solutionServices; private readonly IExtensionMessageHandlerFactory _customMessageHandlerFactory = customMessageHandlerFactory; + // Core design: To make things lightweight, and to avoid locking, all work is computed and cached in simple + // immutable dictionaries. These dictionaries are populated on demand, but contain data that can be recomputed + // safely if missing. + /// - /// Extensions assembly load contexts and loaded handlers, indexed by handler file path. The handlers are indexed by type name. + /// Extensions assembly load contexts and loaded handlers, indexed by extension folder path. /// - private readonly Dictionary _extensions = new(); + private ImmutableDictionary> _folderPathToExtensionFolder = ImmutableDictionary>.Empty; /// - /// Handlers of document-related messages, indexed by handler message name. + /// Cached handlers of document-related messages, indexed by handler message name. /// - private readonly Dictionary> _documentHandlers = new(); + private ImmutableDictionary>>> _cachedDocumentHandlers = ImmutableDictionary>>>.Empty; /// - /// Handlers of non-document-related messages, indexed by handler message name. + /// Cached handlers of non-document-related messages, indexed by handler message name. /// - private readonly Dictionary> _workspaceHandlers = new(); - - // Used to protect access to _extensions, _handlers, _documentHandlers and Extension._assemblies. - private readonly SemaphoreSlim _lock = new(initialCount: 1); + private ImmutableDictionary>>> _cachedWorkspaceHandlers = ImmutableDictionary>>>.Empty; private async ValueTask ExecuteInRemoteOrCurrentProcessAsync( Solution? solution, @@ -74,7 +79,7 @@ private async ValueTask ExecuteInRemoteOrCurrentProcessAsync( if (solution is null) { var result = await client.TryInvokeAsync( - (service, cancellationToken) => executeOutOfProcessAsync(service, null, cancellationToken), + (remoteService, cancellationToken) => executeOutOfProcessAsync(remoteService, null, cancellationToken), cancellationToken).ConfigureAwait(false); // If the remote call succeeded, this will have a valid valid in it and can be returned. If it did not @@ -86,7 +91,7 @@ private async ValueTask ExecuteInRemoteOrCurrentProcessAsync( { var result = await client.TryInvokeAsync( solution, - (service, checksum, cancellationToken) => executeOutOfProcessAsync(service, checksum, cancellationToken), + (remoteService, checksum, cancellationToken) => executeOutOfProcessAsync(remoteService, checksum, cancellationToken), cancellationToken).ConfigureAwait(false); return result.Value; } @@ -99,7 +104,7 @@ public async ValueTask RegisterExtensionAsync( return await ExecuteInRemoteOrCurrentProcessAsync( solution: null, cancellationToken => RegisterExtensionInCurrentProcessAsync(assemblyFilePath, cancellationToken), - (service, _, cancellationToken) => service.RegisterExtensionAsync(assemblyFilePath, cancellationToken), + (remoteService, _, cancellationToken) => remoteService.RegisterExtensionAsync(assemblyFilePath, cancellationToken), cancellationToken).ConfigureAwait(false); } @@ -107,69 +112,26 @@ public async ValueTask RegisterExtensionInCurrentProc string assemblyFilePath, CancellationToken cancellationToken) { - var assemblyFileName = Path.GetFileName(assemblyFilePath); + // var assemblyFileName = Path.GetFileName(assemblyFilePath); var assemblyFolderPath = Path.GetDirectoryName(assemblyFilePath) ?? throw new InvalidOperationException($"Unable to get the directory name for {assemblyFilePath}."); - var analyzerAssemblyLoaderProvider = _solutionServices.GetRequiredService(); - Extension? extension; - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) - { - // Check if the assembly is already loaded. - if (!_extensions.TryGetValue(assemblyFolderPath, out extension)) - { - try - { - var analyzerAssemblyLoader = analyzerAssemblyLoaderProvider.CreateNewShadowCopyLoader(); - - // Allow this assembly loader to load any dll in assemblyFolderPath. - foreach (var dll in Directory.EnumerateFiles(assemblyFolderPath, "*.dll")) - { - try - { - // Check if the file is a valid .NET assembly. - AssemblyName.GetAssemblyName(dll); - } - catch - { - // The file is not a valid .NET assembly, skip it. - continue; - } - - analyzerAssemblyLoader.AddDependencyLocation(dll); - } - - extension = new Extension(this, analyzerAssemblyLoader, assemblyFolderPath); - } - catch - { - _extensions[assemblyFolderPath] = null; - throw; - } - - _extensions[assemblyFolderPath] = extension; - } - - if (extension is null) - { - throw new InvalidOperationException($"A previous attempt to load assemblies from {assemblyFolderPath} failed."); - } + var lazy = ImmutableInterlocked.GetOrAdd( + ref _folderPathToExtensionFolder, + assemblyFolderPath, + static (assemblyFolderPath, @this) => AsyncLazy.Create( + cancellationToken => ExtensionFolder.Create(@this, assemblyFolderPath, cancellationToken)), + this); - if (extension.TryGetAssemblyHandlers(assemblyFileName, out var assemblyHandlers)) - { - if (assemblyHandlers is null) - { - throw new InvalidOperationException($"A previous attempt to load {assemblyFilePath} failed."); - } + var extensionFolder = await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); + var assemblyHandlers = await extensionFolder.RegisterAssemblyAsync(assemblyFilePath, cancellationToken).ConfigureAwait(false); - return new( - [.. assemblyHandlers.WorkspaceMessageHandlers.Keys], - [.. assemblyHandlers.DocumentMessageHandlers.Keys]); - } - } + // After registering, clear out the cached handler names. They will be recomputed the next time we need them. + ClearCachedHandlers(); - // Intentionally call this outside of the lock. - return await extension.LoadAssemblyAsync(assemblyFileName, cancellationToken).ConfigureAwait(false); + return new( + [.. assemblyHandlers.WorkspaceMessageHandlers.Keys], + [.. assemblyHandlers.DocumentMessageHandlers.Keys]); } public async ValueTask UnregisterExtensionAsync( @@ -179,7 +141,7 @@ public async ValueTask UnregisterExtensionAsync( await ExecuteInRemoteOrCurrentProcessAsync( solution: null, cancellationToken => UnregisterExtensionInCurrentProcessAsync(assemblyFilePath, cancellationToken), - (service, _, cancellationToken) => service.UnregisterExtensionAsync(assemblyFilePath, cancellationToken), + (remoteService, _, cancellationToken) => remoteService.UnregisterExtensionAsync(assemblyFilePath, cancellationToken), cancellationToken).ConfigureAwait(false); } @@ -187,43 +149,20 @@ private async ValueTask UnregisterExtensionInCurrentProcessAsync( string assemblyFilePath, CancellationToken cancellationToken) { - var assemblyFileName = Path.GetFileName(assemblyFilePath); var assemblyFolderPath = Path.GetDirectoryName(assemblyFilePath) ?? throw new InvalidOperationException($"Unable to get the directory name for {assemblyFilePath}."); - Extension? extension = null; - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + if (_folderPathToExtensionFolder.TryGetValue(assemblyFolderPath, out var lazy)) { - if (_extensions.TryGetValue(assemblyFolderPath, out extension)) - { - // If loading assemblies from this folder failed earlier, don't do anything. - if (extension is null) - return default; - - if (extension.RemoveAssemblyHandlers(assemblyFileName, out var assemblyHandlers)) - { - if (assemblyHandlers is not null) - { - foreach (var workspaceHandler in assemblyHandlers.WorkspaceMessageHandlers.Keys) - { - _workspaceHandlers.Remove(workspaceHandler); - } - - foreach (var documentHandler in assemblyHandlers.DocumentMessageHandlers.Keys) - { - _documentHandlers.Remove(documentHandler); - } - } - } - - if (extension.AssemblyHandlersCount > 0) - return default; - - _extensions.Remove(assemblyFolderPath); - } + var extensionFolder = await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); + // Unregister this particular assembly file from teh assembly folder. If it was the last extension within + // this folder, we can remove the registration for the extension entirely. + if (await extensionFolder.UnregisterAssemblyAsync(assemblyFilePath, cancellationToken).ConfigureAwait(false)) + _folderPathToExtensionFolder = _folderPathToExtensionFolder.Remove(assemblyFolderPath); } - extension?.AnalyzerAssemblyLoader.Dispose(); + // After unregistering, clear out the cached handler names. They will be recomputed the next time we need them. + ClearCachedHandlers(); return default; } @@ -231,34 +170,31 @@ public async ValueTask ResetAsync(CancellationToken cancellationToken) { await ExecuteInRemoteOrCurrentProcessAsync( solution: null, - cancellationToken => ResetInCurrentProcessAsync(cancellationToken), - (service, _, cancellationToken) => service.ResetAsync(cancellationToken), + _ => ResetInCurrentProcessAsync(), + (remoteService, _, cancellationToken) => remoteService.ResetAsync(cancellationToken), cancellationToken).ConfigureAwait(false); } - private async ValueTask ResetInCurrentProcessAsync(CancellationToken cancellationToken) + private ValueTask ResetInCurrentProcessAsync() { - List extensions; - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) - { - extensions = [.. _extensions.Values]; - _extensions.Clear(); - _workspaceHandlers.Clear(); - _documentHandlers.Clear(); - } - - foreach (var extension in extensions) - extension.AnalyzerAssemblyLoader.Dispose(); - + _folderPathToExtensionFolder = ImmutableDictionary>.Empty; + ClearCachedHandlers(); return default; } + private void ClearCachedHandlers() + { + _cachedWorkspaceHandlers = ImmutableDictionary>>>.Empty; + _cachedDocumentHandlers = ImmutableDictionary>>>.Empty; + } + public async ValueTask HandleExtensionWorkspaceMessageAsync(Solution solution, string messageName, string jsonMessage, CancellationToken cancellationToken) { return await ExecuteInRemoteOrCurrentProcessAsync( solution, - cancellationToken => HandleExtensionWorkspaceMessageInCurrentProcessAsync(solution, messageName, jsonMessage, cancellationToken), - (service, checksum, cancellationToken) => service.HandleExtensionWorkspaceMessageAsync(checksum!.Value, messageName, jsonMessage, cancellationToken), + cancellationToken => HandleExtensionMessageInCurrentProcessAsync( + executeArgument: solution, isSolution: true, messageName, jsonMessage, _cachedWorkspaceHandlers, cancellationToken), + (remoteService, checksum, cancellationToken) => remoteService.HandleExtensionWorkspaceMessageAsync(checksum!.Value, messageName, jsonMessage, cancellationToken), cancellationToken).ConfigureAwait(false); } @@ -266,202 +202,241 @@ public async ValueTask HandleExtensionDocumentMessageAsync(Document docu { return await ExecuteInRemoteOrCurrentProcessAsync( document.Project.Solution, - cancellationToken => HandleExtensionDocumentMessageInCurrentProcessAsync(document, messageName, jsonMessage, cancellationToken), - (service, checksum, cancellationToken) => service.HandleExtensionDocumentMessageAsync(checksum!.Value, messageName, jsonMessage, document.Id, cancellationToken), + cancellationToken => HandleExtensionMessageInCurrentProcessAsync( + executeArgument: document, isSolution: false, messageName, jsonMessage, _cachedDocumentHandlers, cancellationToken), + (remoteService, checksum, cancellationToken) => remoteService.HandleExtensionDocumentMessageAsync(checksum!.Value, messageName, jsonMessage, document.Id, cancellationToken), cancellationToken).ConfigureAwait(false); } - private ValueTask HandleExtensionWorkspaceMessageInCurrentProcessAsync(Solution solution, string messageName, string jsonMessage, CancellationToken cancellationToken) - => HandleExtensionMessageAsync(solution, messageName, jsonMessage, _workspaceHandlers, cancellationToken); - - public ValueTask HandleExtensionDocumentMessageInCurrentProcessAsync(Document document, string messageName, string jsonMessage, CancellationToken cancellationToken) - => HandleExtensionMessageAsync(document, messageName, jsonMessage, _documentHandlers, cancellationToken); - - private async ValueTask HandleExtensionMessageAsync( - TArgument argument, - string messageName, - string jsonMessage, - Dictionary> handlers, + private async ValueTask HandleExtensionMessageInCurrentProcessAsync( + TArgument executeArgument, bool isSolution, string messageName, string jsonMessage, + ImmutableDictionary>>> cachedHandlers, CancellationToken cancellationToken) { - IExtensionMessageHandlerWrapper? handler; - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) - { - // handlers here is either _workspaceHandlers or _documentHandlers, so it must be protected - // by _lockObject. - if (!handlers.TryGetValue(messageName, out handler)) - { - throw new InvalidOperationException($"No handler found for message {messageName}."); - } - } + var lazy = ImmutableInterlocked.GetOrAdd( + ref cachedHandlers, + messageName, + static (messageName, arg) => AsyncLazy.Create( + static (arg, cancellationToken) => ComputeHandlersAsync(arg.@this, arg.messageName, arg.isSolution, cancellationToken), + (messageName, arg.@this, arg.isSolution)), + (@this: this, executeArgument, isSolution)); + + var handlers = await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); + if (handlers.Length == 0) + throw new InvalidOperationException($"No handler found for message {messageName}."); + + if (handlers.Length > 1) + throw new InvalidOperationException($"Multiple handlers found for message {messageName}."); + + var handler = handlers[0]; + if (s_disabledExtensionHandlers.TryGetValue(handler, out _)) + throw new InvalidOperationException($"Handler was disabled due to previous exception."); try { var message = JsonSerializer.Deserialize(jsonMessage, handler.MessageType); - var result = await handler.ExecuteAsync(message, argument, cancellationToken) - .ConfigureAwait(false); - var responseJson = JsonSerializer.Serialize(result, handler.ResponseType); - return responseJson; + var result = await handler.ExecuteAsync(message, executeArgument, cancellationToken).ConfigureAwait(false); + return JsonSerializer.Serialize(result, handler.ResponseType); } - catch + catch (Exception ex) when (DisableHandlerAndPropagate(ex)) { - // Any exception thrown in this method is left to bubble up to the extension. - // But we unregister all handlers from that assembly to minimize the impact of a bad extension. - await UnregisterExtensionAsync(assemblyFilePath: handler.ExtensionIdentifier, cancellationToken).ConfigureAwait(false); - throw; + throw ExceptionUtilities.Unreachable(); } - } - private async Task RegisterAssemblyAsync( - Extension extension, - string assemblyFileName, - AssemblyHandlers? assemblyHandlers, - CancellationToken cancellationToken) - { - using (await _lock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + bool DisableHandlerAndPropagate(Exception ex) { - // Make sure a call to UnloadCustomMessageHandlersAsync hasn't happened while we relinquished the lock on _lockObject - if (!_extensions.TryGetValue(extension.AssemblyFolderPath, out var currentExtension) || !extension.Equals(currentExtension)) - { - throw new InvalidOperationException($"Handlers in {extension.AssemblyFolderPath} were unregistered while loading handlers."); - } + FatalError.ReportNonFatalError(ex, ErrorSeverity.Critical); - try - { - if (assemblyHandlers is not null) - { - var duplicateHandler = _workspaceHandlers.Keys.Intersect(assemblyHandlers.WorkspaceMessageHandlers.Keys).Concat( - _documentHandlers.Keys.Intersect(assemblyHandlers.DocumentMessageHandlers.Keys)).FirstOrDefault(); - - if (duplicateHandler is not null) - { - assemblyHandlers = null; - throw new InvalidOperationException($"Handler name {duplicateHandler} is already registered."); - } + // Any exception thrown in this method is left to bubble up to the extension. But we unregister this handler + // from that assembly to minimize the impact. + s_disabledExtensionHandlers.TryAdd(handler, handler); + return false; + } + } - foreach (var handler in assemblyHandlers.WorkspaceMessageHandlers) - { - _workspaceHandlers.Add(handler.Key, handler.Value); - } + private static async Task>> ComputeHandlersAsync( + ExtensionMessageHandlerService @this, string messageName, bool isSolution, CancellationToken cancellationToken) + { + using var _ = ArrayBuilder>.GetInstance(out var result); + foreach (var (_, lazyExtension) in @this._folderPathToExtensionFolder) + { + cancellationToken.ThrowIfCancellationRequested(); - foreach (var handler in assemblyHandlers.DocumentMessageHandlers) - { - _documentHandlers.Add(handler.Key, handler.Value); - } - } - } - finally - { - extension.SetAssemblyHandlers(assemblyFileName, assemblyHandlers); - } + var extension = await lazyExtension.GetValueAsync(cancellationToken).ConfigureAwait(false); + await extension.AddHandlersAsync(messageName, isSolution, result, cancellationToken).ConfigureAwait(false); } + + return result.ToImmutable(); } - private sealed class Extension( - ExtensionMessageHandlerService extensionMessageHandlerService, - IAnalyzerAssemblyLoaderInternal analyzerAssemblyLoader, - string assemblyFolderPath) + private interface IExtensionFolder { - private readonly ExtensionMessageHandlerService _extensionMessageHandlerService = extensionMessageHandlerService; + ValueTask RegisterAssemblyAsync(string assemblyFilePath, CancellationToken cancellationToken); - public readonly IAnalyzerAssemblyLoaderInternal AnalyzerAssemblyLoader = analyzerAssemblyLoader; + /// + /// Unregisters this assembly path from this extension folder. If this was the last registered path, then this + /// will return true so that this folder can be unloaded. + /// + ValueTask UnregisterAssemblyAsync(string assemblyFilePath, CancellationToken cancellationToken); + + ValueTask AddHandlersAsync(string messageName, bool isSolution, ArrayBuilder> result, CancellationToken cancellationToken); + } - public readonly string AssemblyFolderPath = assemblyFolderPath; + /// + /// Trivial placeholder impl of when we fail for some reason to even process the + /// folder we are told contains extensions. + /// + private sealed class TrivialExtensionFolder : IExtensionFolder + { + public static readonly TrivialExtensionFolder Instance = new(); /// - /// Gets the object that is used to lock in order to avoid multiple calls from the same extensions to load the - /// same assembly concurrently resulting in the constructors of the same handlers being called more than once. - /// All other concurrent operations, including modifying are protected by . + /// No lock needed as registratin/unregistration must happen serially. /// - private readonly SemaphoreSlim _assemblyLoadLock = new(initialCount: 1); + private readonly List _registeredFilePaths = []; - private readonly Dictionary _assemblies = new(); + public ValueTask RegisterAssemblyAsync(string assemblyFilePath, CancellationToken cancellationToken) + { + _registeredFilePaths.Add(assemblyFilePath); + return new(AssemblyHandlers.Empty); + } - public void SetAssemblyHandlers(string assemblyFileName, AssemblyHandlers? value) + public ValueTask UnregisterAssemblyAsync(string assemblyFilePath, CancellationToken cancellationToken) { - EnsureGlobalLockIsOwned(); - _assemblies[assemblyFileName] = value; + _registeredFilePaths.Remove(assemblyFilePath); + return new(_registeredFilePaths.Count == 0); } - public bool TryGetAssemblyHandlers(string assemblyFileName, out AssemblyHandlers? value) + public ValueTask AddHandlersAsync(string messageName, bool isSolution, ArrayBuilder> result, CancellationToken cancellationToken) + => default; + } + + private sealed class ExtensionFolder( + ExtensionMessageHandlerService extensionMessageHandlerService, + IAnalyzerAssemblyLoaderInternal analyzerAssemblyLoader) : IExtensionFolder + { + private readonly ExtensionMessageHandlerService _extensionMessageHandlerService = extensionMessageHandlerService; + private readonly IAnalyzerAssemblyLoaderInternal _analyzerAssemblyLoader = analyzerAssemblyLoader; + + private ImmutableDictionary> _assemblyFilePathToHandlers = ImmutableDictionary>.Empty; + + public static IExtensionFolder Create( + ExtensionMessageHandlerService extensionMessageHandlerService, + string assemblyFolderPath, + CancellationToken cancellationToken) { - EnsureGlobalLockIsOwned(); - return _assemblies.TryGetValue(assemblyFileName, out value); + var analyzerAssemblyLoaderProvider = extensionMessageHandlerService._solutionServices.GetRequiredService(); + var analyzerAssemblyLoader = analyzerAssemblyLoaderProvider.CreateNewShadowCopyLoader(); + + try + { + // Allow this assembly loader to load any dll in assemblyFolderPath. + foreach (var dll in Directory.EnumerateFiles(assemblyFolderPath, "*.dll")) + { + cancellationToken.ThrowIfCancellationRequested(); + try + { + // Check if the file is a valid .NET assembly. + AssemblyName.GetAssemblyName(dll); + } + catch + { + // The file is not a valid .NET assembly, skip it. + continue; + } + + analyzerAssemblyLoader.AddDependencyLocation(dll); + } + + return new ExtensionFolder(extensionMessageHandlerService, analyzerAssemblyLoader); + } + catch (Exception ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.Critical)) + { + // TODO: Log this exception so the client knows something went wrong. + return new TrivialExtensionFolder(); + } } - public bool RemoveAssemblyHandlers(string assemblyFileName, out AssemblyHandlers? value) + public async ValueTask RegisterAssemblyAsync( + string assemblyFilePath, CancellationToken cancellationToken) { - EnsureGlobalLockIsOwned(); - return _assemblies.Remove(assemblyFileName, out value); + var lazy = ImmutableInterlocked.GetOrAdd( + ref _assemblyFilePathToHandlers, + assemblyFilePath, + static (assemblyFilePath, @this) => AsyncLazy.Create( + static (args, cancellationToken) => CreateAssemblyHandlers(args.@this, args.assemblyFilePath, cancellationToken), + (assemblyFilePath, @this)), + this); + + return await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); } - public int AssemblyHandlersCount + private static AssemblyHandlers CreateAssemblyHandlers( + ExtensionFolder @this, string assemblyFilePath, CancellationToken cancellationToken) { - get + try + { + var assembly = @this._analyzerAssemblyLoader.LoadFromPath(assemblyFilePath); + var factory = @this._extensionMessageHandlerService._customMessageHandlerFactory; + + var messageWorkspaceHandlers = factory + .CreateWorkspaceMessageHandlers(assembly, extensionIdentifier: assemblyFilePath, cancellationToken) + .ToImmutableDictionary(h => h.Name); + var messageDocumentHandlers = factory + .CreateDocumentMessageHandlers(assembly, extensionIdentifier: assemblyFilePath, cancellationToken) + .ToImmutableDictionary(h => h.Name); + + return new AssemblyHandlers() + { + WorkspaceMessageHandlers = messageWorkspaceHandlers, + DocumentMessageHandlers = messageDocumentHandlers, + }; + + // We don't add assemblyHandlers to _assemblies here and instead let _extensionMessageHandlerService.RegisterAssembly do it + // since RegisterAssembly can still fail if there are duplicated handler names. + } + catch (Exception ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.General)) { - EnsureGlobalLockIsOwned(); - return _assemblies.Count; + // TODO: Log this exception so the client knows something went wrong. + return AssemblyHandlers.Empty; } } - public async ValueTask LoadAssemblyAsync( - string assemblyFileName, CancellationToken cancellationToken) + public async ValueTask AddHandlersAsync(string messageName, bool isSolution, ArrayBuilder> result, CancellationToken cancellationToken) { - var assemblyFilePath = Path.Combine(AssemblyFolderPath, assemblyFileName); - - // AssemblyLoadLockObject is only used to avoid multiple calls from the same extensions to load the same assembly concurrently - // resulting in the constructors of the same handlers being called more than once. - // All other concurrent operations, including modifying extension.Assemblies are protected by _lockObject. - using (await _assemblyLoadLock.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + foreach (var (_, lazy) in _assemblyFilePathToHandlers) { - AssemblyHandlers? assemblyHandlers = null; - Exception? exception = null; - try - { - var assembly = AnalyzerAssemblyLoader.LoadFromPath(assemblyFilePath); - var factory = _extensionMessageHandlerService._customMessageHandlerFactory; - var messageWorkspaceHandlers = factory.CreateWorkspaceMessageHandlers(assembly, extensionIdentifier: assemblyFilePath) - .ToImmutableDictionary(h => h.Name, h => h); - var messageDocumentHandlers = factory.CreateDocumentMessageHandlers(assembly, extensionIdentifier: assemblyFilePath) - .ToImmutableDictionary(h => h.Name, h => h); - - assemblyHandlers = new AssemblyHandlers() - { - WorkspaceMessageHandlers = messageWorkspaceHandlers, - DocumentMessageHandlers = messageDocumentHandlers, - }; + cancellationToken.ThrowIfCancellationRequested(); - // We don't add assemblyHandlers to _assemblies here and instead let _extensionMessageHandlerService.RegisterAssembly do it - // since RegisterAssembly can still fail if there are duplicated handler names. - } - catch (Exception e) when (FatalError.ReportAndPropagate(exception = e, ErrorSeverity.General)) + var handlers = await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); + if (isSolution) { - throw ExceptionUtilities.Unreachable(); + if (handlers.WorkspaceMessageHandlers.TryGetValue(messageName, out var handler)) + result.Add((IExtensionMessageHandlerWrapper)handler); } - finally + else { - // In case of exception, we cache null so that we don't try to load the same assembly again. - await _extensionMessageHandlerService.RegisterAssemblyAsync( - this, assemblyFileName, exception is null ? assemblyHandlers : null, cancellationToken).ConfigureAwait(false); + if (handlers.DocumentMessageHandlers.TryGetValue(messageName, out var handler)) + result.Add((IExtensionMessageHandlerWrapper)handler); } - - // The return is here, after RegisterAssembly, since RegisterAssembly can also throw an exception: the registration is not - // completed until RegisterAssembly returns. - return new( - [.. assemblyHandlers.WorkspaceMessageHandlers.Keys], - [.. assemblyHandlers.DocumentMessageHandlers.Keys]); } } - private void EnsureGlobalLockIsOwned() + public ValueTask UnregisterAssemblyAsync(string assemblyFilePath, CancellationToken cancellationToken) { - Contract.ThrowIfTrue(_extensionMessageHandlerService._lock.CurrentCount != 0, "Global lock should be owned"); + _assemblyFilePathToHandlers = _assemblyFilePathToHandlers.Remove(assemblyFilePath); + return new(_assemblyFilePathToHandlers.IsEmpty); } } private sealed class AssemblyHandlers { + public static readonly AssemblyHandlers Empty = new() + { + DocumentMessageHandlers = ImmutableDictionary>.Empty, + WorkspaceMessageHandlers = ImmutableDictionary>.Empty, + }; + /// /// Gets the document-specific handlers that can be passed to , indexed by their name. /// diff --git a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerFactory.cs b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerFactory.cs index 8e2b084368495..7f19b218d6c9c 100644 --- a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerFactory.cs +++ b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerFactory.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using System.Reflection; +using System.Threading; namespace Microsoft.CodeAnalysis.Extensions; @@ -19,7 +20,8 @@ internal interface IExtensionMessageHandlerFactory /// The assembly to scan for handlers. /// Unique identifier of the extension owning this handler. /// The handlers. - ImmutableArray> CreateWorkspaceMessageHandlers(Assembly assembly, string extensionIdentifier); + ImmutableArray> CreateWorkspaceMessageHandlers( + Assembly assembly, string extensionIdentifier, CancellationToken cancellationToken); /// /// Creates instances for each @@ -28,5 +30,6 @@ internal interface IExtensionMessageHandlerFactory /// The assembly to scan for handlers. /// Unique identifier of the extension owning this handler. /// The handlers. - ImmutableArray> CreateDocumentMessageHandlers(Assembly assembly, string extensionIdentifier); + ImmutableArray> CreateDocumentMessageHandlers( + Assembly assembly, string extensionIdentifier, CancellationToken cancellationToken); } diff --git a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerService.cs b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerService.cs index 0692a71cb759d..af16a65172367 100644 --- a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerService.cs +++ b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerService.cs @@ -13,14 +13,38 @@ namespace Microsoft.CodeAnalysis.Extensions; /// internal interface IExtensionMessageHandlerService : IWorkspaceService { + /// + /// Registers extension message handlers from the specified assembly. + /// + /// The assembly to register and create message handlers from. + /// The names of the registered handlers. + /// Should be called serially with other , , or calls. + ValueTask RegisterExtensionAsync(string assemblyFilePath, CancellationToken cancellationToken); + + /// + /// Unregisters extension message handlers previously registered from . + /// + /// The assembly for which handlers should be unregistered. + /// Should be called serially with other , , or calls. + ValueTask UnregisterExtensionAsync(string assemblyFilePath, CancellationToken cancellationToken); + + /// + /// Unregisters all extension message handlers. + /// + /// Should be called serially with other , , or calls. + ValueTask ResetAsync(CancellationToken cancellationToken); + /// /// Executes a non-document-specific extension message handler with the given message and solution. /// /// The solution the message refers to. /// The name of the handler to execute. This is generally the full name of the type implementing the handler. /// The json message to be passed to the handler. - /// Cancellation token to cancel the async operation. /// The json message returned by the handler. + /// Can be called concurrently with other message requests. ValueTask HandleExtensionWorkspaceMessageAsync( Solution solution, string messageName, @@ -33,30 +57,11 @@ ValueTask HandleExtensionWorkspaceMessageAsync( /// The document the message refers to. /// The name of the handler to execute. This is generally the full name of the type implementing the handler. /// The json message to be passed to the handler. - /// Cancellation token to cancel the async operation. /// The json message returned by the handler. + /// Can be called concurrently with other message requests. ValueTask HandleExtensionDocumentMessageAsync( Document documentId, string messageName, string jsonMessage, CancellationToken cancellationToken); - - /// - /// Registers extension message handlers from the specified assembly. - /// - /// The assembly to register and create message handlers from. - /// The names of the registered handlers. - ValueTask RegisterExtensionAsync(string assemblyFilePath, CancellationToken cancellationToken); - - /// - /// Unregisters extension message handlers previously registered from . - /// - /// The assembly for which handlers should be unregistered. - /// A task representing the async operation. - ValueTask UnregisterExtensionAsync(string assemblyFilePath, CancellationToken cancellationToken); - - /// - /// Unregisters all extension message handlers. - /// - ValueTask ResetAsync(CancellationToken cancellationToken); } diff --git a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerWrapper.cs b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerWrapper.cs index 607cbbe4b892a..4a13a5e957e57 100644 --- a/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerWrapper.cs +++ b/src/Features/Core/Portable/Extensions/IExtensionMessageHandlerWrapper.cs @@ -8,13 +8,17 @@ namespace Microsoft.CodeAnalysis.Extensions; +internal interface IExtensionMessageHandlerWrapper +{ +} + /// /// Wrapper for an IExtensionWorkspaceMessageHandler or IExtensionDocumentMessageHandler /// as returned by . /// /// The type of object received as parameter by the extension message /// handler. -internal interface IExtensionMessageHandlerWrapper +internal interface IExtensionMessageHandlerWrapper : IExtensionMessageHandlerWrapper { /// /// The type of object received as parameter by the extension message handler. diff --git a/src/LanguageServer/Protocol/Handler/Extensions/ExtensionRegisterHandler.cs b/src/LanguageServer/Protocol/Handler/Extensions/ExtensionRegisterHandler.cs index f72272f641816..dafdb4c7379ca 100644 --- a/src/LanguageServer/Protocol/Handler/Extensions/ExtensionRegisterHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Extensions/ExtensionRegisterHandler.cs @@ -20,7 +20,12 @@ internal sealed class ExtensionRegisterHandler() { private const string MethodName = "roslyn/extensionRegister"; - public bool MutatesSolutionState => false; + /// + /// Report that we mutate solution state so that we only attempt to register or unregister one extension at a time. + /// This ensures we don't have to handle any threading concerns while this is happening. As this should be a rare + /// operation, this simplifies things while ideally being low cost. + /// + public bool MutatesSolutionState => true; public bool RequiresLSPSolution => true; diff --git a/src/LanguageServer/Protocol/Handler/Extensions/ExtensionUnregisterHandler.cs b/src/LanguageServer/Protocol/Handler/Extensions/ExtensionUnregisterHandler.cs index 42ca41e4d1e71..c9d1ce218310d 100644 --- a/src/LanguageServer/Protocol/Handler/Extensions/ExtensionUnregisterHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Extensions/ExtensionUnregisterHandler.cs @@ -20,7 +20,12 @@ internal sealed class ExtensionUnregisterHandler() { private const string MethodName = "roslyn/extensionUnregister"; - public bool MutatesSolutionState => false; + /// + /// Report that we mutate solution state so that we only attempt to register or unregister one extension at a time. + /// This ensures we don't have to handle any threading concerns while this is happening. As this should be a rare + /// operation, this simplifies things while ideally being low cost. + /// + public bool MutatesSolutionState => true; public bool RequiresLSPSolution => true; diff --git a/src/LanguageServer/Protocol/Handler/ServerLifetime/LspServiceLifeCycleManager.cs b/src/LanguageServer/Protocol/Handler/ServerLifetime/LspServiceLifeCycleManager.cs index 94cea1ac624d1..94ca5ca32f332 100644 --- a/src/LanguageServer/Protocol/Handler/ServerLifetime/LspServiceLifeCycleManager.cs +++ b/src/LanguageServer/Protocol/Handler/ServerLifetime/LspServiceLifeCycleManager.cs @@ -9,7 +9,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Extensions; using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.Remote; using Microsoft.CommonLanguageServerProtocol.Framework; using Roslyn.LanguageServer.Protocol; using StreamJsonRpc; @@ -41,21 +40,14 @@ private LspServiceLifeCycleManager(IClientLanguageServerManager clientLanguageSe public async Task ShutdownAsync(string message = "Shutting down") { + // Shutting down is not cancellable. + var cancellationToken = CancellationToken.None; + var hostWorkspace = _lspWorkspaceRegistrationService.GetAllRegistrations().SingleOrDefault(w => w.Kind == WorkspaceKind.Host); if (hostWorkspace is not null) { - var client = await RemoteHostClient.TryGetClientAsync(hostWorkspace, CancellationToken.None).ConfigureAwait(false); - if (client is not null) - { - await client.TryInvokeAsync( - (service, cancellationToken) => service.ResetAsync(cancellationToken), - CancellationToken.None).ConfigureAwait(false); - } - else - { - var service = hostWorkspace.Services.GetRequiredService(); - service.Reset(); - } + var service = hostWorkspace.Services.GetRequiredService(); + await service.ResetAsync(cancellationToken).ConfigureAwait(false); } try @@ -65,7 +57,7 @@ await client.TryInvokeAsync( MessageType = MessageType.Info, Message = message }; - await _clientLanguageServerManager.SendNotificationAsync("window/logMessage", messageParams, CancellationToken.None).ConfigureAwait(false); + await _clientLanguageServerManager.SendNotificationAsync("window/logMessage", messageParams, cancellationToken).ConfigureAwait(false); } catch (Exception ex) when (ex is ObjectDisposedException or ConnectionLostException) { diff --git a/src/Tools/ExternalAccess/Extensions/Internal/ExtensionMessageHandlerFactory.cs b/src/Tools/ExternalAccess/Extensions/Internal/ExtensionMessageHandlerFactory.cs index ddd0a3b317c44..9f8bc626bd28d 100644 --- a/src/Tools/ExternalAccess/Extensions/Internal/ExtensionMessageHandlerFactory.cs +++ b/src/Tools/ExternalAccess/Extensions/Internal/ExtensionMessageHandlerFactory.cs @@ -6,6 +6,7 @@ using System.Collections.Immutable; using System.Composition; using System.Reflection; +using System.Threading; using Microsoft.CodeAnalysis.Host.Mef; namespace Microsoft.CodeAnalysis.Extensions; @@ -15,27 +16,34 @@ namespace Microsoft.CodeAnalysis.Extensions; [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] internal sealed class ExtensionMessageHandlerFactory() : IExtensionMessageHandlerFactory { - public ImmutableArray> CreateDocumentMessageHandlers(Assembly assembly, string extensionIdentifier) + public ImmutableArray> CreateDocumentMessageHandlers( + Assembly assembly, string extensionIdentifier, CancellationToken cancellationToken) => CreateWorkspaceHandlers( assembly, typeof(IExtensionDocumentMessageHandler<,>), - (handler, handlerInterface) => new ExtensionDocumentMessageHandlerWrapper(handler, handlerInterface, extensionIdentifier)); + (handler, handlerInterface) => new ExtensionDocumentMessageHandlerWrapper(handler, handlerInterface, extensionIdentifier), + cancellationToken); - public ImmutableArray> CreateWorkspaceMessageHandlers(Assembly assembly, string extensionIdentifier) + public ImmutableArray> CreateWorkspaceMessageHandlers( + Assembly assembly, string extensionIdentifier, CancellationToken cancellationToken) => CreateWorkspaceHandlers( assembly, typeof(IExtensionWorkspaceMessageHandler<,>), - (handler, handlerInterface) => new ExtensionWorkspaceMessageHandlerWrapper(handler, handlerInterface, extensionIdentifier)); + (handler, handlerInterface) => new ExtensionWorkspaceMessageHandlerWrapper(handler, handlerInterface, extensionIdentifier), + cancellationToken); private static ImmutableArray> CreateWorkspaceHandlers( Assembly assembly, Type unboundInterfaceType, - Func> wrapperCreator) + Func> wrapperCreator, + CancellationToken cancellationToken) { var resultBuilder = ImmutableArray.CreateBuilder>(); foreach (var candidateType in assembly.GetTypes()) { + cancellationToken.ThrowIfCancellationRequested(); + if (candidateType.IsAbstract || candidateType.IsGenericType) { continue; diff --git a/src/Tools/ExternalAccess/Extensions/InternalAPI.Unshipped.txt b/src/Tools/ExternalAccess/Extensions/InternalAPI.Unshipped.txt index 585364db866e3..d556753bb5665 100644 --- a/src/Tools/ExternalAccess/Extensions/InternalAPI.Unshipped.txt +++ b/src/Tools/ExternalAccess/Extensions/InternalAPI.Unshipped.txt @@ -27,8 +27,8 @@ Microsoft.CodeAnalysis.Extensions.ExtensionHandlerWrapper.Name.get -> Microsoft.CodeAnalysis.Extensions.ExtensionHandlerWrapper.ResponseType.get -> System.Type! Microsoft.CodeAnalysis.Extensions.ExtensionMessageContext.ExtensionMessageContext(Microsoft.CodeAnalysis.Solution! solution) -> void Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory -Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory.CreateDocumentMessageHandlers(System.Reflection.Assembly! assembly, string! extensionIdentifier) -> System.Collections.Immutable.ImmutableArray!> -Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory.CreateWorkspaceMessageHandlers(System.Reflection.Assembly! assembly, string! extensionIdentifier) -> System.Collections.Immutable.ImmutableArray!> +Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory.CreateDocumentMessageHandlers(System.Reflection.Assembly! assembly, string! extensionIdentifier, System.Threading.CancellationToken cancellationToken) -> System.Collections.Immutable.ImmutableArray!> +Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory.CreateWorkspaceMessageHandlers(System.Reflection.Assembly! assembly, string! extensionIdentifier, System.Threading.CancellationToken cancellationToken) -> System.Collections.Immutable.ImmutableArray!> Microsoft.CodeAnalysis.Extensions.ExtensionMessageHandlerFactory.ExtensionMessageHandlerFactory() -> void Microsoft.CodeAnalysis.Extensions.ExtensionWorkspaceMessageHandlerWrapper Microsoft.CodeAnalysis.Extensions.ExtensionWorkspaceMessageHandlerWrapper.ExtensionWorkspaceMessageHandlerWrapper(object! handler, System.Type! customMessageHandlerInterface, string! extensionIdentifier) -> void \ No newline at end of file