Files
HellionChat/HellionChat/Util/DbOperationGate.cs
T
JonKazama-Hellion fdb1a98519 fix(privacy): the cleanup could never be applied, and three more from the audit
The cleanup preview marked itself stale before it could be drawn. The
gate bumps a revision on release so a preview cannot survive a wipe; I
then made the preview take the gate, so its own release invalidated it
every single time and the apply button never appeared. The feature has
been shipping non-functional since it was written, with a self-test that
asserted the exact bump that killed it.

Read-only operations no longer move the revision, and preview and
maintenance have their own marks instead of borrowing Cleanup -- which
also stops the five-second metadata refresh from expiring previews, and
stops the UI announcing "another operation is running: cleanup" during a
VACUUM.

The JSON export produced invalid JSON. The chat relation kinds were
interpolated straight into the output, and interpolating an enum writes
its member name, so every message with a recognised relation came out as
"source_kind":LocalPlayer. That is the file a GDPR access request goes
out on. The self-test wrote a JSON file and never parsed it; it does now.

Retention with the limit at zero still deleted. The slider is labelled
"0 = never" and the sweep seeded 31 spec defaults unconditionally before
reading the user's overrides, so zero still lost free company, linkshell
and party history after ninety days -- and the short-circuit written for
exactly this case could never be reached, because the map was never
empty.

A wipe that worked reported that it had failed. VACUUM needs the
database to itself, the refilter walks a lazy reader on the primary
connection outside the lock, and the two collide -- after the DELETE has
committed. The delete paths no longer let that escape: the rows are
gone, an uncompacted file is a housekeeping problem, and telling
somebody their history is still there when it is not is a different kind
of problem.

Also:

- CSV cells starting with =, +, - or @ get a leading apostrophe. The
  content is text other people typed into a chat channel and the file
  exists to be opened in a spreadsheet.
- An export that matched nothing no longer replaces the previous one. It
  used to write its header, move it into place, and then report that
  nothing matched. Dalamud's save dialog offers no overwrite
  confirmation to fall back on, so this is the part that had to move.
- The retention sweep says so when it loses the race for the gate, and
  routes its notifications through the teardown check like everything
  else.
2026-08-19 07:17:33 +02:00

108 lines
4.2 KiB
C#

namespace HellionChat.Util;
// Which long-running database operation currently owns the store.
internal enum DbOperation
{
None,
RetentionSweep,
Export,
Cleanup,
Clear,
// Read-only, but they hold the store long enough to matter: the preview
// scans every row, the metadata read takes the read lock, and maintenance
// rewrites the file without touching a single row.
Preview,
Maintenance,
}
// 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 holds
// a reader open for as long as it writes, and VACUUM needs the database to
// itself. Since v1.12.0 that reader sits on its own connection, so the clash
// surfaces as SQLITE_BUSY and a five-second timeout rather than the immediate
// SQLITE_ERROR a shared connection produced -- but a VACUUM that gives up after
// five seconds still fails, and it fails after the DELETE has committed. The
// rows are gone and the file is not compacted. PerformMaintenance runs VACUUM,
// REINDEX and ANALYZE as one batch, so the latter two never run either.
//
// Serialising the operations removes the question instead of tuning timeouts
// around it.
//
// 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;
// Bumped whenever an operation that could have changed rows finishes. A
// cleanup preview snapshots it and treats a mismatch as stale: after a
// retention sweep or a wipe its numbers describe a database that is gone,
// and the comparison against the config alone cannot see that.
private long _revision;
internal long Revision => Interlocked.Read(ref _revision);
// Which operations can change what a preview counted. Getting this wrong in
// the permissive direction only costs a needless recount; getting it wrong
// the other way lets somebody confirm a number that is no longer true.
//
// The preview itself must not be in here, and that is not a detail: it takes
// the gate, so counting its own release would mark every preview stale the
// instant it finished and the apply button would never appear.
private static bool Mutates(DbOperation operation) =>
operation is DbOperation.RetentionSweep or DbOperation.Cleanup or DbOperation.Clear;
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;
}
}
// Releases only what the caller acquired. Idempotent for that caller, and a
// no-op for anyone else -- a worker whose TryBegin was refused still runs its
// finally, and a blind reset there would hand away the lock of whichever
// operation actually holds it. That is worse than no gate: the refused
// worker walks away believing it did nothing while a VACUUM starts under
// somebody's open reader.
internal void End(DbOperation operation)
{
lock (_lock)
{
if (_current != operation)
return;
_current = DbOperation.None;
if (Mutates(operation))
Interlocked.Increment(ref _revision);
}
}
}