-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Refactors LSP server extension assembly loading #71862
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
Changes from 2 commits
2a220b2
fe8e648
dc3bf43
b1142c5
dc40ba3
cdcf895
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,98 +2,95 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Reflection; | ||
| using System.Runtime.Loader; | ||
| using Microsoft.CodeAnalysis.LanguageServer.Services; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.VisualStudio.Composition; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.LanguageServer; | ||
|
|
||
| internal class CustomExportAssemblyLoader : IAssemblyLoader | ||
| /// <summary> | ||
| /// Defines a MEF assembly loader that knows how to load assemblies from both the default assembly load context | ||
| /// and from the assembly load contexts for any of our extensions. | ||
| /// </summary> | ||
| internal class CustomExportAssemblyLoader(ExtensionAssemblyManager extensionAssemblyManager, ILoggerFactory loggerFactory) : IAssemblyLoader | ||
| { | ||
| private readonly ILogger _logger = loggerFactory.CreateLogger("MEF Assembly Loader"); | ||
| /// <summary> | ||
| /// Cache assemblies that are already loaded by AssemblyName comparison | ||
| /// Loads assemblies from either the host or from our extensions. | ||
| /// If an assembly exists in both the host and an extension, we will use the host assembly for the MEF catalog. | ||
| /// If an assembly exists in two extensions, we use the first one we find for the MEF catalog. | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// </summary> | ||
| private readonly Dictionary<AssemblyName, Assembly> _loadedAssemblies = new Dictionary<AssemblyName, Assembly>(AssemblyNameComparer.Instance); | ||
|
|
||
| /// <summary> | ||
| /// Base directory to search for <see cref="Assembly.LoadFrom(string)"/> if initial load fails | ||
| /// </summary> | ||
| private readonly string _baseDirectory; | ||
| public Assembly LoadAssembly(AssemblyName assemblyName) | ||
| { | ||
| // VS-MEF generally tries to populate AssemblyName.CodeBase with the path to the assembly being loaded. | ||
| // We need to read this in order to figure out which ALC we should load the assembly into. | ||
| #pragma warning disable SYSLIB0044 // Type or member is obsolete | ||
| var codeBasePath = assemblyName.CodeBase; | ||
| #pragma warning restore SYSLIB0044 // Type or member is obsolete | ||
| return LoadAssembly(assemblyName, codeBasePath); | ||
| } | ||
|
|
||
| public CustomExportAssemblyLoader(string baseDirectory) | ||
| public Assembly LoadAssembly(string assemblyFullName, string? codeBasePath) | ||
| { | ||
| _baseDirectory = baseDirectory; | ||
| var assemblyName = new AssemblyName(assemblyFullName); | ||
| return LoadAssembly(assemblyName, codeBasePath); | ||
| } | ||
|
|
||
| public Assembly LoadAssembly(AssemblyName assemblyName) | ||
| private Assembly LoadAssembly(AssemblyName assemblyName, string? codeBasePath) | ||
| { | ||
| Assembly? value; | ||
| lock (_loadedAssemblies) | ||
| _logger.LogTrace($"Loading assembly {assemblyName}"); | ||
| // First attempt to load the assembly from the default context. | ||
| Exception loadException; | ||
| try | ||
| { | ||
| _loadedAssemblies.TryGetValue(assemblyName, out value); | ||
| return AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName); | ||
| } | ||
|
|
||
| if (value == null) | ||
| catch (FileNotFoundException ex) when (assemblyName.Name is not null) | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| // Attempt to load the assembly normally, but fall back to Assembly.LoadFrom in the base | ||
| // directory if the assembly load fails | ||
| try | ||
| { | ||
| value = Assembly.Load(assemblyName); | ||
| } | ||
| catch (FileNotFoundException) when (assemblyName.Name is not null) | ||
| { | ||
| var filePath = Path.Combine(_baseDirectory, assemblyName.Name) | ||
| + (assemblyName.Name.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) | ||
| ? "" | ||
| : ".dll"); | ||
|
|
||
| value = Assembly.LoadFrom(filePath); | ||
|
|
||
| if (value is null) | ||
| { | ||
| throw; | ||
| } | ||
| } | ||
| loadException = ex; | ||
| // continue checking the extension contexts. | ||
| } | ||
|
|
||
| lock (_loadedAssemblies) | ||
| { | ||
| _loadedAssemblies[assemblyName] = value; | ||
| return value; | ||
| } | ||
| if (codeBasePath is not null) | ||
| { | ||
| return LoadAssemblyFromCodeBase(assemblyName, codeBasePath); | ||
| } | ||
|
Comment on lines
+56
to
59
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Random thought: if we have a codebase path and we know the path isn't our application direectory, should have just done this first? Rather than that earlier load that we could have predicted would fail? |
||
|
|
||
| return value; | ||
| } | ||
| // We don't have a code base path for this assembly. We'll search the extension contexts | ||
| // and load the first one that ships the assembly. | ||
| var assembly = extensionAssemblyManager.SearchExtensionContextsForAssembly(assemblyName); | ||
| if (assembly is not null) | ||
| { | ||
| _logger.LogTrace("{assemblyName} found in extension context without code base", assemblyName); | ||
jasonmalinowski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return assembly; | ||
| } | ||
|
|
||
| public Assembly LoadAssembly(string assemblyFullName, string? codeBasePath) | ||
| { | ||
| var assemblyName = new AssemblyName(assemblyFullName); | ||
| return LoadAssembly(assemblyName); | ||
| _logger.LogTrace("{assemblyName} not found in any host or extension context", assemblyName); | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| throw loadException; | ||
| } | ||
|
|
||
| private class AssemblyNameComparer : IEqualityComparer<AssemblyName> | ||
| private Assembly LoadAssemblyFromCodeBase(AssemblyName assemblyName, string codeBaseUriStr) | ||
| { | ||
| public static AssemblyNameComparer Instance = new AssemblyNameComparer(); | ||
|
|
||
| public bool Equals(AssemblyName? x, AssemblyName? y) | ||
| // CodeBase is spec'd as being a URL string. | ||
| var codeBaseUri = ProtocolConversions.CreateAbsoluteUri(codeBaseUriStr); | ||
| if (!codeBaseUri.IsFile) | ||
| { | ||
| if (x == null && y == null) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| if (x == null || y == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return x.Name == y.Name; | ||
| throw new ArgumentException($"Code base {codeBaseUriStr} is not a file URI.", codeBaseUriStr); | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| public int GetHashCode([DisallowNull] AssemblyName obj) | ||
| var codeBasePath = codeBaseUri.LocalPath; | ||
|
|
||
| var assembly = extensionAssemblyManager.TryLoadAssemblyInExtensionContext(codeBasePath); | ||
| if (assembly is not null) | ||
| { | ||
| return obj.Name?.GetHashCode(StringComparison.Ordinal) ?? 0; | ||
| _logger.LogTrace("{assemblyName} with code base {codeBase} found in extension context", assemblyName, codeBasePath); | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return assembly; | ||
| } | ||
|
|
||
| // We were given an explicit code base path, but no extension context had the assembly. | ||
| // This is unexpected, so we'll throw an exception. | ||
| throw new FileNotFoundException($"Could not find assembly {assemblyName} with code base {codeBasePath} in any extension context."); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,17 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Collections.Immutable; | ||
| using System.Composition; | ||
| using System.Reflection; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
| using Microsoft.CodeAnalysis.Host.Mef; | ||
| using Microsoft.CodeAnalysis.LanguageServer.Services; | ||
| using Microsoft.CodeAnalysis.Options; | ||
| using Microsoft.CodeAnalysis.SolutionCrawler; | ||
| using Microsoft.Extensions.Logging; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.LanguageServer.ExternalAccess.VSCode.API; | ||
| namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace; | ||
|
|
||
| [Export(typeof(VSCodeAnalyzerLoader)), Shared] | ||
| internal class VSCodeAnalyzerLoader | ||
|
|
@@ -34,8 +37,33 @@ public void InitializeDiagnosticsServices(Workspace workspace) | |
| _diagnosticService.Register((IDiagnosticUpdateSource)_analyzerService); | ||
| } | ||
|
|
||
| public static IAnalyzerAssemblyLoader CreateAnalyzerAssemblyLoader() | ||
| public static IAnalyzerAssemblyLoader CreateAnalyzerAssemblyLoader(ExtensionAssemblyManager extensionAssemblyManager, ILogger logger) | ||
| { | ||
| return new DefaultAnalyzerAssemblyLoader(); | ||
| return new VSCodeExtensionAssemblyAnalyzerLoader(extensionAssemblyManager, logger); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Analyzer loader that will re-use already loaded assemblies from the extension load context. | ||
| /// </summary> | ||
| private class VSCodeExtensionAssemblyAnalyzerLoader(ExtensionAssemblyManager extensionAssemblyManager, ILogger logger) : IAnalyzerAssemblyLoader | ||
| { | ||
| private readonly DefaultAnalyzerAssemblyLoader _defaultLoader = new(); | ||
|
|
||
| public void AddDependencyLocation(string fullPath) | ||
| { | ||
| _defaultLoader.AddDependencyLocation(fullPath); | ||
| } | ||
|
|
||
| public Assembly LoadFromPath(string fullPath) | ||
| { | ||
| var assembly = extensionAssemblyManager.TryLoadAssemblyInExtensionContext(fullPath); | ||
| if (assembly is not null) | ||
| { | ||
| logger.LogTrace("Loaded analyzer {fullPath} from extension context", fullPath); | ||
| return assembly; | ||
| } | ||
|
|
||
| return _defaultLoader.LoadFromPath(fullPath); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this won't reuse it, right? Because this is going to give it a file path and then reload that again in the default implementation's ALC?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if we do not find the assembly in an extension ALC, it will fallback to the current behavior (which is a separate analyzer ALC). |
||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.