Pad/internal/io/pool/real/filesystem_test.go
Greg Pomerantz 6c6a0c1a27 Fix write-concurrency race: per-file write protocol + unique temps
The review-identified race: saves are async (owner snapshots content,
worker pool writes), nothing serialized per file, and every write of a
file used the SAME deterministic temp ('.<name>.tmp'). Two overlapping
writes (autosave x autosave, retry x autosave, or the synchronous
FlushAll on GoToBrowser/Shutdown x a worker write) interleaved on the
shared temp and could rename a byte-mixture into place; even without
interleaving, last-rename-wins could promote a STALE snapshot.

Owner-side protocol (logic.go, requestSave + result handler):
- at most one write in flight per file (writeInFlight maps filename ->
  the file version whose content the in-flight write carries);
- a save requested while one is in flight is deferred (savePending) and
  re-issued by the write's result handler with a FRESH snapshot, so
  'last rename wins' coincides with 'newest snapshot wins';
- on success the SNAPSHOT version (not the current one) is recorded as
  written, so an edit that arrived during the write leaves the file
  dirty and triggers the re-issue;
- FlushAll (GoToBrowser, Shutdown) defers via the same protocol instead
  of writing concurrently on the shared temp;
- shutdown drain: on done the owner waits (bounded 5 s) for in-flight
  writes and armed retries to settle before exiting, so the post-exit
  synchronous FlushAll and workerPool.Stop cannot race a straggling
  worker write;
- retry timer now sends a non-blocking token (no timer-goroutine stall
  on a full channel); emitFrame no longer blocks on a slow/gone main
  (frames are snapshots; the next emission wins) - also required so the
  drain can never deadlock on frame delivery.

Mechanism (real/filesystem.go):
- WriteFileAtomic uses a unique per-call temp ('.<name>.tmp.<pid>.<seq>'),
  making same-file staging-file interleaving structurally impossible even
  if the serialization regressed (defense in depth);
- each successful write best-effort removes stale temps of the same file
  (crash leftovers, plus the legacy deterministic name for upgraded
  installs); a failed write removes its own temp.

Tests:
- write_serialization_test.go (e2e): a counting FS wrapper proves the
  peak concurrent same-file saves is 1 across two deliberately
  overlapping autosaves (2 s saves; the second edit lands inside the
  first save's window and its token is deferred, then re-issued with the
  newer content), and that a Flush during an in-flight save adds no
  concurrent writer and the newest snapshot still wins. Mutation-verified:
  disabling the deferral fails it with peak = 2. (The pool's WriteFileTask
  calls FS.WriteFile, not WriteFileAtomic - the real FS is atomic only
  because WriteFile delegates to WriteFileAtomic; the wrapper mirrors that
  delegation or the overlap window does not exist.)
- filesystem_test.go: stale-temp test updated to the new pattern, also
  covering the legacy name and asserting a different file's temp is
  untouched.
- real_file_fuzz_test.go: stray-temp check matches both patterns.

Docs: architecture.md 6.5 rewritten (protocol invariants), spec.md
autosave line, development_plan.md v11 + Phase 12.

On-device smoke: open, type, autosave lands exact content on disk, no
temp files left, clean relaunch. Full suite green under -race.

Residuals (documented): no fsync before rename (power-loss window only);
external-change detection absent; a drain-deadline exit with a straggling
write can only lose freshness (unique temps keep every rename a complete
snapshot).
2026-08-17 16:29:47 -04:00

213 lines
6.2 KiB
Go

