From d19805520d6f14a7f09149600a5fad688e10faa7 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 09:39:54 -0800 Subject: [PATCH 1/8] Add syncronization to razor assembly loader --- .../Razor/RazorAnalyzerAssemblyResolver.cs | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index 85c36e8c54a15..becf9f77a9e20 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -3,49 +3,53 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Composition; using System.Diagnostics; using System.Reflection; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.VisualStudio.Composition; namespace Microsoft.CodeAnalysis.ExternalAccess.Razor { [Export(typeof(IAnalyzerAssemblyResolver)), Shared] - internal class RazorAnalyzerAssemblyResolver : IAnalyzerAssemblyResolver + [method: ImportingConstructor] + [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + internal class RazorAnalyzerAssemblyResolver() : IAnalyzerAssemblyResolver { private static Func? s_assemblyResolver; - /// - /// We use this as a heuristic to catch a case where we set the resolver too - /// late and the resolver has already been asked to resolve a razor assembly. - /// - /// Note this isn't perfectly accurate but is only used to trigger an assert - /// in debug builds. - /// - private static bool s_razorRequested = false; + private static readonly List? s_assembliesRequested = []; - [ImportingConstructor] - [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public RazorAnalyzerAssemblyResolver() - { - } + private static readonly object s_resolverLock = new(); - internal static Func? AssemblyResolver + /// + /// Attempts to set the assembly resolver. Will only succeed if the has not already been requested to load. + /// + /// The resolver function to set. + /// The name of an assembly that is checked to see if it has been requested to load. Setting the resolver will fail if this has already been requested. + /// true when the resolver was successfully set. + internal static bool TrySetAssemblyResolver(Func resolver, AssemblyName canaryAssembly) { - get => s_assemblyResolver; - set + lock (s_resolverLock) { - Debug.Assert(s_assemblyResolver == null, "Assembly resolver should not be set multiple times."); - Debug.Assert(!s_razorRequested, "A razor assembly has already been requested before setting the resolver."); - s_assemblyResolver = value; + if (s_assemblyResolver is not null && s_assembliesRequested is not null && !s_assembliesRequested.Contains(canaryAssembly)) + { + s_assemblyResolver = resolver; + return true; + } + return false; } } public Assembly? ResolveAssembly(AssemblyName assemblyName) { - s_razorRequested |= assemblyName.FullName.Contains("Razor"); - return s_assemblyResolver?.Invoke(assemblyName); + lock (s_resolverLock) + { + s_assembliesRequested?.Add(assemblyName); + return s_assemblyResolver?.Invoke(assemblyName); + } } } } From e387c609b0d156eeddf708c1bbad5ee0b9d516d6 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 09:40:18 -0800 Subject: [PATCH 2/8] Add a method to get generator specific documents --- src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs b/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs index 7603f875f31e9..9a3dff5dd42c8 100644 --- a/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs +++ b/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs @@ -17,5 +17,14 @@ internal static class RazorProjectExtensions { return project.GetSourceGeneratorRunResultAsync(cancellationToken); } + + internal static async ValueTask> GetSourceGeneratedDocumentsForGeneratorAsync(this Project project, string assemblyName, string assemblyPath, Version assemblyVersion, string typeName, CancellationToken cancellationToken) + { + var results = await project.GetSourceGeneratedDocumentsAsync(cancellationToken).ConfigureAwait(false); + + var generatorIdentity = new SourceGeneratorIdentity(assemblyName, assemblyPath, assemblyVersion, typeName); + return results.Where(s => s.Identity.Generator == generatorIdentity); + + } } } From c544a78fc6b3fdf2766998ce43996fd32f2817a4 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 17:08:49 -0800 Subject: [PATCH 3/8] Cleanup nullability --- .../ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index becf9f77a9e20..27dbb3040e312 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -20,7 +20,7 @@ internal class RazorAnalyzerAssemblyResolver() : IAnalyzerAssemblyResolver { private static Func? s_assemblyResolver; - private static readonly List? s_assembliesRequested = []; + private static readonly List s_assembliesRequested = []; private static readonly object s_resolverLock = new(); @@ -34,7 +34,7 @@ internal static bool TrySetAssemblyResolver(Func resolv { lock (s_resolverLock) { - if (s_assemblyResolver is not null && s_assembliesRequested is not null && !s_assembliesRequested.Contains(canaryAssembly)) + if (s_assemblyResolver is not null && !s_assembliesRequested.Contains(canaryAssembly)) { s_assemblyResolver = resolver; return true; @@ -47,7 +47,7 @@ internal static bool TrySetAssemblyResolver(Func resolv { lock (s_resolverLock) { - s_assembliesRequested?.Add(assemblyName); + s_assembliesRequested.Add(assemblyName); return s_assemblyResolver?.Invoke(assemblyName); } } From bb9057278cdf4005907260d985a7a017721ddc71 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 17:18:19 -0800 Subject: [PATCH 4/8] Invoke resolver after lock --- .../ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index 27dbb3040e312..a490d451c0914 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -48,8 +48,9 @@ internal static bool TrySetAssemblyResolver(Func resolv lock (s_resolverLock) { s_assembliesRequested.Add(assemblyName); - return s_assemblyResolver?.Invoke(assemblyName); } + + return s_assemblyResolver?.Invoke(assemblyName); } } } From 24c3a1ff03ee72703eacda06e8edb131b4fbe572 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 17:35:41 -0800 Subject: [PATCH 5/8] Capture the state of the resolver under the lock for determinism --- .../ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index a490d451c0914..6d38eb6e9de8a 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -45,12 +45,14 @@ internal static bool TrySetAssemblyResolver(Func resolv public Assembly? ResolveAssembly(AssemblyName assemblyName) { + Func? resolver = null; lock (s_resolverLock) { s_assembliesRequested.Add(assemblyName); + resolver = s_assemblyResolver; } - return s_assemblyResolver?.Invoke(assemblyName); + return resolver?.Invoke(assemblyName); } } } From 28191e48a9750b16db86108133c0e336db3a591d Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 10 Dec 2024 17:45:07 -0800 Subject: [PATCH 6/8] Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs Co-authored-by: Jared Parsons --- src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index 6d38eb6e9de8a..e91eea315d1fe 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -34,7 +34,7 @@ internal static bool TrySetAssemblyResolver(Func resolv { lock (s_resolverLock) { - if (s_assemblyResolver is not null && !s_assembliesRequested.Contains(canaryAssembly)) + if (s_assemblyResolver is null && !s_assembliesRequested.Contains(canaryAssembly)) { s_assemblyResolver = resolver; return true; From ad8996856e14c4ddd457fec5aef809d2ef5c88d2 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Wed, 11 Dec 2024 11:31:19 -0800 Subject: [PATCH 7/8] Update src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs Co-authored-by: Jan Jones --- src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index e91eea315d1fe..57ae3b39a2091 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -25,7 +25,7 @@ internal class RazorAnalyzerAssemblyResolver() : IAnalyzerAssemblyResolver private static readonly object s_resolverLock = new(); /// - /// Attempts to set the assembly resolver. Will only succeed if the has not already been requested to load. + /// Attempts to set the assembly resolver. Will only succeed if the has not already been requested to load and the resolver has not been set previously. /// /// The resolver function to set. /// The name of an assembly that is checked to see if it has been requested to load. Setting the resolver will fail if this has already been requested. From 481921f48e214bc86ee355dbfe4b08fa16fcd0cb Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 12 Dec 2024 14:05:58 -0800 Subject: [PATCH 8/8] PR Feedback --- src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs | 2 +- src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs index e91eea315d1fe..763f6e9c3806f 100644 --- a/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs +++ b/src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs @@ -20,7 +20,7 @@ internal class RazorAnalyzerAssemblyResolver() : IAnalyzerAssemblyResolver { private static Func? s_assemblyResolver; - private static readonly List s_assembliesRequested = []; + private static readonly HashSet s_assembliesRequested = []; private static readonly object s_resolverLock = new(); diff --git a/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs b/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs index 9a3dff5dd42c8..25ec3e65f3f4d 100644 --- a/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs +++ b/src/Tools/ExternalAccess/Razor/RazorProjectExtensions.cs @@ -24,7 +24,6 @@ internal static async ValueTask> GetSourceG var generatorIdentity = new SourceGeneratorIdentity(assemblyName, assemblyPath, assemblyVersion, typeName); return results.Where(s => s.Identity.Generator == generatorIdentity); - } } }