diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 8ed0f49..c43a207 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -184,11 +184,16 @@ public sealed class Plugin : IAsyncDalamudPlugin // tears down (the worker logs "rebuild failed" via Log on error paths). private CancellationTokenSource? _ftsRebuildCts; - // Serialises retention sweeps so a manual trigger and the 24h auto-sweep - // can't run in parallel. Volatile because the ImGui thread reads it outside - // the lock to gate the manual button. - internal readonly object RetentionSweepLock = new(); - internal volatile bool RetentionSweepRunning; + // Serialises every long-running database operation against every other one, + // not just retention sweeps against each other. An export leaves a reader + // open on the primary connection outside _readLock by design -- the + // enumerator is consumed lazily -- and a VACUUM meeting that reader hits a + // connection Microsoft documents as not thread-safe. + // + // Replaces the retention-only pair, which solved the same problem for one + // case. The draw thread reads Current every frame to disable buttons and + // must never block doing so. + internal readonly Util.DbOperationGate DbOperations = new(); // B3: neutral owner of the Config.Tabs LIST-structure lock so both the // worker-thread mutator (AutoTellTabsService) and the framework-thread @@ -989,13 +994,10 @@ public sealed class Plugin : IAsyncDalamudPlugin // IsBackground = true so a stuck sweep never blocks plugin unload. new Thread(() => { - // Bail early if a manual sweep is already in flight. - lock (RetentionSweepLock) - { - if (RetentionSweepRunning) - return; - RetentionSweepRunning = true; - } + // Bails when anything else already owns the store, not only another + // sweep: a user-triggered export or cleanup counts too. + if (!DbOperations.TryBegin(Util.DbOperation.RetentionSweep)) + return; try { @@ -1042,8 +1044,7 @@ public sealed class Plugin : IAsyncDalamudPlugin } finally { - lock (RetentionSweepLock) - RetentionSweepRunning = false; + DbOperations.End(); } }) { diff --git a/HellionChat/Util/DbOperationGate.cs b/HellionChat/Util/DbOperationGate.cs new file mode 100644 index 0000000..7adc8c6 --- /dev/null +++ b/HellionChat/Util/DbOperationGate.cs @@ -0,0 +1,70 @@ +namespace HellionChat.Util; + +// Which long-running database operation currently owns the store. +internal enum DbOperation +{ + None, + RetentionSweep, + Export, + Cleanup, + Clear, +} + +// One gate for every operation that holds the message store for longer than a +// frame. Generalises the retention-sweep lock, which already did exactly this +// for a single case. +// +// The reason it has to cover all of them together, not one each: an export +// leaves a reader open on the primary connection deliberately outside _readLock, +// because the enumerator is consumed lazily by its caller. If a VACUUM starts +// while that reader lives, it meets an active reader on a connection Microsoft +// documents as not thread-safe, and PerformMaintenance sets no command timeout +// so it inherits five seconds before throwing. +// +// Pure state machine, no ImGui and no database, so the build suite can pin the +// transitions without standing up either. +internal sealed class DbOperationGate +{ + private readonly object _lock = new(); + + // Volatile because the draw thread reads it every frame to decide which + // buttons are disabled, and must never block on the lock to do so -- that + // would freeze the game for the length of a VACUUM. + private volatile DbOperation _current = DbOperation.None; + + internal DbOperation Current => _current; + + internal bool IsBusy => _current != DbOperation.None; + + // False when another operation already owns the store. Callers must not + // queue or wait: everything here is user-initiated, and a queued wipe that + // fires minutes later is worse than one that refuses. + internal bool TryBegin(DbOperation operation) + { + if (operation == DbOperation.None) + throw new ArgumentOutOfRangeException( + nameof(operation), + "None is the idle state, not an operation to begin." + ); + + lock (_lock) + { + if (_current != DbOperation.None) + return false; + + _current = operation; + return true; + } + } + + // Idempotent, and deliberately not checking which operation ends. A worker + // that throws must still be able to release from a finally block without + // knowing whether it ever acquired. + internal void End() + { + lock (_lock) + { + _current = DbOperation.None; + } + } +}