From a6fc3c8976d9df7fb5de28e42528e49dd734165b Mon Sep 17 00:00:00 2001 From: Matt Parker <61717342+MattParkerDev@users.noreply.github.com> Date: Sun, 23 Nov 2025 14:49:45 +1000 Subject: [PATCH] Make AllFiles threadsafe --- .../Features/Analysis/RoslynAnalysis.cs | 6 ++--- .../IdeFileExternalChangeHandler.cs | 10 ++++----- .../SharpIdeSolutionModificationService.cs | 22 ++++++++++++++----- .../Features/Search/SearchService.cs | 4 ++-- .../VsPersistence/SharpIdeModels.cs | 4 ++-- .../VsPersistence/VsPersistenceMapper.cs | 2 +- .../Features/CodeEditor/CodeEditorPanel.cs | 2 +- .../SharpIdeCodeEdit_SymbolLookup.cs | 4 ++-- .../Features/Problems/ProblemsPanel.cs | 2 +- src/SharpIDE.Godot/IdeRoot.cs | 2 +- 10 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/SharpIDE.Application/Features/Analysis/RoslynAnalysis.cs b/src/SharpIDE.Application/Features/Analysis/RoslynAnalysis.cs index 9cb1454..68f6685 100644 --- a/src/SharpIDE.Application/Features/Analysis/RoslynAnalysis.cs +++ b/src/SharpIDE.Application/Features/Analysis/RoslynAnalysis.cs @@ -709,7 +709,7 @@ public class RoslynAnalysis(ILogger logger, BuildService buildSe .Select(async (Document doc, CancellationToken ct) => { var text = await doc.GetTextAsync(ct); - var sharpFile = _sharpIdeSolutionModel!.AllFiles.Single(f => f.Path == doc.FilePath); + var sharpFile = _sharpIdeSolutionModel!.AllFiles[doc.FilePath!]; return (sharpFile, text.ToString()); }) .ToListAsync(cancellationToken); @@ -723,8 +723,8 @@ public class RoslynAnalysis(ILogger logger, BuildService buildSe if (semanticModel is null) return null; var enclosingSymbol = ReferenceLocationExtensions.GetEnclosingMethodOrPropertyOrField(semanticModel, referenceLocation); var lineSpan = referenceLocation.Location.GetMappedLineSpan(); - var file = _sharpIdeSolutionModel!.AllFiles.SingleOrDefault(f => f.Path == lineSpan.Path); - var result = new IdeReferenceLocationResult(referenceLocation, file!, enclosingSymbol); + var file = _sharpIdeSolutionModel!.AllFiles[lineSpan.Path]; + var result = new IdeReferenceLocationResult(referenceLocation, file, enclosingSymbol); return result; } diff --git a/src/SharpIDE.Application/Features/FileWatching/IdeFileExternalChangeHandler.cs b/src/SharpIDE.Application/Features/FileWatching/IdeFileExternalChangeHandler.cs index 9f91625..2f9bdde 100644 --- a/src/SharpIDE.Application/Features/FileWatching/IdeFileExternalChangeHandler.cs +++ b/src/SharpIDE.Application/Features/FileWatching/IdeFileExternalChangeHandler.cs @@ -28,14 +28,14 @@ public class IdeFileExternalChangeHandler private async Task OnFileRenamed(string oldFilePath, string newFilePath) { - var sharpIdeFile = SolutionModel.AllFiles.SingleOrDefault(f => f.Path == oldFilePath); + var sharpIdeFile = SolutionModel.AllFiles.GetValueOrDefault(oldFilePath); if (sharpIdeFile is null) return; await _sharpIdeSolutionModificationService.RenameFile(sharpIdeFile, Path.GetFileName(newFilePath)); } private async Task OnFileDeleted(string filePath) { - var sharpIdeFile = SolutionModel.AllFiles.SingleOrDefault(f => f.Path == filePath); + var sharpIdeFile = SolutionModel.AllFiles.GetValueOrDefault(filePath); if (sharpIdeFile is null) return; await _sharpIdeSolutionModificationService.RemoveFile(sharpIdeFile); } @@ -90,7 +90,7 @@ public class IdeFileExternalChangeHandler private async Task OnFileCreated(string filePath) { - var sharpIdeFile = SolutionModel.AllFiles.SingleOrDefault(f => f.Path == filePath); + var sharpIdeFile = SolutionModel.AllFiles.GetValueOrDefault(filePath); if (sharpIdeFile is not null) { // It was likely already created via a parent folder creation @@ -110,7 +110,7 @@ public class IdeFileExternalChangeHandler private async Task OnFileChanged(string filePath) { - var sharpIdeFile = SolutionModel.AllFiles.SingleOrDefault(f => f.Path == filePath); + var sharpIdeFile = SolutionModel.AllFiles.GetValueOrDefault(filePath); if (sharpIdeFile is null) return; if (sharpIdeFile.SuppressDiskChangeEvents is true) return; if (sharpIdeFile.LastIdeWriteTime is not null) @@ -123,7 +123,7 @@ public class IdeFileExternalChangeHandler } } _logger.LogInformation("IdeFileExternalChangeHandler: Changed - '{FilePath}'", filePath); - var file = SolutionModel.AllFiles.SingleOrDefault(f => f.Path == filePath); + var file = SolutionModel.AllFiles.GetValueOrDefault(filePath); if (file is not null) { await _fileChangedService.SharpIdeFileChanged(file, await File.ReadAllTextAsync(file.Path), FileChangeType.ExternalChange); diff --git a/src/SharpIDE.Application/Features/FileWatching/SharpIdeSolutionModificationService.cs b/src/SharpIDE.Application/Features/FileWatching/SharpIdeSolutionModificationService.cs index 5541b8d..1dc21df 100644 --- a/src/SharpIDE.Application/Features/FileWatching/SharpIdeSolutionModificationService.cs +++ b/src/SharpIDE.Application/Features/FileWatching/SharpIdeSolutionModificationService.cs @@ -1,14 +1,16 @@ using System.Collections.Concurrent; using Microsoft.CodeAnalysis; +using Microsoft.Extensions.Logging; using SharpIDE.Application.Features.SolutionDiscovery; using SharpIDE.Application.Features.SolutionDiscovery.VsPersistence; namespace SharpIDE.Application.Features.FileWatching; /// Does not do any file system operations, only modifies the in-memory solution model -public class SharpIdeSolutionModificationService(FileChangedService fileChangedService) +public class SharpIdeSolutionModificationService(FileChangedService fileChangedService, ILogger logger) { private readonly FileChangedService _fileChangedService = fileChangedService; + private readonly ILogger _logger = logger; public SharpIdeSolutionModel SolutionModel { get; set; } = null!; @@ -24,7 +26,11 @@ public class SharpIdeSolutionModificationService(FileChangedService fileChangedS parentNode.Folders.Insert(correctInsertionPosition, sharpIdeFolder); SolutionModel.AllFolders.AddRange((IEnumerable)[sharpIdeFolder, ..allFolders]); - SolutionModel.AllFiles.AddRange(allFiles); + foreach (var sharpIdeFile in allFiles) + { + var success = SolutionModel.AllFiles.TryAdd(sharpIdeFile.Path, sharpIdeFile); + if (success is false) _logger.LogWarning("File {filePath} already exists in SolutionModel.AllFiles when adding directory {directoryPath}", sharpIdeFile.Path, addedDirectoryPath); + } foreach (var file in allFiles) { await _fileChangedService.SharpIdeFileAdded(file, await File.ReadAllTextAsync(file.Path)); @@ -55,7 +61,11 @@ public class SharpIdeSolutionModificationService(FileChangedService fileChangedS var filesToRemove = foldersToRemove.SelectMany(f => f.Files).ToList(); - SolutionModel.AllFiles.RemoveRange(filesToRemove); + foreach (var sharpIdeFile in filesToRemove) + { + var success = SolutionModel.AllFiles.TryRemove(sharpIdeFile.Path, out _); + if (success is false) _logger.LogWarning("File {filePath} not found in SolutionModel.AllFiles when removing directory {directoryPath}", sharpIdeFile.Path, folder.Path); + } SolutionModel.AllFolders.RemoveRange(foldersToRemove); foreach (var file in filesToRemove) { @@ -138,7 +148,8 @@ public class SharpIdeSolutionModificationService(FileChangedService fileChangedS var correctInsertionPosition = GetInsertionPosition(parentNode, sharpIdeFile); parentNode.Files.Insert(correctInsertionPosition, sharpIdeFile); - SolutionModel.AllFiles.Add(sharpIdeFile); + var success = SolutionModel.AllFiles.TryAdd(sharpIdeFile.Path, sharpIdeFile); + if (success is false) _logger.LogWarning("File {filePath} already exists in SolutionModel.AllFiles when creating file", sharpIdeFile.Path); await _fileChangedService.SharpIdeFileAdded(sharpIdeFile, contents); return sharpIdeFile; } @@ -192,7 +203,8 @@ public class SharpIdeSolutionModificationService(FileChangedService fileChangedS { var parentFolderOrProject = (IFolderOrProject)file.Parent; parentFolderOrProject.Files.Remove(file); - SolutionModel.AllFiles.Remove(file); + var success = SolutionModel.AllFiles.TryRemove(file.Path, out _); + if (success is false) _logger.LogWarning("File {filePath} not found in SolutionModel.AllFiles when removing file", file.Path); await _fileChangedService.SharpIdeFileRemoved(file); } diff --git a/src/SharpIDE.Application/Features/Search/SearchService.cs b/src/SharpIDE.Application/Features/Search/SearchService.cs index 9ea826d..be674b3 100644 --- a/src/SharpIDE.Application/Features/Search/SearchService.cs +++ b/src/SharpIDE.Application/Features/Search/SearchService.cs @@ -18,7 +18,7 @@ public class SearchService(ILogger logger) } var timer = Stopwatch.StartNew(); - var files = solutionModel.AllFiles; + var files = solutionModel.AllFiles.Values.ToList(); ConcurrentBag results = []; await Parallel.ForEachAsync(files, cancellationToken, async (file, ct) => { @@ -52,7 +52,7 @@ public class SearchService(ILogger logger) } var timer = Stopwatch.StartNew(); - var files = solutionModel.AllFiles; + var files = solutionModel.AllFiles.Values.ToList(); ConcurrentBag results = []; await Parallel.ForEachAsync(files, cancellationToken, async (file, ct) => { diff --git a/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/SharpIdeModels.cs b/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/SharpIdeModels.cs index f0d630f..d79ec15 100644 --- a/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/SharpIdeModels.cs +++ b/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/SharpIdeModels.cs @@ -55,7 +55,7 @@ public class SharpIdeSolutionModel : ISharpIdeNode, IExpandableSharpIdeNode, ISo public required ObservableHashSet Projects { get; set; } public required ObservableHashSet SlnFolders { get; set; } public required HashSet AllProjects { get; set; } // TODO: this isn't thread safe - public required HashSet AllFiles { get; set; } // TODO: this isn't thread safe + public required ConcurrentDictionary AllFiles { get; set; } public required HashSet AllFolders { get; set; } // TODO: this isn't thread safe public bool Expanded { get; set; } @@ -72,7 +72,7 @@ public class SharpIdeSolutionModel : ISharpIdeNode, IExpandableSharpIdeNode, ISo Projects = new ObservableHashSet(intermediateModel.Projects.Select(s => new SharpIdeProjectModel(s, allProjects, allFiles, allFolders, this))); SlnFolders = new ObservableHashSet(intermediateModel.SolutionFolders.Select(s => new SharpIdeSolutionFolder(s, allProjects, allFiles, allFolders, this))); AllProjects = allProjects.ToHashSet(); - AllFiles = allFiles.ToHashSet(); + AllFiles = new ConcurrentDictionary(allFiles.DistinctBy(s => s.Path).ToDictionary(s => s.Path)); AllFolders = allFolders.ToHashSet(); } } diff --git a/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/VsPersistenceMapper.cs b/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/VsPersistenceMapper.cs index a91da93..c5ddb6c 100644 --- a/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/VsPersistenceMapper.cs +++ b/src/SharpIDE.Application/Features/SolutionDiscovery/VsPersistence/VsPersistenceMapper.cs @@ -22,7 +22,7 @@ public static class VsPersistenceMapper { // Assumes solution file is at git repo root var filePath = new FileInfo(Path.Combine(solutionModel.DirectoryPath, entry.FilePath)).FullName; // used to normalise path separators - var fileInSolution = solutionModel.AllFiles.SingleOrDefault(f => f.Path.Equals(filePath, StringComparison.OrdinalIgnoreCase)); + var fileInSolution = solutionModel.AllFiles.GetValueOrDefault(filePath); if (fileInSolution is null) continue; var mappedGitStatus = entry.State switch diff --git a/src/SharpIDE.Godot/Features/CodeEditor/CodeEditorPanel.cs b/src/SharpIDE.Godot/Features/CodeEditor/CodeEditorPanel.cs index e3414f8..a9657df 100644 --- a/src/SharpIDE.Godot/Features/CodeEditor/CodeEditorPanel.cs +++ b/src/SharpIDE.Godot/Features/CodeEditor/CodeEditorPanel.cs @@ -126,7 +126,7 @@ public partial class CodeEditorPanel : MarginContainer if (executionStopInfo.FilePath != currentSharpIdeFile?.Path) { - var file = Solution.AllFiles.Single(s => s.Path == executionStopInfo.FilePath); + var file = Solution.AllFiles[executionStopInfo.FilePath]; await GodotGlobalEvents.Instance.FileExternallySelected.InvokeParallelAsync(file, null).ConfigureAwait(false); } var lineInt = executionStopInfo.Line - 1; // Debugging is 1-indexed, Godot is 0-indexed diff --git a/src/SharpIDE.Godot/Features/CodeEditor/SharpIdeCodeEdit_SymbolLookup.cs b/src/SharpIDE.Godot/Features/CodeEditor/SharpIdeCodeEdit_SymbolLookup.cs index 4e19cb6..06f09cc 100644 --- a/src/SharpIDE.Godot/Features/CodeEditor/SharpIdeCodeEdit_SymbolLookup.cs +++ b/src/SharpIDE.Godot/Features/CodeEditor/SharpIdeCodeEdit_SymbolLookup.cs @@ -44,7 +44,7 @@ public partial class SharpIdeCodeEdit var referenceLocation = locations[0]; var referenceLineSpan = referenceLocation.Location.GetMappedLineSpan(); - var sharpIdeFile = Solution!.AllFiles.SingleOrDefault(f => f.Path == referenceLineSpan.Path); + var sharpIdeFile = Solution!.AllFiles.GetValueOrDefault(referenceLineSpan.Path); if (sharpIdeFile is null) { GD.Print($"Reference file not found in solution: {referenceLineSpan.Path}"); @@ -82,7 +82,7 @@ public partial class SharpIdeCodeEdit // Lets jump to the definition var definitionLocation = locations[0]; var definitionLineSpan = definitionLocation.GetMappedLineSpan(); - var sharpIdeFile = Solution!.AllFiles.SingleOrDefault(f => f.Path == definitionLineSpan.Path); + var sharpIdeFile = Solution!.AllFiles.GetValueOrDefault(definitionLineSpan.Path); if (sharpIdeFile is null) { GD.Print($"Definition file not found in solution: {definitionLineSpan.Path}"); diff --git a/src/SharpIDE.Godot/Features/Problems/ProblemsPanel.cs b/src/SharpIDE.Godot/Features/Problems/ProblemsPanel.cs index 81cc649..ac943f5 100644 --- a/src/SharpIDE.Godot/Features/Problems/ProblemsPanel.cs +++ b/src/SharpIDE.Godot/Features/Problems/ProblemsPanel.cs @@ -119,7 +119,7 @@ public partial class ProblemsPanel : Control private void OpenDocumentContainingDiagnostic(Diagnostic diagnostic) { var fileLinePositionSpan = diagnostic.Location.GetMappedLineSpan(); - var file = Solution!.AllFiles.Single(f => f.Path == fileLinePositionSpan.Path); + var file = Solution!.AllFiles[fileLinePositionSpan.Path]; var linePosition = new SharpIdeFileLinePosition(fileLinePositionSpan.StartLinePosition.Line, fileLinePositionSpan.StartLinePosition.Character); GodotGlobalEvents.Instance.FileExternallySelected.InvokeParallelFireAndForget(file, linePosition); } diff --git a/src/SharpIDE.Godot/IdeRoot.cs b/src/SharpIDE.Godot/IdeRoot.cs index 13bd05e..3593c9f 100644 --- a/src/SharpIDE.Godot/IdeRoot.cs +++ b/src/SharpIDE.Godot/IdeRoot.cs @@ -164,7 +164,7 @@ public partial class IdeRoot : Control var previousTabs = Singletons.AppState.RecentSlns.Single(s => s.FilePath == solutionModel.FilePath).IdeSolutionState.OpenTabs; var filesToOpen = previousTabs - .Select(s => (solutionModel.AllFiles.SingleOrDefault(f => f.Path == s.FilePath), new SharpIdeFileLinePosition(s.CaretLine, s.CaretColumn), s.IsSelected)) + .Select(s => (solutionModel.AllFiles.GetValueOrDefault(s.FilePath), new SharpIdeFileLinePosition(s.CaretLine, s.CaretColumn), s.IsSelected)) .Where(s => s.Item1 is not null) .OfType<(SharpIdeFile file, SharpIdeFileLinePosition linePosition, bool isSelected)>() .ToList();