package real
// Data-integrity tests for the persistence layer's atomicity contract. The
// editor's corruption protection against crashes rests on WriteFileAtomic's
// write-sibling-temp-then-rename scheme; these tests pin the observable
// consequences of that contract:
//
// - a written file is byte-exact (no truncation/padding at any size),
// - a concurrent reader NEVER observes a torn file: every read returns
// exactly one of the known complete payloads (the "crash/kill mid-write
// leaves a complete snapshot" property, without needing a real crash),
// - a FAILED write leaves the original file byte-identical,
// - a stale leftover temp file is consumed by the next successful write.
import (
"bytes"
"math/rand"
"os"
"path/filepath"
"strconv"
"sync"
"sync/atomic"
"testing"
)
func randomPayload(t *testing.T, seed int64, size int) []byte {
t.Helper()
rng := rand.New(rand.NewSource(seed))
b := make([]byte, size)
rng.Read(b)
return b
}
func TestWriteFileAtomic_RoundTripExact(t *testing.T) {
for _, size := range []int{0, 1, 2, 65537, 5 * 1024 * 1024} {
d := t.TempDir()
fs := NewRealFileSystem(d)
data := randomPayload(t, int64(size), size)
if err := fs.WriteFileAtomic("/rt.txt", data); err != nil {
t.Fatalf("size %d: write: %v", size, err)
}
got, err := fs.ReadFile("/rt.txt")
if err != nil {
t.Fatalf("size %d: read: %v", size, err)
}
if !bytes.Equal(got, data) {
t.Fatalf("size %d: round trip mismatch (got %d bytes)", size, len(got))
}
// Overwrite with different content of the same size.
data2 := randomPayload(t, int64(size)+1, size)
if err := fs.WriteFileAtomic("/rt.txt", data2); err != nil {
t.Fatalf("size %d: overwrite: %v", size, err)
}
got, err = fs.ReadFile("/rt.txt")
if err != nil {
t.Fatalf("size %d: read after overwrite: %v", size, err)
}
if !bytes.Equal(got, data2) {
t.Fatalf("size %d: overwrite mismatch", size)
}
}
}
// TestWriteFileAtomic_ConcurrentReader_NeverTorn hammers the file with two
// alternating 2 MB payloads while a reader spins; every single read must
// return exactly one of the two payloads in full. Any torn read (a mix, a
// truncation, a padding) is a data-corruption failure.
func TestWriteFileAtomic_ConcurrentReader_NeverTorn(t *testing.T) {
d := t.TempDir()
fs := NewRealFileSystem(d)
const size = 2 * 1024 * 1024
payloadA := randomPayload(t, 1, size)
payloadB := randomPayload(t, 2, size)
if err := fs.WriteFileAtomic("/torn.txt", payloadA); err != nil {
t.Fatal(err)
}
stop := make(chan struct{})
var (
wg sync.WaitGroup
reads atomic.Int64
torn atomic.Int64
tornDetail string // written only by the reader, read after wg.Wait()
)
wg.Add(1)
go func() {
defer wg.Done()
for {
select {
case <-stop:
return
default:
}
data, err := fs.ReadFile("/torn.txt")
if err != nil {
continue // transient (rename boundary); next read decides
}
reads.Add(1)
if !bytes.Equal(data, payloadA) && !bytes.Equal(data, payloadB) {
torn.Store(1)
at := 0
ref := payloadA
if len(data) < len(ref) {
ref = data
}
for at < len(data) && at < len(ref) && data[at] == ref[at] {
at++
}
tornDetail = "first diff at " + itoa(at) + " (read " + itoa(len(data)) + " bytes, want " + itoa(size) + ")"
return
}
}
}()
const iters = 150
for i := 0; i < iters; i++ {
payload := payloadA
if i%2 == 1 {
payload = payloadB
}
if err := fs.WriteFileAtomic("/torn.txt", payload); err != nil {
t.Fatal(err)
}
}
close(stop)
wg.Wait()
if torn.Load() == 1 {
t.Fatalf("torn read observed: %s", tornDetail) // safe: reader exited (wg.Wait)
}
if n := reads.Load(); n < 20 {
t.Fatalf("only %d concurrent reads happened; test not meaningful", n)
}
}
// TestWriteFileAtomic_FailedWriteLeavesOriginalIntact verifies the crash
// property directly: when the write cannot complete (read-only directory),
// the original file is untouched, byte for byte.
func TestWriteFileAtomic_FailedWriteLeavesOriginalIntact(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("running as root: permission bits are bypassed")
}
d := t.TempDir()
fs := NewRealFileSystem(d)
original := []byte("original content that must survive\n")
if err := fs.WriteFileAtomic("/f.txt", original); err != nil {
t.Fatal(err)
}
if err := os.Chmod(d, 0o555); err != nil {
t.Fatal(err)
}
defer func() { _ = os.Chmod(d, 0o755) }()
if err := fs.WriteFileAtomic("/f.txt", []byte("new content")); err == nil {
t.Fatal("expected write to fail in read-only directory")
}
got, err := fs.ReadFile("/f.txt")
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(got, original) {
t.Fatalf("failed write modified the original file: got %q", got)
}
}
// TestWriteFileAtomic_StaleTempFileIsConsumed verifies a crash-leftover temp
// file (garbage from an interrupted write) does not poison the next
// successful write and does not survive it.
func TestWriteFileAtomic_StaleTempFileIsConsumed(t *testing.T) {
d := t.TempDir()
fs := NewRealFileSystem(d)
// Stale temps in both the legacy deterministic pattern (older app
// versions) and the current unique pattern (another/crashed process).
stale := []string{".f.txt.tmp", ".f.txt.tmp.99999.7"}
for _, name := range stale {
if err := os.WriteFile(filepath.Join(d, name), []byte("stale garbage from a crashed write"), 0o644); err != nil {
t.Fatal(err)
}
}
// A different file's temp must be untouched.
if err := os.WriteFile(filepath.Join(d, ".other.txt.tmp.99999.8"), []byte("not ours"), 0o644); err != nil {
t.Fatal(err)
}
fresh := []byte("fresh content")
if err := fs.WriteFileAtomic("/f.txt", fresh); err != nil {
t.Fatal(err)
}
got, err := fs.ReadFile("/f.txt")
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(got, fresh) {
t.Fatalf("target file != fresh content: %q", got)
}
entries, err := os.ReadDir(d)
if err != nil {
t.Fatal(err)
}
seen := map[string]bool{}
for _, e := range entries {
seen[e.Name()] = true
}
if seen[".f.txt.tmp"] || seen[".f.txt.tmp.99999.7"] {
t.Fatalf("stale temp file(s) were not consumed by the successful write: %v", seen)
}
if !seen[".other.txt.tmp.99999.8"] {
t.Fatalf("another file's temp was removed: %v", seen)
}
}
func itoa(n int) string { return strconv.Itoa(n) }