From 152d4c3d57d537eeed310b894d5584965fbbf49a Mon Sep 17 00:00:00 2001 From: Claudio Schaad Date: Thu, 2 Jul 2026 20:40:55 +0200 Subject: [PATCH] PR B: atomic XML writes and per-file locking Save writes to a .tmp sibling and atomically renames, so a crash mid-write leaves the previous file intact. Load, Save, and EnsureFileExisits share a per-absolute-path lock so concurrent Save+Save and Save+Load can't observe a half-written file. Co-Authored-By: Claude Opus 4.7 --- .../Repositories/BaseRepository.cs | 70 ++++++++++++++----- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/Schaad.Accounting.Db/Repositories/BaseRepository.cs b/Schaad.Accounting.Db/Repositories/BaseRepository.cs index 139c9da..9f727d8 100644 --- a/Schaad.Accounting.Db/Repositories/BaseRepository.cs +++ b/Schaad.Accounting.Db/Repositories/BaseRepository.cs @@ -1,4 +1,6 @@ -using System.IO; +using System; +using System.Collections.Concurrent; +using System.IO; using System.Text; using System.Xml; using System.Xml.Serialization; @@ -8,6 +10,11 @@ namespace Schaad.Accounting.Repositories { public abstract class BaseRepository { + // One lock per absolute file path so concurrent Save+Save and Save+Load are serialized + // and can't observe a half-written file. + private static readonly ConcurrentDictionary FileLocks = + new(StringComparer.OrdinalIgnoreCase); + protected readonly ISettingsService settingsService; protected BaseRepository(ISettingsService settingsService) @@ -18,35 +25,53 @@ namespace Schaad.Accounting.Repositories protected void EnsureFileExisits(string fileName) { string filePath = Path.Combine(settingsService.GetDbPath(), fileName); - if (File.Exists(filePath) == false) + lock (GetLock(filePath)) { - var lastYearFile = Path.Combine(settingsService.GetLastYearDbPath(), fileName); - if (File.Exists(lastYearFile)) + if (File.Exists(filePath) == false) { - File.Copy(lastYearFile, filePath); + var lastYearFile = Path.Combine(settingsService.GetLastYearDbPath(), fileName); + if (File.Exists(lastYearFile)) + { + File.Copy(lastYearFile, filePath); + } } } } /// - /// Save an object to an xml file + /// Save an object to an xml file. Writes to a .tmp sibling and then atomically + /// renames it, so a crash mid-write leaves the previous file intact. /// protected void Save(T obj, string fileName) { var filePath = Path.Combine(settingsService.GetDbPath(), fileName); - using (var sww = new MemoryStream()) + var tmpPath = filePath + ".tmp"; + + lock (GetLock(filePath)) { var settings = new XmlWriterSettings { Encoding = Encoding.UTF8, Indent = true }; - using (var writer = XmlWriter.Create(sww, settings)) + + try { - var xsSubmit = new XmlSerializer(typeof(T)); - xsSubmit.Serialize(writer, obj); - var xml = Encoding.UTF8.GetString(sww.ToArray()); - File.WriteAllText(filePath, xml); + using (var writer = XmlWriter.Create(tmpPath, settings)) + { + var serializer = new XmlSerializer(typeof(T)); + serializer.Serialize(writer, obj); + } + + File.Move(tmpPath, filePath, overwrite: true); + } + catch + { + if (File.Exists(tmpPath)) + { + try { File.Delete(tmpPath); } catch { /* best effort */ } + } + throw; } } } @@ -57,16 +82,23 @@ namespace Schaad.Accounting.Repositories protected T Load(string fileName) { var filePath = Path.Combine(settingsService.GetDbPath(), fileName); - if (File.Exists(filePath) == false) - { - return default(T); - } - using (XmlReader reader = XmlReader.Create(filePath)) + lock (GetLock(filePath)) { - var serializer = new XmlSerializer(typeof(T)); - return (T)serializer.Deserialize(reader); + if (File.Exists(filePath) == false) + { + return default(T); + } + + using (XmlReader reader = XmlReader.Create(filePath)) + { + var serializer = new XmlSerializer(typeof(T)); + return (T)serializer.Deserialize(reader); + } } } + + private static object GetLock(string filePath) + => FileLocks.GetOrAdd(filePath, _ => new object()); } } \ No newline at end of file