diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerFileReferenceTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerFileReferenceTests.cs index ee5884433c796..700e0a6b64f8d 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerFileReferenceTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerFileReferenceTests.cs @@ -750,5 +750,6 @@ public void AddDependencyLocation(string fullPath) { } public bool IsHostAssembly(Assembly assembly) => false; public Assembly LoadFromPath(string fullPath) => throw new Exception(); public string? GetOriginalDependencyLocation(AssemblyName assembly) => throw new Exception(); + public void Dispose() { } } } diff --git a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs index ef1aae7ebd9d8..97657d216693b 100644 --- a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs +++ b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs @@ -46,7 +46,7 @@ internal void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compi var compilerContextCount = compilerContext.Assemblies.Count(); using var tempRoot = new TempRoot(); - AnalyzerAssemblyLoader loader = kind switch + using AnalyzerAssemblyLoader loader = kind switch { AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromDisk, externalResolvers.ToImmutableArray()), AnalyzerTestKind.LoadStream => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromStream, externalResolvers.ToImmutableArray()), @@ -60,7 +60,6 @@ internal void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compi } finally { - loader.UnloadAll(); testOutputHelper.WriteLine($"Test fixture root: {fixture.TempDirectory}"); foreach (var context in loader.GetDirectoryLoadContextsSnapshot()) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs index c18b863c37250..23a1f89ab1eca 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs @@ -13,6 +13,8 @@ using System.Reflection; using System.Runtime.InteropServices; using System.Runtime.Loader; +using Microsoft.CodeAnalysis.ErrorReporting; +using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis { @@ -57,6 +59,8 @@ internal AnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, Analyz public bool IsHostAssembly(Assembly assembly) { + CheckIfDisposed(); + var alc = AssemblyLoadContext.GetLoadContext(assembly); return alc == _compilerLoadContext || alc == AssemblyLoadContext.Default; } @@ -83,25 +87,37 @@ private partial bool IsMatch(AssemblyName requestedName, AssemblyName candidateN internal DirectoryLoadContext[] GetDirectoryLoadContextsSnapshot() { + CheckIfDisposed(); + lock (_guard) { return _loadContextByDirectory.Values.OrderBy(v => v.Directory).ToArray(); } } - internal void UnloadAll() + private partial void DisposeWorker() { - List contexts; + var contexts = ArrayBuilder.GetInstance(); lock (_guard) { - contexts = _loadContextByDirectory.Values.ToList(); + foreach (var (_, context) in _loadContextByDirectory) + contexts.Add(context); + _loadContextByDirectory.Clear(); } foreach (var context in contexts) { - context.Unload(); + try + { + context.Unload(); + } + catch (Exception ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.Critical)) + { + } } + + contexts.Free(); } internal sealed class DirectoryLoadContext : AssemblyLoadContext diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs index 75d6271100354..98bb543cdae6a 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Desktop.cs @@ -34,8 +34,15 @@ internal AnalyzerAssemblyLoader(ImmutableArray extern _externalResolvers = externalResolvers; } + private partial void DisposeWorker() + { + EnsureResolvedUnhooked(); + } + public bool IsHostAssembly(Assembly assembly) { + CheckIfDisposed(); + // When an assembly is loaded from the GAC then the load result would be the same if // this ran on command line compiler. So there is no consistency issue here, this // is just runtime rules expressing themselves. @@ -74,6 +81,8 @@ private partial bool IsMatch(AssemblyName requestedName, AssemblyName candidateN internal bool EnsureResolvedHooked() { + CheckIfDisposed(); + lock (_guard) { if (!_hookedAssemblyResolve) @@ -89,6 +98,8 @@ internal bool EnsureResolvedHooked() internal bool EnsureResolvedUnhooked() { + // Called from Dispose. We don't want to throw if we're disposed. + lock (_guard) { if (_hookedAssemblyResolve) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs index 9b35df57eee87..d3273fed73187 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.cs @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis { - internal interface IAnalyzerAssemblyLoaderInternal : IAnalyzerAssemblyLoader + internal interface IAnalyzerAssemblyLoaderInternal : IAnalyzerAssemblyLoader, IDisposable { /// /// Is this an that the loader considers to be part of the hosting @@ -76,6 +76,11 @@ internal abstract partial class AnalyzerAssemblyLoader : IAnalyzerAssemblyLoader /// winning. private readonly ImmutableArray _externalResolvers; + /// + /// Whether or not we're disposed. Once disposed, all functionality on this type should throw. + /// + private bool _isDisposed; + /// /// The implementation needs to load an with the specified . The /// parameter is the original path. It may be different than @@ -93,8 +98,31 @@ internal abstract partial class AnalyzerAssemblyLoader : IAnalyzerAssemblyLoader /// private partial bool IsMatch(AssemblyName requestedName, AssemblyName candidateName); + private void CheckIfDisposed() + { +#if NET + ObjectDisposedException.ThrowIf(_isDisposed, this); +#else + if (_isDisposed) + throw new ObjectDisposedException(this.GetType().FullName); +#endif + } + + public void Dispose() + { + if (_isDisposed) + return; + + _isDisposed = true; + DisposeWorker(); + } + + private partial void DisposeWorker(); + internal bool IsAnalyzerDependencyPath(string fullPath) { + CheckIfDisposed(); + lock (_guard) { return _analyzerAssemblyInfoMap.ContainsKey(fullPath); @@ -103,6 +131,8 @@ internal bool IsAnalyzerDependencyPath(string fullPath) public void AddDependencyLocation(string fullPath) { + CheckIfDisposed(); + CompilerPathUtilities.RequireAbsolutePath(fullPath, nameof(fullPath)); string simpleName = PathUtilities.GetFileName(fullPath, includeExtension: false); @@ -127,6 +157,8 @@ public void AddDependencyLocation(string fullPath) public Assembly LoadFromPath(string originalAnalyzerPath) { + CheckIfDisposed(); + CompilerPathUtilities.RequireAbsolutePath(originalAnalyzerPath, nameof(originalAnalyzerPath)); (AssemblyName? assemblyName, _) = GetAssemblyInfoForPath(originalAnalyzerPath); @@ -158,6 +190,8 @@ public Assembly LoadFromPath(string originalAnalyzerPath) /// protected (AssemblyName? AssemblyName, string RealAssemblyPath) GetAssemblyInfoForPath(string originalAnalyzerPath) { + CheckIfDisposed(); + lock (_guard) { if (!_analyzerAssemblyInfoMap.TryGetValue(originalAnalyzerPath, out var tuple)) @@ -205,6 +239,8 @@ public Assembly LoadFromPath(string originalAnalyzerPath) /// internal string? GetRealSatelliteLoadPath(string originalAnalyzerPath, CultureInfo cultureInfo) { + CheckIfDisposed(); + string? realSatelliteAssemblyPath = null; lock (_guard) @@ -250,14 +286,19 @@ public Assembly LoadFromPath(string originalAnalyzerPath) } } - public string? GetOriginalDependencyLocation(AssemblyName assemblyName) => - GetBestPath(assemblyName).BestOriginalPath; + public string? GetOriginalDependencyLocation(AssemblyName assemblyName) + { + CheckIfDisposed(); + return GetBestPath(assemblyName).BestOriginalPath; + } /// /// Return the best (original, real) path information for loading an assembly with the specified . /// protected (string? BestOriginalPath, string? BestRealPath) GetBestPath(AssemblyName requestedName) { + CheckIfDisposed(); + if (requestedName.Name is null) { return (null, null); @@ -327,6 +368,8 @@ protected static string GetSatelliteFileName(string assemblyFileName) => /// internal string GetRealAnalyzerLoadPath(string originalFullPath) { + CheckIfDisposed(); + lock (_guard) { if (!_analyzerAssemblyInfoMap.TryGetValue(originalFullPath, out var tuple)) @@ -340,6 +383,8 @@ internal string GetRealAnalyzerLoadPath(string originalFullPath) internal (string OriginalAssemblyPath, string RealAssemblyPath)[] GetPathMapSnapshot() { + CheckIfDisposed(); + lock (_guard) { return _analyzerAssemblyInfoMap @@ -357,6 +402,8 @@ internal string GetRealAnalyzerLoadPath(string originalFullPath) /// An if one of the resolvers is successful, or internal Assembly? ResolveAssemblyExternally(AssemblyName assemblyName) { + CheckIfDisposed(); + if (!_externalResolvers.IsDefaultOrEmpty) { foreach (var resolver in _externalResolvers) diff --git a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs index 492df67ae2597..a620bf64f1de7 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs @@ -147,11 +147,6 @@ public static bool ReportAndPropagateUnlessCanceled(Exception exception, Cancell return ReportAndPropagate(exception, severity); } - // Since the command line compiler has no way to catch exceptions, report them, and march on, we - // simply don't offer such a mechanism here to avoid accidental swallowing of exceptions. - -#if !COMPILERCORE - /// /// Report an error. /// Calls and doesn't pass the exception through (the method returns true). @@ -169,6 +164,11 @@ public static bool ReportAndCatch(Exception exception, ErrorSeverity severity = return true; } + // Since the command line compiler has no way to catch exceptions, report them, and march on, we + // simply don't offer such a mechanism here to avoid accidental swallowing of exceptions. + +#if !COMPILERCORE + [DebuggerHidden] public static bool ReportWithDumpAndCatch(Exception exception, ErrorSeverity severity = ErrorSeverity.Uncategorized) { diff --git a/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs b/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs index 8bb31b511dfb6..4c381c8dc8d4d 100644 --- a/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs +++ b/src/Compilers/Server/VBCSCompilerTests/AnalyzerConsistencyCheckerTests.cs @@ -251,5 +251,6 @@ public void AddDependencyLocation(string fullPath) { } public bool IsHostAssembly(Assembly assembly) => false; public Assembly LoadFromPath(string fullPath) => throw new Exception(); public string? GetOriginalDependencyLocation(AssemblyName assembly) => throw new Exception(); + public void Dispose() { } } } diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/VSCodeAnalyzerLoader.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/VSCodeAnalyzerLoader.cs index 60762f3fa40bb..e60f91cd232e1 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/VSCodeAnalyzerLoader.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/VSCodeAnalyzerLoader.cs @@ -24,7 +24,7 @@ internal sealed class VSCodeAnalyzerLoaderProvider( private readonly ExtensionAssemblyManager _extensionAssemblyManager = extensionAssemblyManager; private readonly ILoggerFactory _loggerFactory = loggerFactory; - protected override IAnalyzerAssemblyLoader CreateShadowCopyLoader(ImmutableArray externalResolvers) + protected override IAnalyzerAssemblyLoaderInternal CreateShadowCopyLoader(ImmutableArray externalResolvers) { var baseLoader = base.CreateShadowCopyLoader(externalResolvers); return new VSCodeExtensionAssemblyAnalyzerLoader(baseLoader, _extensionAssemblyManager, _loggerFactory.CreateLogger()); @@ -33,12 +33,19 @@ protected override IAnalyzerAssemblyLoader CreateShadowCopyLoader(ImmutableArray /// /// Analyzer loader that will re-use already loaded assemblies from the extension load context. /// - private class VSCodeExtensionAssemblyAnalyzerLoader(IAnalyzerAssemblyLoader defaultLoader, ExtensionAssemblyManager extensionAssemblyManager, ILogger logger) : IAnalyzerAssemblyLoader + private sealed class VSCodeExtensionAssemblyAnalyzerLoader( + IAnalyzerAssemblyLoaderInternal defaultLoader, + ExtensionAssemblyManager extensionAssemblyManager, + ILogger logger) : IAnalyzerAssemblyLoaderInternal { public void AddDependencyLocation(string fullPath) - { - defaultLoader.AddDependencyLocation(fullPath); - } + => defaultLoader.AddDependencyLocation(fullPath); + + public string? GetOriginalDependencyLocation(AssemblyName assembly) + => defaultLoader.GetOriginalDependencyLocation(assembly); + + public bool IsHostAssembly(Assembly assembly) + => defaultLoader.IsHostAssembly(assembly); public Assembly LoadFromPath(string fullPath) { @@ -51,5 +58,8 @@ public Assembly LoadFromPath(string fullPath) return defaultLoader.LoadFromPath(fullPath); } + + public void Dispose() + => defaultLoader.Dispose(); } } diff --git a/src/LanguageServer/Protocol/Features/AbstractAnalyzerAssemblyLoaderProvider.cs b/src/LanguageServer/Protocol/Features/AbstractAnalyzerAssemblyLoaderProvider.cs index de91680443643..08304bce126c9 100644 --- a/src/LanguageServer/Protocol/Features/AbstractAnalyzerAssemblyLoaderProvider.cs +++ b/src/LanguageServer/Protocol/Features/AbstractAnalyzerAssemblyLoaderProvider.cs @@ -14,21 +14,22 @@ namespace Microsoft.CodeAnalysis.Host; /// internal abstract class AbstractAnalyzerAssemblyLoaderProvider : IAnalyzerAssemblyLoaderProvider { - private readonly Lazy _shadowCopyLoader; + private readonly Lazy _shadowCopyLoader; public AbstractAnalyzerAssemblyLoaderProvider(ImmutableArray externalResolvers) { // We use a lazy here in case creating the loader requires MEF imports in the derived constructor. - _shadowCopyLoader = new Lazy(() => CreateShadowCopyLoader(externalResolvers)); + _shadowCopyLoader = new(() => CreateShadowCopyLoader(externalResolvers)); } - public IAnalyzerAssemblyLoader GetShadowCopyLoader() + public IAnalyzerAssemblyLoaderInternal GetShadowCopyLoader() => _shadowCopyLoader.Value; - protected virtual IAnalyzerAssemblyLoader CreateShadowCopyLoader(ImmutableArray externalResolvers) - { - return DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader( - Path.Combine(Path.GetTempPath(), "VS", "AnalyzerAssemblyLoader"), + protected virtual IAnalyzerAssemblyLoaderInternal CreateShadowCopyLoader(ImmutableArray externalResolvers) + => DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader( + GetDefaultShadowCopyPath(), externalResolvers: externalResolvers); - } + + public static string GetDefaultShadowCopyPath() + => Path.Combine(Path.GetTempPath(), "VS", "AnalyzerAssemblyLoader"); } diff --git a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs index 1c640695c962f..113cbccf17b96 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs @@ -30,11 +30,11 @@ private sealed class DefaultAnalyzerAssemblyLoaderProvider(string workspaceKind, /// locking it. The exception is fine, since the cleanup is just hygienic and isn't intended to be needed for /// correctness. But it is annoying and does cause noise in our perf test harness. /// - private readonly IAnalyzerAssemblyLoader _shadowCopyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader( + private readonly IAnalyzerAssemblyLoaderInternal _shadowCopyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader( Path.Combine(Path.GetTempPath(), "CodeAnalysis", "WorkspacesAnalyzerShadowCopies", workspaceKind), externalResolvers: externalResolvers); - public IAnalyzerAssemblyLoader GetShadowCopyLoader() + public IAnalyzerAssemblyLoaderInternal GetShadowCopyLoader() => _shadowCopyLoader; } } diff --git a/src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs b/src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs index 926b557f6b6ee..edbc8e806c9b0 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/Metadata/IAnalyzerAssemblyLoaderProvider.cs @@ -6,5 +6,5 @@ namespace Microsoft.CodeAnalysis.Host; internal interface IAnalyzerAssemblyLoaderProvider : IWorkspaceService { - IAnalyzerAssemblyLoader GetShadowCopyLoader(); + IAnalyzerAssemblyLoaderInternal GetShadowCopyLoader(); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs index 8374a7d9a3081..b3f7d4f917d5e 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs @@ -23,8 +23,8 @@ internal sealed class RemoteAnalyzerAssemblyLoaderService( : IAnalyzerAssemblyLoaderProvider { private readonly ShadowCopyAnalyzerAssemblyLoader _shadowCopyLoader = - new(Path.Combine(Path.GetTempPath(), "VS", "AnalyzerAssemblyLoader"), externalResolvers.ToImmutableArray()); + new(AbstractAnalyzerAssemblyLoaderProvider.GetDefaultShadowCopyPath(), externalResolvers.ToImmutableArray()); - public IAnalyzerAssemblyLoader GetShadowCopyLoader() + public IAnalyzerAssemblyLoaderInternal GetShadowCopyLoader() => _shadowCopyLoader; }