fileindex: withFolderRO reloads without the cross-process lock — can race a locked compaction and poison shared in-memory header (NextUID regression persists after #645)

[0kaba0hub]

Summary

#645 fixed one real trigger of #644 (a cached logFD surviving truncateLog's rename, detected via inode identity), but the NextUID regression still reproduces on 2.1.11 under the same concurrent imaptest+smoketest load — confirmed live, same signature (UIDValidity unchanged, NextUID regresses from a high value straight to 1/2/3), and this time with no ".log replaced" WARN from #645's new detector ever firing. A second, distinct trigger for the same class of bug remains open.

Root cause (code-verified)

withFolderRO (internal/storage/index/file/file.go:451) calls fs.reload() under only the in-process mutex (fs.mu) — it never acquires the cross-process distributed lock (u.b.locker):

func (u *userIndex) withFolderRO(folderID uint64, fn func(*folderState) error) error {
	u.mu.Lock()
	fs, ok := u.open[folderID]
	u.mu.Unlock()
	...
	fs.mu.Lock()
	err := fs.reload()   // <- no u.b.locker acquisition, unlike withFolderLock
	fs.mu.Unlock()
	...
}

Compare with the write path, withFolderLock (file.go:437), which acquires u.b.locker (locks.MailboxKey(...)) before touching fs.mu/reload() — every write-side reload is serialized against every other process's writes to the same folder. withFolderRO's reload has no such guarantee: it can run concurrently with another process's locked compaction (flush() + truncateLog()).

withFolderRO is called from 5 sites, most importantly OpenFolder's dedup path (folder.go:55) — the first thing every deliverOne call in internal/lmtp/deliver.go does. If this unlocked reload interleaves with a concurrent, lock-holding compaction on the same folder (a real ordering that can happen: nothing prevents a reader from calling os.Stat/reading the log mid-write from a writer that is not yet done, since the reader took no lock to exclude it), it can load a torn or momentarily-inconsistent view into the folder's in-memory header — the same shared folderState (via fs.mu, not u.b.locker) that every subsequent correctly-locked operation (AllocateUID, AppendMessage, …) then trusts as a baseline. If the torn read leaves fs.baseMod/fs.logSize looking "as expected" (coincidentally matching the true current on-disk values despite the header contents being stale), later locked reloads' fast-path (newBaseMod == fs.baseMod && newLogSize == fs.logSize) can short-circuit and never notice the header itself was poisoned.

This is a distinct mechanism from #644/#645: it isn't about a stale cached logFD surviving a rename (which #645's inode-identity check detects) — it's an unlocked reader racing a locked writer's in-progress compaction, which can poison the shared in-memory state without ever tripping the fd-replacement check, because the fd itself may not have been replaced yet (or the race is in reading base+log content, not fd identity).

Reproduction (live, 2.1.11 — after #645)

Same imaptest + smoketest (all three mailbox types) concurrent-load battery used to originally reproduce #644. u1@d00001.test INBOX:

01:21:33.099 fileindex: uid allocated folder=INBOX uid=28
...(no "reload full", no "log replaced" WARN, no compaction log line for INBOX in between)...
01:21:43.285 fileindex: uid allocated folder=INBOX uid=1   <- regression
01:21:43.404 fileindex: uid allocated folder=INBOX uid=2

fts: index run start breadcrumbs (from #640) confirm UIDValidity never changed across this window (stored_uidvalidity == current_uidvalidity throughout, both before and after the regression) — ruling out any UIDVALIDITY-triggered reset. And critically, no "fileindex: .log replaced under open fd" WARN appears anywhere in the window — #645's detector never fired, yet the regression happened anyway.

Suggested fix

withFolderRO needs to be serialized against writers the same way withFolderLock is — at minimum for its reload() call. Options, roughly in order of preference:

1. Have withFolderRO acquire the same u.b.locker resource (shared/read intent if the lock backend supports it, or the same exclusive key if not — correctness over throughput here) before calling reload(), mirroring withFolderLock's pattern exactly for that one call.
2. At minimum, ensure OpenFolder's dedup path (the highest-impact caller, hit on every delivery) does a locked reload rather than routing through withFolderRO.

Debug coverage requested

Filed alongside a PR adding more breadcrumbs around withFolderRO/withFolderLock's reload calls (logging fs.file.Header.NextUID/fs.baseMod/fs.logSize before and after each reload, tagged with whether the cross-process lock was held) so the next live reproduction can directly show an unlocked reload racing a locked writer and catch the exact interleaving, rather than inferring it from absence of the #645 detector firing.

Related

Distinct trigger for the same underlying class of bug as #644 (RFC 3501 UID-monotonicity violation under concurrent load, mdbox/sdbox-observed, report/imap objectid smoketest symptom). #645 correctly fixed the stale-logFD/inode-mismatch trigger but did not close this one.