From 78bc40eb8c5c91372509dbad5d9ab7608c19c459 Mon Sep 17 00:00:00 2001 From: Jesse Klaasse <11753409+JesseKlaasse@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:22:08 +0200 Subject: [PATCH] Apply the version rule to the resource fallback, and unpack each resource once With s_assembliesWithResources populated, the resource fallback runs, and it had two problems that the lookup above it does not: - It returned whatever it extracted, regardless of version: a request for 2.0.0.0 got the 1.0.0.0 copy. That is what fails Request_For_A_Newer_Version_Does_Not_Resolve_To_An_Older_One in CI. - Assembly.Load(byte[]) loads a new copy on every call and none unloads, so every unsatisfied request added another copy of an assembly that was already loaded. The fallback now skips a copy that does not satisfy the request (same Satisfies rule as the lookup), and a resource is unpacked at most once, whether by AddAssembly or by the fallback (s_extractedResources). Also removes the unused 'alreadyLoaded' local (CS0168). Tests: an unsatisfied request does not load the assembly again; the fallback unpacks a resource AddAssembly has not reached yet (the window another generator initialising on another thread can hit), and does not hand back an older version from there. Each fails with its guard removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../AssemblyResolverTests.cs | 50 ++++++++++++++++++- .../Templates/SourceGeneratorHoistBase.cs | 48 +++++++++++++++--- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/Sandbox/ConsoleApp.SourceGenerator.Tests/AssemblyResolverTests.cs b/src/Sandbox/ConsoleApp.SourceGenerator.Tests/AssemblyResolverTests.cs index 14666ec..ac72e50 100644 --- a/src/Sandbox/ConsoleApp.SourceGenerator.Tests/AssemblyResolverTests.cs +++ b/src/Sandbox/ConsoleApp.SourceGenerator.Tests/AssemblyResolverTests.cs @@ -77,6 +77,54 @@ public void Request_For_A_Newer_Version_Does_Not_Resolve_To_An_Older_One() Assert.Throws(() => Assembly.Load(new AssemblyName("OutdatedDependency, Version=2.0.0.0"))); } + [Fact] + public void Unsatisfied_Request_Does_Not_Load_The_Embedded_Assembly_Again() + { + byte[] dependency = Emit("ReloadedDependency", "1.0.0.0"); + LoadInOwnContext(Emit("ReloadingGenerator", "1.0.0.0", ("SGF.Assembly::ReloadedDependency.dll", dependency))); + + for (int i = 0; i < 3; i++) + { + Assert.Throws(() => Assembly.Load(new AssemblyName("ReloadedDependency, Version=2.0.0.0"))); + } + + Assert.Single(AppDomain.CurrentDomain.GetAssemblies(), a => a.GetName().Name == "ReloadedDependency"); + } + + [Fact] + public void Resource_Fallback_Unpacks_An_Embedded_Assembly_Not_Unpacked_Yet() + { + byte[] dependency = Emit("LateDependency", "1.0.0.0"); + Assembly generator = LoadInOwnContext(Emit("LateGenerator", "1.0.0.0", ("SGF.Assembly::LateDependency.dll", dependency))); + ForgetUnpacking(generator, "LateDependency, Version=1.0.0.0"); + + Assembly resolved = Assembly.Load(new AssemblyName("LateDependency, Version=1.0.0.0")); + + Assert.Equal(new Version(1, 0, 0, 0), resolved.GetName().Version); + } + + [Fact] + public void Resource_Fallback_Does_Not_Resolve_To_An_Older_Version() + { + byte[] dependency = Emit("LateOutdatedDependency", "1.0.0.0"); + Assembly generator = LoadInOwnContext(Emit("LateOutdatedGenerator", "1.0.0.0", ("SGF.Assembly::LateOutdatedDependency.dll", dependency))); + ForgetUnpacking(generator, "LateOutdatedDependency, Version=1.0.0.0"); + + Assert.Throws(() => Assembly.Load(new AssemblyName("LateOutdatedDependency, Version=2.0.0.0"))); + } + + // Puts the resolver back in the window between registering an assembly with resources and unpacking them, + // which another generator initialising on another thread can hit. Only the resource fallback covers it. + private static void ForgetUnpacking(Assembly generator, string dependency) + { + Type hoist = typeof(ConsoleAppSourceGeneratorHoist).BaseType!; + T Field(string name) => (T)hoist.GetField(name, BindingFlags.NonPublic | BindingFlags.Static)!.GetValue(null)!; + + AssemblyName name = new(dependency); + Field>("s_extractedResources").Remove($"{generator.FullName}|SGF.Assembly::{name.Name}.dll"); + Field>("s_loadedAssemblies").Remove(name); + } + private static byte[] Emit(string name, string version, params (string Name, byte[] Content)[] resources) { CSharpCompilation compilation = CSharpCompilation.Create( @@ -94,7 +142,7 @@ private static byte[] Emit(string name, string version, params (string Name, byt } // Roslyn loads each analyzer directory in a context of its own; the resolver sees them all through the AppDomain. - private static void LoadInOwnContext(byte[] image) + private static Assembly LoadInOwnContext(byte[] image) => new AssemblyLoadContext(name: null).LoadFromStream(new MemoryStream(image)); } } diff --git a/src/SourceGenerator.Foundations/Templates/SourceGeneratorHoistBase.cs b/src/SourceGenerator.Foundations/Templates/SourceGeneratorHoistBase.cs index f925633..1ac2328 100644 --- a/src/SourceGenerator.Foundations/Templates/SourceGeneratorHoistBase.cs +++ b/src/SourceGenerator.Foundations/Templates/SourceGeneratorHoistBase.cs @@ -34,6 +34,7 @@ internal abstract class SourceGeneratorHoist private static readonly Dictionary s_loadedAssemblies; private static readonly object s_loadedAssembliesLock; private static readonly object s_assembliesWithResourcesLock; + private static readonly HashSet s_extractedResources; static SourceGeneratorHoist() { @@ -50,6 +51,7 @@ static SourceGeneratorHoist() s_loadedAssemblies = new Dictionary(new AssemblyNameComparer()); s_loadedAssembliesLock = new object(); s_assembliesWithResourcesLock = new object(); + s_extractedResources = new HashSet(); Initialize(); } @@ -130,6 +132,10 @@ private static void AddAssembly(Assembly assembly) foreach (string resource in resources) { + if (!TryMarkExtracted(assembly, resource)) + { + continue; + } #pragma warning disable RS1035 // Do not use APIs banned for analyzers System.Console.WriteLine($"Extracting {resource} assembly from {assemblyName.Name}'s resources."); @@ -149,7 +155,6 @@ private static void AddAssembly(Assembly assembly) { AssemblyName assemblyName = new(args.Name); - Assembly? alreadyLoaded; KeyValuePair[] loadedSnapshot; lock (s_loadedAssembliesLock) { @@ -168,9 +173,7 @@ private static void AddAssembly(Assembly assembly) foreach (KeyValuePair loaded in loadedSnapshot) { Version version = loaded.Key.Version ?? new Version(0, 0); - bool sameName = string.Equals(loaded.Key.Name, assemblyName.Name); - bool satisfies = assemblyName.Version == null || version >= assemblyName.Version; - if (sameName && satisfies && (newest == null || version > newestVersion)) + if (Satisfies(loaded.Key, assemblyName) && (newest == null || version > newestVersion)) { newest = loaded.Value; newestVersion = version; @@ -188,19 +191,52 @@ private static void AddAssembly(Assembly assembly) resourceSnapshot = s_assembliesWithResources.ToArray(); } + // Reaches a resource AddAssembly has not unpacked yet (generators initialise concurrently). Same version + // rule as above: a copy that does not satisfy the request is skipped, so an older one is never returned. + // A resource is unpacked once: Assembly.Load(byte[]) loads a new copy on every call, and none unloads. + string resourceName = $"SGF.Assembly::{assemblyName.Name}.dll"; foreach (Assembly loadedAssembly in resourceSnapshot) { - string resourceName = $"SGF.Assembly::{assemblyName.Name}.dll"; + if (!TryMarkExtracted(loadedAssembly, resourceName)) + { + continue; + } + if (TryExtractingAssembly(loadedAssembly, resourceName, out Assembly? extractedAssembly)) { AddAssembly(extractedAssembly!); - return extractedAssembly!; + if (Satisfies(extractedAssembly!.GetName(), assemblyName)) + { + return extractedAssembly; + } }; } return null; } + /// + /// Claims a resource of an assembly for extraction. False if it was claimed before. + /// + private static bool TryMarkExtracted(Assembly assembly, string resourceName) + { + lock (s_assembliesWithResourcesLock) + { + return s_extractedResources.Add($"{assembly.FullName}|{resourceName}"); + } + } + + /// + /// True if can stand in for : same name, and at least + /// the requested version when there is one. + /// + private static bool Satisfies(AssemblyName candidate, AssemblyName requested) + { + Version version = candidate.Version ?? new Version(0, 0); + return string.Equals(candidate.Name, requested.Name) + && (requested.Version == null || version >= requested.Version); + } + /// /// Attempts to load an assembly that is contained within another assembly as a resource