Serialise retention sweeps so the auto and manual paths cannot overlap
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.
This commit is contained in:
@@ -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();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user