From 9760b22acab79af8e74be0a8f2f5aa359bdd2c3e Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 4 Aug 2023 16:39:16 -0700 Subject: [PATCH 1/6] Fix analyzer conflicts on Linux This changes analyzer loading on Linux to be per user where before clashes could occur in the temp directory. This fix initially started out by making our temp directory logic more robust. Essentially add per user sub directories, do extended permission checks, etc ... That exacerbated a number of existing corner cases in the code and forced us to put a lot more error handling into the workspaces layer. After discussion with the IDE team I decided to take the approach of loading from a `Stream` on non-Windows platforms. The performance difference is neglible (1ms to 2ms per assembly) and it removes the overhead of copying them around on disk as well as the complexities that come with managing the shadow copy directory. This will be observable to any analyzer that used `Assembly.Location` as it will now be `""`. That is only useful though for inspecting disks which is already illegal for analyzers hence it's considered an acceptable break. closes #65415 --- .../AnalyzerAssemblyLoaderTests.cs | 183 ++++++++++-------- .../Core/CodeAnalysisTest/InvokeUtil.cs | 24 ++- .../AnalyzerAssemblyLoader.Core.cs | 48 ++++- .../DefaultAnalyzerAssemblyLoader.cs | 50 ++++- .../ShadowCopyAnalyzerAssemblyLoader.cs | 23 +-- .../VBCSCompiler/CompilerRequestHandler.cs | 2 +- .../OmnisharpAnalyzerLoaderFactory.cs | 3 +- .../DefaultAnalyzerAssemblyLoaderService.cs | 4 +- .../Host/RemoteAnalyzerAssemblyLoader.cs | 1 + 9 files changed, 224 insertions(+), 114 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs b/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs index 4834a205d698e..11b5eb2688a0a 100644 --- a/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/AnalyzerAssemblyLoaderTests.cs @@ -30,6 +30,15 @@ namespace Microsoft.CodeAnalysis.UnitTests { + public enum AnalyzerTestKind + { + LoadDirect, + ShadowLoad, +#if NETCOREAPP + LoadStream, +#endif + } + /// /// Contains the bulk of our analyzer / generator loading tests. /// @@ -87,15 +96,15 @@ public AnalyzerAssemblyLoaderTests(ITestOutputHelper testOutputHelper, AssemblyL #if NETCOREAPP - private void Run(bool shadowLoad, Action testAction, [CallerMemberName] string? memberName = null) => + private void Run(AnalyzerTestKind kind, Action testAction, [CallerMemberName] string? memberName = null) => Run( - shadowLoad, + kind, static (_, _) => { }, testAction, memberName); private void Run( - bool shadowLoad, + AnalyzerTestKind kind, Action prepLoadContextAction, Action testAction, [CallerMemberName] string? memberName = null) @@ -105,7 +114,7 @@ private void Run( { prepLoadContextAction(alc, TestFixture); var util = new InvokeUtil(); - util.Exec(TestOutputHelper, alc, TestFixture, shadowLoad, testAction.Method.DeclaringType!.FullName!, testAction.Method.Name); + util.Exec(TestOutputHelper, alc, TestFixture, kind, testAction.Method.DeclaringType!.FullName!, testAction.Method.Name); } finally { @@ -116,7 +125,7 @@ private void Run( #else private void Run( - bool shadowLoad, + AnalyzerTestKind kind, Action testAction, [CallerMemberName] string? memberName = null) { @@ -127,7 +136,7 @@ private void Run( var testOutputHelper = new AppDomainTestOutputHelper(TestOutputHelper); var type = typeof(InvokeUtil); var util = (InvokeUtil)appDomain.CreateInstanceAndUnwrap(type.Assembly.FullName, type.FullName); - util.Exec(testOutputHelper, TestFixture, shadowLoad, testAction.Method.DeclaringType.FullName, testAction.Method.Name); + util.Exec(testOutputHelper, TestFixture, kind, testAction.Method.DeclaringType.FullName, testAction.Method.Name); } finally { @@ -159,9 +168,9 @@ internal static void InvokeTestCode(AnalyzerAssemblyLoader loader, AssemblyLoadT [Theory] [CombinatorialData] [WorkItem(32226, "https://github.com/dotnet/roslyn/issues/32226")] - public void LoadWithDependency(bool shadowLoad) + public void LoadWithDependency(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var analyzerDependencyFile = testFixture.AnalyzerDependency; var analyzerMainFile = testFixture.AnalyzerWithDependency; @@ -184,9 +193,9 @@ public void LoadWithDependency(bool shadowLoad) [Theory] [CombinatorialData] - public void AddDependencyLocationThrowsOnNull(bool shadowLoad) + public void AddDependencyLocationThrowsOnNull(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { Assert.Throws("fullPath", () => loader.AddDependencyLocation(null!)); Assert.Throws("fullPath", () => loader.AddDependencyLocation("a")); @@ -195,9 +204,9 @@ public void AddDependencyLocationThrowsOnNull(bool shadowLoad) [Theory] [CombinatorialData] - public void ThrowsForMissingFile(bool shadowLoad) + public void ThrowsForMissingFile(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".dll"); Assert.ThrowsAny(() => loader.LoadFromPath(path)); @@ -206,9 +215,9 @@ public void ThrowsForMissingFile(bool shadowLoad) [Theory] [CombinatorialData] - public void BasicLoad(bool shadowLoad) + public void BasicLoad(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { loader.AddDependencyLocation(testFixture.Alpha); Assembly alpha = loader.LoadFromPath(testFixture.Alpha); @@ -219,9 +228,9 @@ public void BasicLoad(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_Multiple(bool shadowLoad) + public void AssemblyLoading_Multiple(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -258,9 +267,9 @@ public void AssemblyLoading_Multiple(bool shadowLoad) /// [Theory] [CombinatorialData] - public void AssemblyLoading_OverwriteBeforeLoad(bool shadowLoad) + public void AssemblyLoading_OverwriteBeforeLoad(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -280,9 +289,9 @@ public void AssemblyLoading_OverwriteBeforeLoad(bool shadowLoad) [ConditionalTheory(typeof(WindowsOnly), Reason = "https://github.com/dotnet/roslyn/issues/66621")] [CombinatorialData] - public void AssemblyLoading_AssemblyLocationNotAdded(bool shadowLoad) + public void AssemblyLoading_AssemblyLocationNotAdded(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { loader.AddDependencyLocation(testFixture.Gamma); loader.AddDependencyLocation(testFixture.Delta1); @@ -292,9 +301,9 @@ public void AssemblyLoading_AssemblyLocationNotAdded(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_DependencyLocationNotAdded(bool shadowLoad) + public void AssemblyLoading_DependencyLocationNotAdded(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -332,12 +341,10 @@ private static void VerifyAssemblies(AnalyzerAssemblyLoader loader, IEnumerable< private static void VerifyAssemblies(AnalyzerAssemblyLoader loader, IEnumerable assemblies, int? expectedCopyCount, params (string simpleName, string version, string path)[] expected) { - expected = expected - .Select(x => (x.simpleName, x.version, loader.GetRealLoadPath(x.path))) - .ToArray(); - Assert.Equal( - expected, + expected + .Select(x => (x.simpleName, x.version, getExpectedLoadPath(x.path))) + .ToArray(), assemblies.Select(assembly => (assembly.GetName().Name!, assembly.GetName().Version!.ToString(), assembly.Location)) .OrderBy(static x => x) .ToArray()); @@ -347,6 +354,19 @@ private static void VerifyAssemblies(AnalyzerAssemblyLoader loader, IEnumerable< Assert.All(assemblies, x => x.Location.StartsWith(shadowLoader.BaseDirectory, StringComparison.Ordinal)); Assert.Equal(expectedCopyCount ?? expected.Length, shadowLoader.CopyCount); } + + string getExpectedLoadPath(string path) + { +#if NETCOREAPP + if (loader is AnalyzerAssemblyLoader { AnalyzerLoadOption: AnalyzerLoadOption.LoadFromStream }) + { + return ""; + } +#endif + + return loader.GetRealLoadPath(path ?? ""); + } + } private static void VerifyAssemblies(AnalyzerAssemblyLoader loader, IEnumerable assemblies, int? copyCount, params string[] assemblyPaths) @@ -417,9 +437,9 @@ static bool isInLoadFromContext(AnalyzerAssemblyLoader loader, Assembly assembly [Theory] [CombinatorialData] - public void AssemblyLoading_Simple(bool shadowLoad) + public void AssemblyLoading_Simple(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); StringBuilder sb = new StringBuilder(); @@ -445,9 +465,9 @@ public void AssemblyLoading_Simple(bool shadowLoad) [ConditionalTheory(typeof(WindowsOnly), Reason = "https://github.com/dotnet/runtime/issues/81108")] [CombinatorialData] - public void AssemblyLoading_DependencyInDifferentDirectory(bool shadowLoad) + public void AssemblyLoading_DependencyInDifferentDirectory(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -481,9 +501,9 @@ public void AssemblyLoading_DependencyInDifferentDirectory(bool shadowLoad) /// [Theory] [CombinatorialData] - public void AssemblyLoading_DependencyInDifferentDirectory2(bool shadowLoad) + public void AssemblyLoading_DependencyInDifferentDirectory2(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -536,9 +556,9 @@ public void AssemblyLoading_DependencyInDifferentDirectory2(bool shadowLoad) /// [Theory] [CombinatorialData] - public void AssemblyLoading_DependencyInDifferentDirectory3(bool shadowLoad) + public void AssemblyLoading_DependencyInDifferentDirectory3(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -569,9 +589,9 @@ public void AssemblyLoading_DependencyInDifferentDirectory3(bool shadowLoad) [ConditionalTheory(typeof(WindowsOnly), Reason = "https://github.com/dotnet/roslyn/issues/66626")] [CombinatorialData] [WorkItem(32226, "https://github.com/dotnet/roslyn/issues/32226")] - public void AssemblyLoading_DependencyInDifferentDirectory4(bool shadowLoad) + public void AssemblyLoading_DependencyInDifferentDirectory4(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var analyzerDependencyFile = testFixture.AnalyzerDependency; var analyzerMainFile = testFixture.AnalyzerWithDependency; @@ -599,9 +619,9 @@ public void AssemblyLoading_DependencyInDifferentDirectory4(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions(bool shadowLoad) + public void AssemblyLoading_MultipleVersions(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -656,9 +676,9 @@ public void AssemblyLoading_MultipleVersions(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_NoExactMatch(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_NoExactMatch(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -715,9 +735,9 @@ public void AssemblyLoading_MultipleVersions_NoExactMatch(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_MultipleEqualMatches(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_MultipleEqualMatches(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -765,9 +785,9 @@ public void AssemblyLoading_MultipleVersions_MultipleEqualMatches(bool shadowLoa [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_MultipleVersionsOfSameAnalyzerItself(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_MultipleVersionsOfSameAnalyzerItself(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -782,9 +802,12 @@ public void AssemblyLoading_MultipleVersions_MultipleVersionsOfSameAnalyzerItsel #if NETCOREAPP // On Core, we're able to load both of these into separate AssemblyLoadContexts. - Assert.NotEqual(delta2B.Location, delta2.Location); - Assert.Equal(loader.GetRealLoadPath(testFixture.Delta2), delta2.Location); - Assert.Equal(loader.GetRealLoadPath(testFixture.Delta2B), delta2B.Location); + if (loader.AnalyzerLoadOption == AnalyzerLoadOption.LoadFromDisk) + { + Assert.NotEqual(delta2B.Location, delta2.Location); + Assert.Equal(loader.GetRealLoadPath(testFixture.Delta2), delta2.Location); + Assert.Equal(loader.GetRealLoadPath(testFixture.Delta2B), delta2B.Location); + } #else @@ -799,9 +822,9 @@ public void AssemblyLoading_MultipleVersions_MultipleVersionsOfSameAnalyzerItsel [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_ExactAndGreaterMatch(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_ExactAndGreaterMatch(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -851,9 +874,9 @@ public void AssemblyLoading_MultipleVersions_ExactAndGreaterMatch(bool shadowLoa [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_WorseMatchInSameDirectory(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_WorseMatchInSameDirectory(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); StringBuilder sb = new StringBuilder(); @@ -910,9 +933,9 @@ public void AssemblyLoading_MultipleVersions_WorseMatchInSameDirectory(bool shad [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_MultipleLoaders(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_MultipleLoaders(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader1, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader1, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -973,9 +996,9 @@ public void AssemblyLoading_MultipleVersions_MultipleLoaders(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_MultipleVersions_MissingVersion(bool shadowLoad) + public void AssemblyLoading_MultipleVersions_MissingVersion(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -1002,9 +1025,9 @@ public void AssemblyLoading_MultipleVersions_MissingVersion(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_UnifyToHighest(bool shadowLoad) + public void AssemblyLoading_UnifyToHighest(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var sb = new StringBuilder(); @@ -1037,9 +1060,9 @@ public void AssemblyLoading_UnifyToHighest(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_CanLoadDifferentVersionsDirectly(bool shadowLoad) + public void AssemblyLoading_CanLoadDifferentVersionsDirectly(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var sb = new StringBuilder(); @@ -1067,9 +1090,9 @@ public void AssemblyLoading_CanLoadDifferentVersionsDirectly(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_01(bool shadowLoad) + public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_01(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -1094,9 +1117,9 @@ public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_01(bool [Theory] [CombinatorialData] - public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_02(bool shadowLoad) + public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_02(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { StringBuilder sb = new StringBuilder(); @@ -1112,9 +1135,9 @@ public void AssemblyLoading_AnalyzerReferencesSystemCollectionsImmutable_02(bool [Theory] [CombinatorialData] - public void AssemblyLoading_CompilerDependencyDuplicated(bool shadowLoad) + public void AssemblyLoading_CompilerDependencyDuplicated(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var assembly = typeof(ImmutableArray).Assembly; @@ -1133,9 +1156,9 @@ public void AssemblyLoading_CompilerDependencyDuplicated(bool shadowLoad) [ConditionalTheory(typeof(WindowsOnly))] [CombinatorialData] - public void AssemblyLoading_NativeDependency(bool shadowLoad) + public void AssemblyLoading_NativeDependency(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { const int INVALID_FILE_ATTRIBUTES = -1; loader.AddDependencyLocation(testFixture.AnalyzerWithNativeDependency); @@ -1150,9 +1173,9 @@ public void AssemblyLoading_NativeDependency(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_DeleteAfterLoad1(bool shadowLoad) + public void AssemblyLoading_DeleteAfterLoad1(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -1173,9 +1196,9 @@ public void AssemblyLoading_DeleteAfterLoad1(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_DeleteAfterLoad2(bool shadowLoad) + public void AssemblyLoading_DeleteAfterLoad2(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); StringBuilder sb = new StringBuilder(); @@ -1204,9 +1227,9 @@ public void AssemblyLoading_DeleteAfterLoad2(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_DeleteAfterLoad3(bool shadowLoad) + public void AssemblyLoading_DeleteAfterLoad3(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -1244,9 +1267,9 @@ public void AssemblyLoading_DeleteAfterLoad3(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_RepeatedLoads1(bool shadowLoad) + public void AssemblyLoading_RepeatedLoads1(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { var path = testFixture.Delta1; loader.AddDependencyLocation(path); @@ -1265,9 +1288,9 @@ public void AssemblyLoading_RepeatedLoads1(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoading_RepeatedLoads2(bool shadowLoad) + public void AssemblyLoading_RepeatedLoads2(AnalyzerTestKind kind) { - Run(shadowLoad, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => + Run(kind, static (AnalyzerAssemblyLoader loader, AssemblyLoadTestFixture testFixture) => { using var temp = new TempRoot(); var tempDir = temp.CreateDirectory(); @@ -1302,9 +1325,9 @@ public void AssemblyLoading_RepeatedLoads2(bool shadowLoad) [Theory] [CombinatorialData] - public void AssemblyLoadingInNonDefaultContext_AnalyzerReferencesSystemCollectionsImmutable(bool shadowLoad) + public void AssemblyLoadingInNonDefaultContext_AnalyzerReferencesSystemCollectionsImmutable(AnalyzerTestKind kind) { - Run(shadowLoad, + Run(kind, static (AssemblyLoadContext compilerContext, AssemblyLoadTestFixture testFixture) => { // Load the compiler assembly and a modified version of S.C.I into the compiler load context. We diff --git a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs index 3fdc9a58c0257..cf0684e8e0b38 100644 --- a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs +++ b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs @@ -35,7 +35,7 @@ namespace Microsoft.CodeAnalysis.UnitTests public sealed class InvokeUtil { - public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compilerContext, AssemblyLoadTestFixture fixture, bool shadowLoad, string typeName, string methodName) + public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compilerContext, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName) { // Ensure that the test did not load any of the test fixture assemblies into // the default load context. That should never happen. Assemblies should either @@ -46,9 +46,14 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compile var compilerContextCount = compilerContext.Assemblies.Count(); using var tempRoot = new TempRoot(); - AnalyzerAssemblyLoader loader = shadowLoad - ? new ShadowCopyAnalyzerAssemblyLoader(compilerContext, tempRoot.CreateDirectory().Path) - : new DefaultAnalyzerAssemblyLoader(compilerContext); + AnalyzerAssemblyLoader loader = kind switch + { + AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromDisk), + AnalyzerTestKind.LoadStream => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromStream), + AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(compilerContext, tempRoot.CreateDirectory().Path), + _ => throw new Exception() + }; + try { AnalyzerAssemblyLoaderTests.InvokeTestCode(loader, fixture, typeName, methodName); @@ -88,12 +93,15 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compile public sealed class InvokeUtil : MarshalByRefObject { - public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadTestFixture fixture, bool shadowLoad, string typeName, string methodName) + public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadTestFixture fixture, AnalyzerTestKind kind, string typeName, string methodName) { using var tempRoot = new TempRoot(); - AnalyzerAssemblyLoader loader = shadowLoad - ? new ShadowCopyAnalyzerAssemblyLoader(tempRoot.CreateDirectory().Path) - : new DefaultAnalyzerAssemblyLoader(); + AnalyzerAssemblyLoader loader = kind switch + { + AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(), + AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(tempRoot.CreateDirectory().Path), + _ => throw new Exception() + }; try { diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs index ef4bb64ea4193..b7cca9aefcede 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -11,19 +11,46 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; using System.Runtime.Loader; namespace Microsoft.CodeAnalysis { + internal enum AnalyzerLoadOption + { + /// + /// Once the assembly path is chosen load it directly from disk at that location + /// + LoadFromDisk, + + /// + /// Once the assembly path is chosen, read the contents of disk and load from memory + /// + /// + /// While Windows supports this option it comes with a significant performance penalty do + /// to anti virus scans. It can have a load time of 300-500ms while loading from disk + /// is generally 1-2ms. Use this with caution on Windows. + /// + LoadFromStream + } + internal partial class AnalyzerAssemblyLoader { private readonly AssemblyLoadContext _compilerLoadContext; private readonly Dictionary _loadContextByDirectory = new Dictionary(StringComparer.Ordinal); + private readonly AnalyzerLoadOption _loadOption; internal AssemblyLoadContext CompilerLoadContext => _compilerLoadContext; + internal AnalyzerLoadOption AnalyzerLoadOption => _loadOption; - internal AnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext = null) + internal AnalyzerAssemblyLoader() + : this(null, AnalyzerLoadOption.LoadFromDisk) { + } + + internal AnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, AnalyzerLoadOption loadOption) + { + _loadOption = loadOption; _compilerLoadContext = compilerLoadContext ?? AssemblyLoadContext.GetLoadContext(typeof(AnalyzerAssemblyLoader).GetTypeInfo().Assembly)!; } @@ -105,7 +132,7 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass if (_loader.IsAnalyzerDependencyPath(assemblyPath)) { (_, var loadPath) = _loader.GetAssemblyInfoForPath(assemblyPath); - return LoadFromAssemblyPath(loadPath); + return loadCore(loadPath); } // Next prefer registered dependencies from other directories. Ideally this would not @@ -114,11 +141,24 @@ public DirectoryLoadContext(string directory, AnalyzerAssemblyLoader loader, Ass // https://github.com/dotnet/roslyn/issues/56442 if (_loader.GetBestPath(assemblyName) is string bestRealPath) { - return LoadFromAssemblyPath(bestRealPath); + return loadCore(bestRealPath); } // No analyzer registered this dependency. Time to fail return null; + + Assembly loadCore(string assemblyPath) + { + if (_loader.AnalyzerLoadOption == AnalyzerLoadOption.LoadFromDisk) + { + return LoadFromAssemblyPath(assemblyPath); + } + else + { + using var stream = File.Open(assemblyPath, FileMode.Open, FileAccess.Read, FileShare.Read); + return LoadFromStream(stream); + } + } } protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs index f059cf6dda665..664c9df7a48f4 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs @@ -9,6 +9,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis @@ -17,14 +18,19 @@ internal sealed class DefaultAnalyzerAssemblyLoader : AnalyzerAssemblyLoader { #if NETCOREAPP - // Called from a netstandard2.0 project, so need to ensure a parameterless constructor is available. internal DefaultAnalyzerAssemblyLoader() : this(null) { } - internal DefaultAnalyzerAssemblyLoader(System.Runtime.Loader.AssemblyLoadContext? compilerLoadContext = null) - : base(compilerLoadContext) + internal DefaultAnalyzerAssemblyLoader(System.Runtime.Loader.AssemblyLoadContext? compilerLoadContext = null, AnalyzerLoadOption loadOption = AnalyzerLoadOption.LoadFromDisk) + : base(compilerLoadContext, loadOption) + { + } + +#else + + internal DefaultAnalyzerAssemblyLoader() { } @@ -36,5 +42,43 @@ internal DefaultAnalyzerAssemblyLoader(System.Runtime.Loader.AssemblyLoadContext /// /// protected override string PreparePathToLoad(string fullPath) => fullPath; + + /// + /// Return an which does not lock assemblies on disk that is + /// most appropriate for the current platform. + /// + /// In the case a directory must be created on disk for shadow loading this + /// is the suffix added to that path + /// + internal static IAnalyzerAssemblyLoader CreateNonLockingLoader(string? subPath = null) + { +#if NETCOREAPP + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // The cost of doing stream based loading on Windows is too expensive and we must continue to + // use the shadow copy loader. + return createShadowLoaderWindows(); + } + else + { + return new DefaultAnalyzerAssemblyLoader(loadOption: AnalyzerLoadOption.LoadFromStream); + } +#else + return createShadowLoaderWindows(); +#endif + + ShadowCopyAnalyzerAssemblyLoader createShadowLoaderWindows() + { + // The shadow copy analyzer should only be created on Windows. To create on Linux we cannot use + // GetTempPath as it's not per-user. Generally there is no need as LoadFromStream achieves the same + // effect + Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + subPath ??= Path.Combine("CodeAnalysis", "AnalyzerShadowCopies"); + var baseDirectory = Path.IsPathRooted(subPath) + ? subPath + : Path.Combine(Path.GetTempPath(), subPath); + return new ShadowCopyAnalyzerAssemblyLoader(baseDirectory); + } + } } } diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs index b05ff2b4b2aa5..39da507093ac7 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs @@ -44,30 +44,23 @@ internal sealed class ShadowCopyAnalyzerAssemblyLoader : AnalyzerAssemblyLoader internal int CopyCount => _assemblyDirectoryId; #if NETCOREAPP - public ShadowCopyAnalyzerAssemblyLoader(string? baseDirectory = null) + public ShadowCopyAnalyzerAssemblyLoader(string baseDirectory) : this(null, baseDirectory) { } - public ShadowCopyAnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, string? baseDirectory = null) - : base(compilerLoadContext) + public ShadowCopyAnalyzerAssemblyLoader(AssemblyLoadContext? compilerLoadContext, string baseDirectory) + : base(compilerLoadContext, AnalyzerLoadOption.LoadFromDisk) #else - public ShadowCopyAnalyzerAssemblyLoader(string? baseDirectory = null) + public ShadowCopyAnalyzerAssemblyLoader(string baseDirectory) #endif { - if (baseDirectory != null) + if (baseDirectory is null) { - _baseDirectory = baseDirectory; - } - else - { - // https://github.com/dotnet/roslyn/issues/65415 - // Fixing that issue will involve removing this GetTempPath call -#pragma warning disable RS0030 - _baseDirectory = Path.Combine(Path.GetTempPath(), "CodeAnalysis", "AnalyzerShadowCopies"); -#pragma warning restore RS0030 + throw new ArgumentNullException(nameof(baseDirectory)); } + _baseDirectory = baseDirectory; _shadowCopyDirectoryAndMutex = new Lazy<(string directory, Mutex)>( () => CreateUniqueDirectoryForProcess(), LazyThreadSafetyMode.ExecutionAndPublication); @@ -87,7 +80,7 @@ private void DeleteLeftoverDirectories() } catch (DirectoryNotFoundException) { - return; + return; } foreach (var subDirectory in subDirectories) diff --git a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs index d5da3d8306c6e..2e7c6b1bfa919 100644 --- a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs +++ b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs @@ -41,7 +41,7 @@ public RunRequest(Guid requestId, string language, string? workingDirectory, str internal sealed class CompilerServerHost : ICompilerServerHost { - public IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; } = new ShadowCopyAnalyzerAssemblyLoader(baseDirectory: Path.Combine(Path.GetTempPath(), "VBCSCompiler", "AnalyzerAssemblyLoader")); + public IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; } = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(Path.Combine("VBCSCompiler", "AnalyzerAssemblyLoader")); public static Func SharedAssemblyReferenceProvider { get; } = (path, properties) => new CachingMetadataReference(path, properties); diff --git a/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs b/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs index 899a27f9768cb..5b9aa9a66927b 100644 --- a/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs +++ b/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using Microsoft.CodeAnalysis.Diagnostics; +using System.IO; namespace Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Analyzers { @@ -10,7 +11,7 @@ internal static class OmnisharpAnalyzerAssemblyLoaderFactory { public static IAnalyzerAssemblyLoader CreateShadowCopyAnalyzerAssemblyLoader(string? baseDirectory = null) { - return new ShadowCopyAnalyzerAssemblyLoader(baseDirectory); + return DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(baseDirectory); } } } diff --git a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs index b6580eae4708f..4390d0174bb78 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs @@ -12,8 +12,8 @@ namespace Microsoft.CodeAnalysis.Host [ExportWorkspaceService(typeof(IAnalyzerAssemblyLoaderProvider))] internal sealed class DefaultAnalyzerAssemblyLoaderService : IAnalyzerAssemblyLoaderProvider { - private readonly DefaultAnalyzerAssemblyLoader _loader = new(); - private readonly ShadowCopyAnalyzerAssemblyLoader _shadowCopyLoader = new(); + private readonly IAnalyzerAssemblyLoader _loader = new DefaultAnalyzerAssemblyLoader(); + private readonly IAnalyzerAssemblyLoader _shadowCopyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(); [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs index 1fc17bbbefd05..3046e9f3fa886 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoader.cs @@ -18,6 +18,7 @@ internal sealed class RemoteAnalyzerAssemblyLoader : AnalyzerAssemblyLoader public RemoteAnalyzerAssemblyLoader(string baseDirectory) { + // jason should we load from stream here on Linux or is this VS only? _baseDirectory = baseDirectory; } From 1249be83c94420425fba6f2f3601242ef08cf761 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 7 Aug 2023 09:47:21 -0700 Subject: [PATCH 2/6] more --- .../DefaultAnalyzerAssemblyLoader.cs | 19 +++++++++---------- .../ShadowCopyAnalyzerAssemblyLoader.cs | 2 +- .../VBCSCompiler/CompilerRequestHandler.cs | 3 ++- .../OmnisharpAnalyzerLoaderFactory.cs | 1 + .../DefaultAnalyzerAssemblyLoaderService.cs | 4 +++- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs index 664c9df7a48f4..eb9f5d1650ecb 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs @@ -47,10 +47,9 @@ internal DefaultAnalyzerAssemblyLoader() /// Return an which does not lock assemblies on disk that is /// most appropriate for the current platform. /// - /// In the case a directory must be created on disk for shadow loading this - /// is the suffix added to that path - /// - internal static IAnalyzerAssemblyLoader CreateNonLockingLoader(string? subPath = null) + /// A shadow copy path will be created on Windows and this value + /// will be the base directory where shadow copy assemblies are stored. + internal static IAnalyzerAssemblyLoader CreateNonLockingLoader(string? windowsShadowPath = null) { #if NETCOREAPP if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -72,12 +71,12 @@ ShadowCopyAnalyzerAssemblyLoader createShadowLoaderWindows() // The shadow copy analyzer should only be created on Windows. To create on Linux we cannot use // GetTempPath as it's not per-user. Generally there is no need as LoadFromStream achieves the same // effect - Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); - subPath ??= Path.Combine("CodeAnalysis", "AnalyzerShadowCopies"); - var baseDirectory = Path.IsPathRooted(subPath) - ? subPath - : Path.Combine(Path.GetTempPath(), subPath); - return new ShadowCopyAnalyzerAssemblyLoader(baseDirectory); + if (string.IsNullOrEmpty(windowsShadowPath) || !Path.IsPathRooted(windowsShadowPath)) + { + throw new ArgumentException("Must be a full path.", nameof(windowsShadowPath)); + } + + return new ShadowCopyAnalyzerAssemblyLoader(windowsShadowPath); } } } diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs index 39da507093ac7..eae0e67a96a6b 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/ShadowCopyAnalyzerAssemblyLoader.cs @@ -80,7 +80,7 @@ private void DeleteLeftoverDirectories() } catch (DirectoryNotFoundException) { - return; + return; } foreach (var subDirectory in subDirectories) diff --git a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs index 2e7c6b1bfa919..7c15c6697578f 100644 --- a/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs +++ b/src/Compilers/Server/VBCSCompiler/CompilerRequestHandler.cs @@ -41,7 +41,7 @@ public RunRequest(Guid requestId, string language, string? workingDirectory, str internal sealed class CompilerServerHost : ICompilerServerHost { - public IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; } = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(Path.Combine("VBCSCompiler", "AnalyzerAssemblyLoader")); + public IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; } public static Func SharedAssemblyReferenceProvider { get; } = (path, properties) => new CachingMetadataReference(path, properties); @@ -72,6 +72,7 @@ internal CompilerServerHost(string clientDirectory, string? sdkDirectory, ICompi ClientDirectory = clientDirectory; SdkDirectory = sdkDirectory; Logger = logger; + AnalyzerAssemblyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(Path.Combine(Path.GetTempPath(), "VBCSCompiler", "AnalyzerAssemblyLoader")); } public bool TryCreateCompiler(in RunRequest request, BuildPaths buildPaths, [NotNullWhen(true)] out CommonCompiler? compiler) diff --git a/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs b/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs index 5b9aa9a66927b..e876349f28962 100644 --- a/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs +++ b/src/Tools/ExternalAccess/OmniSharp/Analyzers/OmnisharpAnalyzerLoaderFactory.cs @@ -11,6 +11,7 @@ internal static class OmnisharpAnalyzerAssemblyLoaderFactory { public static IAnalyzerAssemblyLoader CreateShadowCopyAnalyzerAssemblyLoader(string? baseDirectory = null) { + baseDirectory ??= Path.Combine(Path.GetTempPath(), "CodeAnalysis", "OmnisharpAnalyzerShadowCopies"); return DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(baseDirectory); } } diff --git a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs index 4390d0174bb78..75f287f223404 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/DefaultAnalyzerAssemblyLoaderService.cs @@ -4,6 +4,7 @@ using System; using System.Composition; +using System.IO; using Microsoft.CodeAnalysis.Host.Mef; namespace Microsoft.CodeAnalysis.Host @@ -13,7 +14,8 @@ namespace Microsoft.CodeAnalysis.Host internal sealed class DefaultAnalyzerAssemblyLoaderService : IAnalyzerAssemblyLoaderProvider { private readonly IAnalyzerAssemblyLoader _loader = new DefaultAnalyzerAssemblyLoader(); - private readonly IAnalyzerAssemblyLoader _shadowCopyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader(); + private readonly IAnalyzerAssemblyLoader _shadowCopyLoader = DefaultAnalyzerAssemblyLoader.CreateNonLockingLoader( + Path.Combine(Path.GetTempPath(), "CodeAnalysis", "WorkspacesAnalyzerShadowCopies")); [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] From 30920a0b9da7b712eedb508187fa6dbeec774889 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 7 Aug 2023 15:54:45 -0700 Subject: [PATCH 3/6] PR feedback --- src/Compilers/Shared/BuildServerConnection.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Compilers/Shared/BuildServerConnection.cs b/src/Compilers/Shared/BuildServerConnection.cs index 990f2a11986ca..8e2c993d3e6a7 100644 --- a/src/Compilers/Shared/BuildServerConnection.cs +++ b/src/Compilers/Shared/BuildServerConnection.cs @@ -34,16 +34,6 @@ internal sealed class BuildServerConnection /// /// Create a build request for processing on the server. /// - /// - /// Even though compilation itself does not specifically require a temporary directory to be successful - /// this API deliberately requires to have a value. The reason for this is - /// that the server itself requires a temporary directory to startup and it uses the same logic as the client - /// to calculate it. - /// - /// That means if the client can't calculate the temporary directory the server won't be able to either and - /// hence won't be able to start. So creating instances isn't useful cause it's - /// wasting time trying to start a server that won't be able to start. - /// internal static BuildRequest CreateBuildRequest( Guid requestId, RequestLanguage language, From 02652a88605ce43ffdc473069b941422af3e919f Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 8 Aug 2023 09:03:08 -0700 Subject: [PATCH 4/6] PR feedback --- docs/Breaking API Changes.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/Breaking API Changes.md b/docs/Breaking API Changes.md index 4b2782b0c79f3..150b4269e55af 100644 --- a/docs/Breaking API Changes.md +++ b/docs/Breaking API Changes.md @@ -87,4 +87,12 @@ All `SymbolDisplayFormat`s (predefined and user-created) now include parameter n `IncrementalGeneratorRunStep.Outputs` previously contained `IncrementalStepRunReason.Modified` as `Reason` when the input to the step was modified in a way that produced a new output. -Now the reason will be reported more accurately as `IncrementalStepRunReason.New`. \ No newline at end of file +Now the reason will be reported more accurately as `IncrementalStepRunReason.New`. + +# Version 4.8.0 + +### Changed `Assembly.Location` behavior in non-Windows + +The value of `Assembly.Location` previously held the location on disk where an analyzer or source generator was loaded from. This could be either the original location or the shadow copy location. In 4.8 this will be `""` in certain cases when running on non Windows platforms. This is due the compiler server loading assemblies using `AssemblyLoadContext.LoadFromStream` instead of loading from disk. + +This could already happen in other load scenarios but this change moves it into mainline build scenarios. From 8ad24517f91c6ba571e0712f83c75396184cfd64 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 8 Aug 2023 14:38:14 -0700 Subject: [PATCH 5/6] PR feedback --- src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs | 4 ++-- .../DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs | 4 ++-- .../DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs index cf0684e8e0b38..3dc9fec60ed45 100644 --- a/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs +++ b/src/Compilers/Core/CodeAnalysisTest/InvokeUtil.cs @@ -51,7 +51,7 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadContext compile AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromDisk), AnalyzerTestKind.LoadStream => new DefaultAnalyzerAssemblyLoader(compilerContext, AnalyzerLoadOption.LoadFromStream), AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(compilerContext, tempRoot.CreateDirectory().Path), - _ => throw new Exception() + _ => throw ExceptionUtilities.Unreachable() }; try @@ -100,7 +100,7 @@ public void Exec(ITestOutputHelper testOutputHelper, AssemblyLoadTestFixture fix { AnalyzerTestKind.LoadDirect => new DefaultAnalyzerAssemblyLoader(), AnalyzerTestKind.ShadowLoad => new ShadowCopyAnalyzerAssemblyLoader(tempRoot.CreateDirectory().Path), - _ => throw new Exception() + _ => throw ExceptionUtilities.Unreachable() }; try diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs index b7cca9aefcede..d5fd00ce05024 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerAssemblyLoader.Core.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis internal enum AnalyzerLoadOption { /// - /// Once the assembly path is chosen load it directly from disk at that location + /// Once the assembly path is chosen, load it directly from disk at that location /// LoadFromDisk, @@ -27,7 +27,7 @@ internal enum AnalyzerLoadOption /// Once the assembly path is chosen, read the contents of disk and load from memory /// /// - /// While Windows supports this option it comes with a significant performance penalty do + /// While Windows supports this option it comes with a significant performance penalty due /// to anti virus scans. It can have a load time of 300-500ms while loading from disk /// is generally 1-2ms. Use this with caution on Windows. /// diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs index eb9f5d1650ecb..061e6f4bd95a8 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs @@ -19,7 +19,6 @@ internal sealed class DefaultAnalyzerAssemblyLoader : AnalyzerAssemblyLoader #if NETCOREAPP internal DefaultAnalyzerAssemblyLoader() - : this(null) { } From 7cff17297960de1e0ac70ebdd2039ed501607c03 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 8 Aug 2023 15:35:23 -0700 Subject: [PATCH 6/6] PR feedback --- .../DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs index 061e6f4bd95a8..7e3a95ce87050 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.cs @@ -48,7 +48,7 @@ internal DefaultAnalyzerAssemblyLoader() /// /// A shadow copy path will be created on Windows and this value /// will be the base directory where shadow copy assemblies are stored. - internal static IAnalyzerAssemblyLoader CreateNonLockingLoader(string? windowsShadowPath = null) + internal static IAnalyzerAssemblyLoader CreateNonLockingLoader(string windowsShadowPath) { #if NETCOREAPP if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -70,7 +70,7 @@ ShadowCopyAnalyzerAssemblyLoader createShadowLoaderWindows() // The shadow copy analyzer should only be created on Windows. To create on Linux we cannot use // GetTempPath as it's not per-user. Generally there is no need as LoadFromStream achieves the same // effect - if (string.IsNullOrEmpty(windowsShadowPath) || !Path.IsPathRooted(windowsShadowPath)) + if (!Path.IsPathRooted(windowsShadowPath)) { throw new ArgumentException("Must be a full path.", nameof(windowsShadowPath)); }