-
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 1 commit
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,48 @@ | |
| // 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.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) : IAssemblyLoader | ||
| { | ||
| /// <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 CustomExportAssemblyLoader(string baseDirectory) | ||
| { | ||
| _baseDirectory = baseDirectory; | ||
| } | ||
|
|
||
| public Assembly LoadAssembly(AssemblyName assemblyName) | ||
| { | ||
| Assembly? value; | ||
| lock (_loadedAssemblies) | ||
| // First attempt to load the assembly from the default context. | ||
| try | ||
| { | ||
| _loadedAssemblies.TryGetValue(assemblyName, out value); | ||
| return AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName); | ||
| } | ||
|
|
||
| if (value == null) | ||
| catch (FileNotFoundException) when (assemblyName.Name is not null) | ||
| { | ||
| // 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; | ||
| } | ||
| } | ||
| // continue checking the extension contexts. | ||
| } | ||
|
|
||
| lock (_loadedAssemblies) | ||
| { | ||
| _loadedAssemblies[assemblyName] = value; | ||
| return value; | ||
| } | ||
| var extensionAssembly = extensionAssemblyManager.TryLoadAssemblyInExtensionContext(assemblyName); | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (extensionAssembly != null) | ||
| { | ||
| return extensionAssembly; | ||
| } | ||
|
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; | ||
| throw new FileNotFoundException($"Could not find assembly {assemblyName.Name} in any host or extension context."); | ||
dibarbet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| public Assembly LoadAssembly(string assemblyFullName, string? codeBasePath) | ||
| { | ||
| var assemblyName = new AssemblyName(assemblyFullName); | ||
| return LoadAssembly(assemblyName); | ||
| } | ||
|
|
||
| private class AssemblyNameComparer : IEqualityComparer<AssemblyName> | ||
| { | ||
| public static AssemblyNameComparer Instance = new AssemblyNameComparer(); | ||
|
|
||
| public bool Equals(AssemblyName? x, AssemblyName? y) | ||
| { | ||
| if (x == null && y == null) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| if (x == null || y == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return x.Name == y.Name; | ||
| } | ||
|
|
||
| public int GetHashCode([DisallowNull] AssemblyName obj) | ||
| { | ||
| return obj.Name?.GetHashCode(StringComparison.Ordinal) ?? 0; | ||
| } | ||
| } | ||
| } | ||
| 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,34 @@ 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> | ||
| /// <param name="extensionAssemblyManager"></param> | ||
| 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). |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| using Microsoft.CodeAnalysis.LanguageServer.HostWorkspace; | ||
| using Microsoft.CodeAnalysis.LanguageServer.LanguageServer; | ||
| using Microsoft.CodeAnalysis.LanguageServer.Logging; | ||
| using Microsoft.CodeAnalysis.LanguageServer.Services; | ||
| using Microsoft.CodeAnalysis.LanguageServer.StarredSuggestions; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.Extensions.Logging.Console; | ||
|
|
@@ -76,7 +77,17 @@ static async Task RunAsync(ServerConfiguration serverConfiguration, Cancellation | |
|
|
||
| logger.LogTrace($".NET Runtime Version: {RuntimeInformation.FrameworkDescription}"); | ||
|
|
||
| using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(serverConfiguration.ExtensionAssemblyPaths, serverConfiguration.DevKitDependencyPath, loggerFactory); | ||
| var extensionAssemblyPaths = serverConfiguration.ExtensionAssemblyPaths.ToImmutableArray(); | ||
| if (serverConfiguration.StarredCompletionsPath != null) | ||
| { | ||
| // TODO - the starred completion component should be passed as a regular extension. | ||
| // Currently we get passed the path to the directory, but ideally we should get passed the path to the dll as part of the --extension argument. | ||
| extensionAssemblyPaths = extensionAssemblyPaths.Add(StarredCompletionAssemblyHelper.GetStarredCompletionAssemblyPath(serverConfiguration.StarredCompletionsPath)); | ||
| } | ||
|
|
||
| var extensionManager = ExtensionAssemblyManager.Create(extensionAssemblyPaths, serverConfiguration.DevKitDependencyPath, loggerFactory); | ||
|
|
||
| using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(serverConfiguration.ExtensionAssemblyPaths, serverConfiguration.DevKitDependencyPath, extensionManager, loggerFactory); | ||
|
||
|
|
||
| // The log file directory passed to us by VSCode might not exist yet, though its parent directory is guaranteed to exist. | ||
| Directory.CreateDirectory(serverConfiguration.ExtensionLogDirectory); | ||
|
|
@@ -96,10 +107,13 @@ static async Task RunAsync(ServerConfiguration serverConfiguration, Cancellation | |
| .Select(f => f.FullName) | ||
| .ToImmutableArray(); | ||
|
|
||
| await workspaceFactory.InitializeSolutionLevelAnalyzersAsync(analyzerPaths); | ||
| // Include analyzers from extension assemblies. | ||
| analyzerPaths = analyzerPaths.AddRange(serverConfiguration.ExtensionAssemblyPaths); | ||
|
||
|
|
||
| await workspaceFactory.InitializeSolutionLevelAnalyzersAsync(analyzerPaths, extensionManager); | ||
|
|
||
| var serviceBrokerFactory = exportProvider.GetExportedValue<ServiceBrokerFactory>(); | ||
| StarredCompletionAssemblyHelper.InitializeInstance(serverConfiguration.StarredCompletionsPath, loggerFactory, serviceBrokerFactory); | ||
| StarredCompletionAssemblyHelper.InitializeInstance(serverConfiguration.StarredCompletionsPath, extensionManager, loggerFactory, serviceBrokerFactory); | ||
| // TODO: Remove, the path should match exactly. Workaround for https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1830914. | ||
| Microsoft.CodeAnalysis.EditAndContinue.EditAndContinueMethodDebugInfoReader.IgnoreCaseWhenComparingDocumentNames = Path.DirectorySeparatorChar == '\\'; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.