From 03c99d26a601e711643ee519917eb4cd74d0ade6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 21:31:43 +0000 Subject: [PATCH 1/2] Lock the description store while a run writes to it [patch] Every run loaded the whole store at start and saved its whole in-memory copy back, so two Scans over different folders, or an Import or Configure during a Scan, overwrote each other's additions without a word. Scan, Import and Configure now hold an exclusive lock file beside the store for the whole run and reload the store under it. A second writing run fails fast with a clear message instead of losing work. Search, Stats and Export only read, so they don't take the lock. Fixes #164 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TTeaDNAaTEviJbNWuEi5hP --- ImageDescriber.Test/StoreLockTests.cs | 81 +++++++++++++++++++++++++++ ImageDescriber/StoreLock.cs | 53 ++++++++++++++++++ ImageDescriber/Verbs/BaseVerb.cs | 31 +++++++++- ImageDescriber/Verbs/Configure.cs | 2 + ImageDescriber/Verbs/Import.cs | 2 + ImageDescriber/Verbs/Scan.cs | 2 + 6 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 ImageDescriber.Test/StoreLockTests.cs create mode 100644 ImageDescriber/StoreLock.cs diff --git a/ImageDescriber.Test/StoreLockTests.cs b/ImageDescriber.Test/StoreLockTests.cs new file mode 100644 index 0000000..1a910dd --- /dev/null +++ b/ImageDescriber.Test/StoreLockTests.cs @@ -0,0 +1,81 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ImageDescriber.Tests; + +using ktsu.ImageDescriber.Verbs; + +[TestClass] +public class StoreLockTests +{ + private string lockPath = string.Empty; + private string originalLockPath = string.Empty; + + [TestInitialize] + public void Initialize() + { + lockPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString(), "ImageDescriber.lock"); + originalLockPath = StoreLock.StorePath; + StoreLock.StorePath = lockPath; + } + + [TestCleanup] + public void Cleanup() + { + StoreLock.StorePath = originalLockPath; + string directory = Path.GetDirectoryName(lockPath)!; + if (Directory.Exists(directory)) + { + Directory.Delete(directory, true); + } + } + + [TestMethod] + public void SecondAcquireFailsWhileTheFirstIsHeld() + { + using (StoreLock? first = StoreLock.TryAcquire(lockPath)) + { + Assert.IsNotNull(first); + Assert.IsNull(StoreLock.TryAcquire(lockPath), "A second run took the lock while the first still held it"); + } + + using StoreLock? again = StoreLock.TryAcquire(lockPath); + Assert.IsNotNull(again, "The lock was not released when the first run ended"); + } + + [TestMethod] + public void VerbsThatSaveTheStoreHoldTheLock() + { + Assert.IsTrue(new Scan().WritesStore); + Assert.IsTrue(new Import().WritesStore); + Assert.IsTrue(new Configure().WritesStore); + Assert.IsFalse(new Search().WritesStore); + Assert.IsFalse(new Stats().WritesStore); + Assert.IsFalse(new Export().WritesStore); + } + + [TestMethod] + public void AWritingVerbRefusesToRunWhileAnotherRunHoldsTheLock() + { + TextWriter originalOut = Console.Out; + using StringWriter output = new(); + using StoreLock? held = StoreLock.TryAcquire(lockPath); + Assert.IsNotNull(held); + + try + { + Console.SetOut(output); + + // The input file does not exist, so if the verb ran at all it would say so, and it + // would never reach a save. + new Import { InputPath = Path.Combine(Path.GetDirectoryName(lockPath)!, "missing.json") }.Run(); + } + finally + { + Console.SetOut(originalOut); + } + + string text = output.ToString(); + Assert.Contains(StoreLock.BusyMessage, text); + Assert.DoesNotContain("File not found", text); + } +} diff --git a/ImageDescriber/StoreLock.cs b/ImageDescriber/StoreLock.cs new file mode 100644 index 0000000..a3c4514 --- /dev/null +++ b/ImageDescriber/StoreLock.cs @@ -0,0 +1,53 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ImageDescriber; + +using System.IO; + +using ktsu.AppDataStorage; + +/// +/// An exclusive, cross-process hold on the description store for the length of one run. +/// +/// +/// Every run loads the whole store and saves its whole in-memory copy back, so two runs that write +/// at the same time each overwrite what the other added. A run that writes takes this lock first +/// and reloads the store under it; a second one fails fast instead of silently losing work. +/// +internal sealed class StoreLock : IDisposable +{ + internal const string BusyMessage = "Another ImageDescriber run is updating the description store. Wait for it to finish and try again."; + + /// + /// The file a run that writes to the store locks, kept beside the store itself. + /// + internal static string StorePath { get; set; } = Path.Combine(AppData.Path.WeakString, "ImageDescriber.lock"); + + private readonly FileStream stream; + + private StoreLock(FileStream stream) => this.stream = stream; + + /// + /// Takes the lock held by the file at , creating it if needed. + /// + /// The held lock, or when another run holds it. + internal static StoreLock? TryAcquire(string path) + { + string? directory = Path.GetDirectoryName(path); + if (!string.IsNullOrEmpty(directory)) + { + Directory.CreateDirectory(directory); + } + + try + { + return new(new FileStream(path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None)); + } + catch (IOException) + { + return null; + } + } + + public void Dispose() => stream.Dispose(); +} diff --git a/ImageDescriber/Verbs/BaseVerb.cs b/ImageDescriber/Verbs/BaseVerb.cs index 00c9f44..c3dd0dc 100644 --- a/ImageDescriber/Verbs/BaseVerb.cs +++ b/ImageDescriber/Verbs/BaseVerb.cs @@ -39,6 +39,11 @@ internal OllamaEndpoint Endpoint internal virtual bool ValidateArgs() => true; + /// + /// Whether the verb saves the store, and so has to hold while it runs. + /// + internal virtual bool WritesStore => false; + public void Execute() => Run(); } @@ -55,9 +60,33 @@ public override void Run() } isActive = false; - Run((T)this); + + if (WritesStore) + { + RunHoldingStoreLock(); + } + else + { + Run((T)this); + } + isActive = true; } + private void RunHoldingStoreLock() + { + using StoreLock? storeLock = StoreLock.TryAcquire(StoreLock.StorePath); + if (storeLock is null) + { + Console.WriteLine(StoreLock.BusyMessage); + return; + } + + // Another run may have saved since this process loaded the store, and this run's saves + // write back the whole copy, so start from what is on disk now. + Program.Settings = PersistentState.LoadOrCreate(); + Run((T)this); + } + internal abstract void Run(T options); } diff --git a/ImageDescriber/Verbs/Configure.cs b/ImageDescriber/Verbs/Configure.cs index 5cd8876..9f919d5 100644 --- a/ImageDescriber/Verbs/Configure.cs +++ b/ImageDescriber/Verbs/Configure.cs @@ -11,6 +11,8 @@ namespace ktsu.ImageDescriber.Verbs; [Verb("Configure", HelpText = "Configure the Ollama endpoint and model settings.")] internal sealed class Configure : BaseVerb { + internal override bool WritesStore => true; + internal override void Run(Configure options) { Console.WriteLine("Current Settings:"); diff --git a/ImageDescriber/Verbs/Import.cs b/ImageDescriber/Verbs/Import.cs index 5fdecec..008706e 100644 --- a/ImageDescriber/Verbs/Import.cs +++ b/ImageDescriber/Verbs/Import.cs @@ -18,6 +18,8 @@ namespace ktsu.ImageDescriber.Verbs; [Verb("Import", HelpText = "Import descriptions from a JSON or CSV file.")] internal sealed class Import : BaseVerb { + internal override bool WritesStore => true; + private static readonly JsonSerializerOptions JsonOptions = new() { Converters = { new RoundTripStringJsonConverterFactory() }, diff --git a/ImageDescriber/Verbs/Scan.cs b/ImageDescriber/Verbs/Scan.cs index 5436b38..827221c 100644 --- a/ImageDescriber/Verbs/Scan.cs +++ b/ImageDescriber/Verbs/Scan.cs @@ -18,6 +18,8 @@ namespace ktsu.ImageDescriber.Verbs; [Verb("Scan", HelpText = "Scan a directory for images, describe them using Ollama, and store results.")] internal sealed class Scan : BaseVerb { + internal override bool WritesStore => true; + internal override bool ValidateArgs() { if (PathString is "." or "") From a27eef83e9bf6a2e4738b418c9d6c884df06a174 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 21:38:07 +0000 Subject: [PATCH 2/2] Join the lock path instead of combining it Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TTeaDNAaTEviJbNWuEi5hP --- ImageDescriber/StoreLock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ImageDescriber/StoreLock.cs b/ImageDescriber/StoreLock.cs index a3c4514..d4e0391 100644 --- a/ImageDescriber/StoreLock.cs +++ b/ImageDescriber/StoreLock.cs @@ -21,7 +21,7 @@ internal sealed class StoreLock : IDisposable /// /// The file a run that writes to the store locks, kept beside the store itself. /// - internal static string StorePath { get; set; } = Path.Combine(AppData.Path.WeakString, "ImageDescriber.lock"); + internal static string StorePath { get; set; } = Path.Join(AppData.Path.WeakString, "ImageDescriber.lock"); private readonly FileStream stream;