feat(db): one gate for every long-running database operation

Generalises the retention-sweep lock, which already solved this for a single
case: it stopped a manual sweep from racing the automatic one, and nothing else.

Export, cleanup and clear need the same protection against each other, and for a
sharper reason. An export leaves a reader open on the primary connection
deliberately outside _readLock, because the enumerator is consumed lazily by its
caller. A VACUUM starting while that reader lives 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 -- after the DELETE
has already committed.

TryBegin refuses rather than queues. Every one of these is user-initiated, and a
wipe that fires minutes after the click is worse than one that declines. End is
idempotent and does not check which operation ends, so a worker that throws
before acquiring can still release from its finally block.

Current is volatile because the draw thread reads it every frame to decide which
buttons are disabled. Blocking on the lock to find that out would freeze the game
for the length of a VACUUM, which is the exact failure this is meant to prevent.

Pure state machine, so the transitions are pinned without a database or an ImGui
frame -- including that exactly one of 64 competing callers wins.
This commit is contained in:
2026-08-18 20:19:27 +02:00
parent 125a57167e
commit d0eb2934ed
2 changed files with 85 additions and 14 deletions
+15 -14
View File
@@ -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();
}
})
{
+70
View File
@@ -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;
}
}
}