Lite has two conventions for locking a DuckDB write, and one of them is written down as the rule #2463

Found while ruling on #2455, and deliberately not fixed there — #2455 is one file's lock choice, this is the model it sits in, and changing that inside a three-method PR is how a repo-wide convention gets altered without anyone deciding to.

#2455 asked whether FindingStore's read-lock-around-a-write is correct. It is, on the axis the class doc names: DuckDbInitializer.s_dbLock coordinates everyone against MAINTENANCE (CHECKPOINT, archive DELETEs, compaction), which takes the exclusive write lock, and a held read lock blocks EnterWriteLock — so holding one is how a write says "not while I am in flight". Verified rather than assumed, on DuckDB.NET 1.5.5: two connections appending to the same table in concurrent transactions both commit, and the retention DELETE overlaps an insert batch cleanly.

What that ruling surfaced is that the codebase does not have one convention here. It has two.

The split

Writes that take the WRITE lock — and one of them states it as the rule:

• LocalDataService.OpenWriteConnectionAsync (Lite/Services/LocalDataService.cs:62), whose own doc comment reads: "Creates and opens a DuckDB connection wrapped in an exclusive write lock. Use for UPDATE/DELETE/INSERT operations that must not race with archival or compaction." That is a house rule, written down, with the same justification FindingStore uses to reach the opposite answer.
• DuckDbAlertHistoryStore — 6 sites, all ordinary alert-state writes (alert log INSERT, watermark upsert, incident-occurrence delete+insert).
• DuckDbMuteRuleStore — 5 sites, ordinary mute-rule CRUD.

Writes that take the READ lock:

• FindingStore.InsertFindingsAsync, MuteStoryAsync, CleanupOldFindingsAsync.

The genuinely maintenance-shaped callers (ArchiveService, QueryStoreSliceRepairService, DuckDbInitializer, the post-collection CHECKPOINT in RemoteCollectorService) take the write lock and are not in question.

So DuckDbMuteRuleStore.InsertAsync writes config_mute_rules under a write lock while FindingStore.MuteStoryAsync writes analysis_muted under a read lock. Same species of write, same store, opposite lock, no note anywhere saying why either is right.

Why it matters more than a tidiness complaint

Both cannot be the rule, and the difference is not free in either direction.

• If the write lock is the rule, FindingStore is wrong and every finding batch can currently overlap a CHECKPOINT-adjacent operation the other stores exclude.
• If the read lock is the rule, then 11 write sites serialize themselves against every UI read for no reason — and that is a real cost, because AcquireWriteLock() with no timeout blocks behind archival indefinitely. LocalDataService is the only one that passes a timeout (5 s, explicitly "prevents UI freeze if archival currently holds the lock"); the other ten would simply wait.

My reading is that the read lock is right and the write-lock sites are over-locking, because DuckDB does its own concurrency control and the only thing the lock has to exclude is the file being reorganized underneath a running statement. But that is a judgement about eleven call sites in three files I did not measure, and it should be made deliberately, with the answer written into DuckDbInitializer's class doc so the next writer inherits a rule instead of picking one.

A second, latent hazard in the same model

Worth recording while the lock is in view, and explicitly not a live defect today.

ReaderWriterLockSlim is thread-affine: ExitReadLock throws SynchronizationLockException when called from a thread that did not enter. LockReleaser.Dispose calls it unguarded, and every one of these locks is held across await:

using var readLock = _duckDb.AcquireReadLock(context.CancellationToken);
using var connection = _duckDb.CreateConnection();
await connection.OpenAsync(context.CancellationToken);   // a resumption here on another thread
...                                                       // would make the Dispose below throw

The analysis pass runs under Task.Run with no SynchronizationContext, so a continuation is free to resume on a different pool thread. It does not, and the reason is measured rather than lucky: DuckDB.NET 1.5.5's async methods complete synchronously. Thread id across OpenAsync, a DDL statement, 200 ExecuteNonQueryAsync calls and an ExecuteReaderAsync/ReadAsync drain: 4, 4, 4, 4, 4. Nothing ever yields, so the entering thread is always the exiting thread.

That makes the whole AcquireReadLock/await pattern — 66 call sites in Lite/Analysis alone per #2443 — dependent on an undocumented property of the driver's async implementation. A DuckDB.NET release that made any of these genuinely asynchronous would turn it into intermittent SynchronizationLockExceptions across the store layer, in the paths that hold locks longest. Cheap mitigations if it is worth pre-empting: guard LockReleaser.Dispose with IsReadLockHeld/IsWriteLockHeld, or replace the lock with a SemaphoreSlim (not thread-affine, and it has a genuine WaitAsync(CancellationToken) — which would also retire #2443's 50 ms poll).

Suggested shape

1. Decide the rule and write it into DuckDbInitializer's class doc, which is where every caller already looks.
2. Reconcile whichever set of call sites is on the wrong side of it.
3. Decide the thread-affinity question separately — it is a property of the lock primitive, not of any caller.

Not urgent on any of the three. Filed so the split is recorded rather than rediscovered by the next person who reads FindingStore and reaches for AcquireWriteLock.

Related

• Lite's finding INSERT batch takes a read lock to write #2455 (the ruling that surfaced this; FindingStore keeps the read lock and now says why)
• The analysis token is armed but 167 Darling and ~138 Lite store calls never receive it #2443 (made the read-lock WAIT abandonable — the reason switching these to write locks is not free)
• A finding batch that fails mid-insert leaves a truncated set that reads as a complete one #2448 (the same batch, on partial persists)