diff --git a/CSharpRepl.Services/CodeTransformation/Disassembly/Disassembler.cs b/CSharpRepl.Services/CodeTransformation/Disassembly/Disassembler.cs index e92f1c3b..3d982767 100644 --- a/CSharpRepl.Services/CodeTransformation/Disassembly/Disassembler.cs +++ b/CSharpRepl.Services/CodeTransformation/Disassembly/Disassembler.cs @@ -36,7 +36,7 @@ internal partial class Disassembler /// public (EvaluationResult Result, IReadOnlyList CommentSpans) Render(CompilationResult compilation) { - var file = new PEFile(Guid.NewGuid().ToString(), compilation.AssemblyStream, PEStreamOptions.LeaveOpen); + using var file = new PEFile(Guid.NewGuid().ToString(), compilation.AssemblyStream, PEStreamOptions.LeaveOpen); using var debugInfo = new PortablePdbDebugInfoProvider(compilation.PdbStream!); var ilCodeOutput = DisassembleFile(file, debugInfo); diff --git a/CSharpRepl.Services/Configuration.cs b/CSharpRepl.Services/Configuration.cs index b20878db..9b41ed3a 100644 --- a/CSharpRepl.Services/Configuration.cs +++ b/CSharpRepl.Services/Configuration.cs @@ -174,7 +174,7 @@ public Configuration( } else { - Console.Error.WriteLine($"{AnsiColor.Red.GetEscapeSequence()}Unable to parse '{prompt}' markup. Defaut prompt '{PromptDefault}' will be used.{AnsiEscapeCodes.Reset}"); + Console.Error.WriteLine($"{AnsiColor.Red.GetEscapeSequence()}Unable to parse '{promptMarkup}' markup. Default prompt '{PromptDefault}' will be used.{AnsiEscapeCodes.Reset}"); Prompt = PromptDefault; } diff --git a/CSharpRepl.Services/Roslyn/References/AssemblyReferenceService.cs b/CSharpRepl.Services/Roslyn/References/AssemblyReferenceService.cs index 269a796d..c298ea2e 100644 --- a/CSharpRepl.Services/Roslyn/References/AssemblyReferenceService.cs +++ b/CSharpRepl.Services/Roslyn/References/AssemblyReferenceService.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; @@ -31,11 +32,12 @@ internal sealed class AssemblyReferenceService private readonly DotNetInstallationLocator dotnetInstallationLocator; private readonly ConcurrentDictionary sharedFrameworksByReferencePath; private readonly ConcurrentDictionary cachedMetadataReferences; - private readonly HashSet loadedReferenceAssemblies; - private readonly HashSet loadedImplementationAssemblies; - private readonly HashSet referenceAssemblyPaths; - private readonly HashSet implementationAssemblyPaths; - private readonly HashSet sharedFrameworkImplementationAssemblyPaths; + + private ImmutableHashSet loadedReferenceAssemblies = ImmutableHashSet.Create(new AssemblyReferenceComparer()); + private ImmutableHashSet loadedImplementationAssemblies = ImmutableHashSet.Create(new AssemblyReferenceComparer()); + private ImmutableHashSet referenceAssemblyPaths = ImmutableHashSet.Empty; + private ImmutableHashSet implementationAssemblyPaths = ImmutableHashSet.Empty; + private ImmutableHashSet sharedFrameworkImplementationAssemblyPaths = ImmutableHashSet.Empty; private readonly HashSet usings; private readonly CSharpParseOptions parseOptions; private readonly ITraceLogger logger; @@ -55,13 +57,8 @@ public AssemblyReferenceService(Configuration config, CSharpParseOptions parseOp this.parseOptions = parseOptions; this.logger = logger; this.dotnetInstallationLocator = new DotNetInstallationLocator(logger); - this.referenceAssemblyPaths = []; - this.implementationAssemblyPaths = []; - this.sharedFrameworkImplementationAssemblyPaths = []; this.sharedFrameworksByReferencePath = new(); this.cachedMetadataReferences = new(); - this.loadedReferenceAssemblies = new(new AssemblyReferenceComparer()); - this.loadedImplementationAssemblies = new(new AssemblyReferenceComparer()); this.usings = new[] { "System", "System.IO", "System.Collections.Generic", @@ -126,11 +123,12 @@ public SharedFramework[] GetSharedFrameworkConfiguration(string framework, Versi internal IReadOnlyCollection EnsureReferenceAssemblyWithDocumentation(IReadOnlyCollection references) { - loadedReferenceAssemblies.UnionWith( - references.Select(suppliedReference => EnsureReferenceAssembly(suppliedReference)).WhereNotNull() - ); + // Materialize first: ImmutableInterlocked.Update re-invokes the transformer on contention, so the added items + // must be a stable collection (not a lazy query that would re-run EnsureReferenceAssembly's file I/O each retry). + var resolved = references.Select(suppliedReference => EnsureReferenceAssembly(suppliedReference)).WhereNotNull().ToArray(); + ImmutableInterlocked.Update(ref loadedReferenceAssemblies, static (set, toAdd) => set.Union(toAdd), resolved); // loadedReferenceAssemblies accumulates across submissions and is deduplicated only by path, so it can hold - // several versions of the same assembly. Unify before handing the set to the workspace/compilation. + // several versions of the same assembly. Unify (against the immutable snapshot) before handing it to the workspace/compilation. return RemoveDuplicateReferences(loadedReferenceAssemblies); } @@ -276,8 +274,13 @@ private static (string framework, Version version) GetDesiredFrameworkVersion(st return cachedReference; } + // Read the immutable sets once into locals so all checks below see a consistent snapshot even if another + // thread (e.g. background initialization) swaps in an additional framework concurrently. + var referenceAssemblyPathsSnapshot = referenceAssemblyPaths; + var sharedFrameworkImplementationAssemblyPathsSnapshot = sharedFrameworkImplementationAssemblyPaths; + // it's already a reference assembly, just cache it and use it. - if (referenceAssemblyPaths.Any(path => suppliedAssemblyPath.StartsWith(path))) + if (referenceAssemblyPathsSnapshot.Any(path => suppliedAssemblyPath.StartsWith(path))) { cachedMetadataReferences[suppliedAssemblyPath] = reference; return reference; @@ -287,12 +290,12 @@ private static (string framework, Version version) GetDesiredFrameworkVersion(st var suppliedAssemblyFileName = Path.GetFileName(suppliedAssemblyPath); var suppliedAssemblyName = AssemblyName.GetAssemblyName(suppliedAssemblyPath).ToString(); - var assembly = referenceAssemblyPaths + var assembly = referenceAssemblyPathsSnapshot .Select(path => Path.Combine(path, suppliedAssemblyFileName)) .FirstOrDefault(potentialReferencePath => File.Exists(potentialReferencePath) && AssemblyName.GetAssemblyName(potentialReferencePath).ToString() == suppliedAssemblyName) ?? suppliedAssemblyPath; - if (sharedFrameworkImplementationAssemblyPaths.Any(path => assembly.StartsWith(path))) + if (sharedFrameworkImplementationAssemblyPathsSnapshot.Any(path => assembly.StartsWith(path))) { return null; } @@ -316,12 +319,14 @@ private static (string framework, Version version) GetDesiredFrameworkVersion(st internal void AddImplementationAssemblyReferences(IEnumerable references) { - var paths = references + var referenceList = references as IReadOnlyCollection ?? references.ToArray(); + var paths = referenceList .Select(r => Path.GetDirectoryName(r.Display) ?? r.Display) // GetDirectoryName returns null when at root directory - .WhereNotNull(); + .WhereNotNull() + .ToArray(); // materialize: the Update transformer below may re-enumerate this on contention - this.implementationAssemblyPaths.UnionWith(paths); - this.loadedImplementationAssemblies.UnionWith(references); + ImmutableInterlocked.Update(ref implementationAssemblyPaths, static (set, toAdd) => set.Union(toAdd), paths); + ImmutableInterlocked.Update(ref loadedImplementationAssemblies, static (set, toAdd) => set.Union(toAdd), referenceList); } public void LoadSharedFrameworkConfiguration(string framework, Version version) @@ -332,15 +337,21 @@ public void LoadSharedFrameworkConfiguration(string framework, Version version) public void LoadSharedFrameworkConfiguration(SharedFramework[] sharedFrameworks) { - this.referenceAssemblyPaths.UnionWith(sharedFrameworks.Select(framework => framework.ReferencePath)); - this.implementationAssemblyPaths.UnionWith(sharedFrameworks.Select(framework => framework.ImplementationPath)); - this.sharedFrameworkImplementationAssemblyPaths.UnionWith(sharedFrameworks.Select(framework => framework.ImplementationPath)); - this.loadedReferenceAssemblies.UnionWith(sharedFrameworks.SelectMany(framework => framework.ReferenceAssemblies)); - this.loadedImplementationAssemblies.UnionWith(sharedFrameworks.SelectMany(framework => framework.ImplementationAssemblies)); + // Materialize the projections up front: ImmutableInterlocked.Update may re-invoke its transformer on contention. + var referencePaths = sharedFrameworks.Select(framework => framework.ReferencePath).ToArray(); + var implementationPaths = sharedFrameworks.Select(framework => framework.ImplementationPath).ToArray(); + var referenceAssemblies = sharedFrameworks.SelectMany(framework => framework.ReferenceAssemblies).ToArray(); + var implementationAssemblies = sharedFrameworks.SelectMany(framework => framework.ImplementationAssemblies).ToArray(); + + ImmutableInterlocked.Update(ref referenceAssemblyPaths, static (set, toAdd) => set.Union(toAdd), referencePaths); + ImmutableInterlocked.Update(ref implementationAssemblyPaths, static (set, toAdd) => set.Union(toAdd), implementationPaths); + ImmutableInterlocked.Update(ref sharedFrameworkImplementationAssemblyPaths, static (set, toAdd) => set.Union(toAdd), implementationPaths); + ImmutableInterlocked.Update(ref loadedReferenceAssemblies, static (set, toAdd) => set.Union(toAdd), referenceAssemblies); + ImmutableInterlocked.Update(ref loadedImplementationAssemblies, static (set, toAdd) => set.Union(toAdd), implementationAssemblies); // make the framework reference assemblies findable by path, so EnsureReferenceAssembly reuses // them instead of loading a second copy when it maps an implementation assembly to its reference assembly. - foreach (var reference in sharedFrameworks.SelectMany(framework => framework.ReferenceAssemblies)) + foreach (var reference in referenceAssemblies) { if (reference.Display is not null) { diff --git a/CSharpRepl.Services/Roslyn/References/DotNetInstallationLocator.cs b/CSharpRepl.Services/Roslyn/References/DotNetInstallationLocator.cs index aa80b43c..8e2bd71e 100644 --- a/CSharpRepl.Services/Roslyn/References/DotNetInstallationLocator.cs +++ b/CSharpRepl.Services/Roslyn/References/DotNetInstallationLocator.cs @@ -111,7 +111,10 @@ internal DotNetInstallationLocator(ITraceLogger logger, IFileSystem io, string d var referenceAssemblyPath = io.Directory .GetDirectories(referenceAssemblyRoot, "net*" + version.Major + "." + version.Minor + "*", SearchOption.AllDirectories) - .OrderBy(path => path) + // Order by the parsed framework version, not lexicographically: a plain string sort puts "10.0.10" + // before "10.0.9", so LastOrDefault() would pick the lower patch. Mirrors GetGlobalImplementationAssemblyPath. + .OrderBy(path => ParseReferenceAssemblyVersion(path)) + .ThenBy(path => path + ".") // trick to get e.g. 6.0 to come after 6.0-preview .LastOrDefault(); if (referenceAssemblyPath is null) return null; @@ -119,6 +122,18 @@ internal DotNetInstallationLocator(ITraceLogger logger, IFileSystem io, string d return Path.GetFullPath(referenceAssemblyPath); } + /// + /// Extracts the framework version from a reference-assembly path like + /// C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\10.0.10\ref\net10.0 + /// two levels above the target-framework leaf, which holds for both the global packs and the NuGet ".ref" package layouts. + /// + private static Version ParseReferenceAssemblyVersion(string targetFrameworkPath) + { + var versionDirectory = Path.GetFileName(Path.GetDirectoryName(Path.GetDirectoryName(targetFrameworkPath))); + // discard any trailing preview suffix, e.g. 6.0.0-preview.4.21253.7, mirroring SharedFramework.ToDotNetVersion. + return Version.TryParse(versionDirectory?.Split('-', 2)[0], out var parsed) ? parsed : new Version(0, 0, 0, 0); + } + /// /// Returns the path to globally installed Implementation Assemblies like C:\Program Files\dotnet\shared\Microsoft.NETCore.App\5.0.10 /// diff --git a/CSharpRepl.Services/SymbolExploration/SourceLink/SourceLinkLookup.cs b/CSharpRepl.Services/SymbolExploration/SourceLink/SourceLinkLookup.cs index 5118d431..afbea326 100644 --- a/CSharpRepl.Services/SymbolExploration/SourceLink/SourceLinkLookup.cs +++ b/CSharpRepl.Services/SymbolExploration/SourceLink/SourceLinkLookup.cs @@ -67,7 +67,14 @@ public bool TryGetSourceLinkUrl(MetadataReader symbolReader, SequencePointRange { var cdi = symbolReader.GetCustomDebugInformation(cdih); if (symbolReader.GetGuid(cdi.Kind) == SourceLinkId) + { blobh = cdi.Value; + } + } + + if (blobh.IsNil) + { + return null; } var utf8JsonBytes = symbolReader.GetBlobBytes(blobh); diff --git a/CSharpRepl.Services/Theming/StyledString.cs b/CSharpRepl.Services/Theming/StyledString.cs index 02b0c569..02eb8ba1 100644 --- a/CSharpRepl.Services/Theming/StyledString.cs +++ b/CSharpRepl.Services/Theming/StyledString.cs @@ -54,7 +54,7 @@ public StyledString(IEnumerable parts) } public char FirstChar => parts[0].Text[0]; - public char LastChar => parts[^1].Text[0]; + public char LastChar => parts[^1].Text[^1]; /// /// diff --git a/CSharpRepl/Program.cs b/CSharpRepl/Program.cs index dd9be56a..bdbfb1e6 100644 --- a/CSharpRepl/Program.cs +++ b/CSharpRepl/Program.cs @@ -306,5 +306,5 @@ internal static class ExitCodes public const int ErrorParseArguments = 1; public const int ErrorAnsiEscapeSequencesNotSupported = 2; public const int ErrorInvalidConsoleHandle = 3; - public const int ErrorCancelled = 3; + public const int ErrorCancelled = 4; } \ No newline at end of file diff --git a/Tests/CSharpRepl.Tests/DotNetInstallationLocatorTest.cs b/Tests/CSharpRepl.Tests/DotNetInstallationLocatorTest.cs index 61d7b2f1..a80736cc 100644 --- a/Tests/CSharpRepl.Tests/DotNetInstallationLocatorTest.cs +++ b/Tests/CSharpRepl.Tests/DotNetInstallationLocatorTest.cs @@ -81,6 +81,38 @@ public void GetSharedFrameworkConfiguration_Net10Installation_IsLocated() ); } + [Fact] + public void GetSharedFrameworkConfiguration_MultipleDoubleDigitPatches_PicksHighestVersion() + { + // "10.0.10" sorts *before* "10.0.9" lexicographically, so a plain string sort over the paths would wrongly + // pick the lower patch 10.0.9. Both the reference and implementation paths must select 10.0.10. + var fileSystem = new MockFileSystem(new Dictionary + { + { @"/Program Files/dotnet/packs/Microsoft.NETCore.App.Ref/10.0.9/ref/net10.0/Microsoft.CSharp.dll", string.Empty }, + { @"/Program Files/dotnet/packs/Microsoft.NETCore.App.Ref/10.0.10/ref/net10.0/Microsoft.CSharp.dll", string.Empty }, + + { @"/Program Files/dotnet/shared/Microsoft.NETCore.App/10.0.9/Microsoft.CSharp.dll", string.Empty }, + { @"/Program Files/dotnet/shared/Microsoft.NETCore.App/10.0.10/Microsoft.CSharp.dll", string.Empty } + }); + + var locator = new DotNetInstallationLocator( + logger: new TestTraceLogger(), io: fileSystem, + dotnetRuntimePath: @"/Program Files/dotnet/", + userProfilePath: @"/Users/bob/" + ); + + var (refPath, implPath) = locator.FindInstallation("Microsoft.NETCore.App", new Version(10, 0, 9)); + + Assert.Equal( + CrossPlatform(@"/Program Files/dotnet/packs/Microsoft.NETCore.App.Ref/10.0.10/ref/net10.0"), + CrossPlatform(refPath) + ); + Assert.Equal( + CrossPlatform(@"/Program Files/dotnet/shared/Microsoft.NETCore.App/10.0.10"), + CrossPlatform(implPath) + ); + } + [Fact] public void GetSharedFrameworkConfiguration_Net10UsesNuGetWhenNotInstalledGlobally() {