From de0d2c80cd9f059fcd090cad59222a1b3026c294 Mon Sep 17 00:00:00 2001
From: JonKazama-Hellion <f.wathling@gmail.com>
Date: Sat, 2 May 2026 02:52:34 +0200
Subject: [PATCH] Serialise retention sweeps so the auto and manual paths
 cannot overlap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Audit findings M-3 and M-4. The 24h auto-sweep launched from
Plugin's constructor and the manual button in the Privacy tab were
both starting a background thread that called DeleteByRetentionPolicy
on the shared MessageStore connection without coordinating. With
unfortunate timing — manual click moments after a fresh plugin load
— two sweeps would race for the same connection and the second
would just re-do work the first one already did, while still
overwriting RetentionLastRunAt.

Move the running flag and a lock object to Plugin so both paths see
the same gate. Each entry point takes the lock long enough to check
and set the flag, then runs the actual delete on its background
thread without holding the lock (other DB operations already happen
without locking; spreading the lock further would suggest a
guarantee we do not actually provide). The Privacy tab keeps a
read-only property that surfaces the shared flag for its UI disable
state — ImGui is single-threaded and bool reads are atomic, so the
lock-free read is fine.
---
 ChatTwo/Plugin.cs                  | 24 ++++++++++++++++++++++++
 ChatTwo/Ui/SettingsTabs/Privacy.cs | 21 ++++++++++++++++-----
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/ChatTwo/Plugin.cs b/ChatTwo/Plugin.cs
index e3c90ea..3c9e441 100755
--- a/ChatTwo/Plugin.cs
+++ b/ChatTwo/Plugin.cs
@@ -64,6 +64,15 @@ public sealed class Plugin : IDalamudPlugin
 
     internal int DeferredSaveFrames = -1;
 
+    // Serialises retention sweeps. The 24h auto-sweep on plugin load and
+    // the manual button in the Privacy tab both run on background threads;
+    // without this gate, hitting the manual button moments after a fresh
+    // plugin start would launch two sweeps in parallel and the second one
+    // would just re-do work the first one already finished. The lock guards
+    // the flag — the flag check itself bails before we touch the database.
+    internal readonly object RetentionSweepLock = new();
+    internal bool RetentionSweepRunning;
+
     internal DateTime GameStarted { get; }
 
     // Tab management needs to happen outside the chatlog window class for access reasons
@@ -405,6 +414,16 @@ public sealed class Plugin : IDalamudPlugin
 
         new Thread(() =>
         {
+            // Bail out cheaply if a manual sweep is already in flight; the
+            // lock around the actual work would queue us up otherwise and
+            // we would just re-do whatever the manual run already did.
+            lock (RetentionSweepLock)
+            {
+                if (RetentionSweepRunning)
+                    return;
+                RetentionSweepRunning = true;
+            }
+
             try
             {
                 var deleted = MessageManager.Store.DeleteByRetentionPolicy(policy, defaultDays);
@@ -429,6 +448,11 @@ public sealed class Plugin : IDalamudPlugin
             {
                 Log.Error(e, "Retention sweep failed");
             }
+            finally
+            {
+                lock (RetentionSweepLock)
+                    RetentionSweepRunning = false;
+            }
         }) { IsBackground = true }.Start();
     }
 
diff --git a/ChatTwo/Ui/SettingsTabs/Privacy.cs b/ChatTwo/Ui/SettingsTabs/Privacy.cs
index 42523af..9db5cfe 100644
--- a/ChatTwo/Ui/SettingsTabs/Privacy.cs
+++ b/ChatTwo/Ui/SettingsTabs/Privacy.cs
@@ -55,7 +55,10 @@ internal sealed class Privacy : ISettingsTab
     private long CleanupDeleteCount;
     private bool CleanupRunning;
 
-    private bool RetentionRunning;
+    // The retention-running state lives on Plugin so the auto-sweep and
+    // this manual button see the same flag. UI reads stay lock-free
+    // because ImGui is single-threaded and bool reads are atomic in .NET.
+    private bool RetentionRunning => Plugin.RetentionSweepRunning;
 
     // Export form state
     private int ExportRangeDays = 30;
@@ -410,10 +413,17 @@ internal sealed class Privacy : ISettingsTab
 
     private void StartRetentionRun()
     {
-        if (RetentionRunning)
-            return;
+        // Take the shared retention lock so we cannot fight the auto-sweep
+        // for the database connection. If the auto-sweep is already in
+        // flight we just bail — the user can press the button again once
+        // it finishes.
+        lock (Plugin.RetentionSweepLock)
+        {
+            if (Plugin.RetentionSweepRunning)
+                return;
+            Plugin.RetentionSweepRunning = true;
+        }
 
-        RetentionRunning = true;
         var policy = Plugin.Config.RetentionPerChannelDays.ToDictionary(p => (int)(ushort)p.Key, p => p.Value);
         var defaultDays = Plugin.Config.RetentionDefaultDays;
 
@@ -445,7 +455,8 @@ internal sealed class Privacy : ISettingsTab
             }
             finally
             {
-                RetentionRunning = false;
+                lock (Plugin.RetentionSweepLock)
+                    Plugin.RetentionSweepRunning = false;
             }
         }) { IsBackground = true }.Start();
     }