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).
205 lines
6.0 KiB
Go
205 lines
6.0 KiB
Go
package e2e_test
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"pad/internal/editor"
|
|
"pad/internal/io/pool"
|
|
"pad/internal/io/pool/real"
|
|
"pad/internal/io/pool/types"
|
|
"pad/internal/test/e2e"
|
|
)
|
|
|
|
// slowCountingFS wraps a pool.FileSystem, delaying each save by delay and
|
|
// recording the peak number of concurrent saves. A peak > 1 is exactly the
|
|
// same-file write concurrency that the write protocol must eliminate (two
|
|
// concurrent writes would otherwise race on the per-file staging path and,
|
|
// pre-unique-temp, could interleave byte-mixtures onto disk).
|
|
//
|
|
// NOTE: the pool's WriteFileTask calls FS.WriteFile (the real FS implements
|
|
// that as an atomic write via delegation to WriteFileAtomic), while
|
|
// FlushAll calls WriteFileAtomic directly — so both entry points are
|
|
// intercepted here, mirroring the real FS's delegation.
|
|
type slowCountingFS struct {
|
|
inner pool.FileSystem
|
|
delay time.Duration
|
|
|
|
mu sync.Mutex
|
|
cur int
|
|
max int
|
|
}
|
|
|
|
func (f *slowCountingFS) begin() {
|
|
f.mu.Lock()
|
|
f.cur++
|
|
if f.cur > f.max {
|
|
f.max = f.cur
|
|
}
|
|
f.mu.Unlock()
|
|
time.Sleep(f.delay)
|
|
}
|
|
|
|
func (f *slowCountingFS) end() {
|
|
f.mu.Lock()
|
|
f.cur--
|
|
f.mu.Unlock()
|
|
}
|
|
|
|
func (f *slowCountingFS) current() int {
|
|
f.mu.Lock()
|
|
defer f.mu.Unlock()
|
|
return f.cur
|
|
}
|
|
|
|
func (f *slowCountingFS) peak() int {
|
|
f.mu.Lock()
|
|
defer f.mu.Unlock()
|
|
return f.max
|
|
}
|
|
|
|
func (f *slowCountingFS) ReadDir(path string) ([]types.DirEntry, error) {
|
|
return f.inner.ReadDir(path)
|
|
}
|
|
func (f *slowCountingFS) DirExists(path string) bool {
|
|
return f.inner.DirExists(path)
|
|
}
|
|
func (f *slowCountingFS) FileExists(path string) bool {
|
|
return f.inner.FileExists(path)
|
|
}
|
|
func (f *slowCountingFS) ReadFile(path string) ([]byte, error) {
|
|
return f.inner.ReadFile(path)
|
|
}
|
|
func (f *slowCountingFS) ReadFileAt(path string, offset, size int) ([]byte, error) {
|
|
return f.inner.ReadFileAt(path, offset, size)
|
|
}
|
|
func (f *slowCountingFS) WriteFile(path string, content []byte) error {
|
|
f.begin()
|
|
err := f.inner.WriteFile(path, content)
|
|
f.end()
|
|
return err
|
|
}
|
|
func (f *slowCountingFS) WriteFileAtomic(path string, content []byte) error {
|
|
f.begin()
|
|
err := f.inner.WriteFileAtomic(path, content)
|
|
f.end()
|
|
return err
|
|
}
|
|
func (f *slowCountingFS) DeleteFile(path string) error {
|
|
return f.inner.DeleteFile(path)
|
|
}
|
|
func (f *slowCountingFS) CreateDir(path string) error {
|
|
return f.inner.CreateDir(path)
|
|
}
|
|
|
|
// waitForDisk polls the on-disk file until it equals want (or the timeout).
|
|
// The final model is unique, so matching it proves the whole save chain
|
|
// (deferred re-issues included) has landed.
|
|
func waitForDisk(t *testing.T, path, want string, timeout time.Duration) {
|
|
t.Helper()
|
|
deadline := time.Now().Add(timeout)
|
|
for time.Now().Before(deadline) {
|
|
b, err := os.ReadFile(path)
|
|
if err == nil && string(b) == want {
|
|
return
|
|
}
|
|
time.Sleep(100 * time.Millisecond)
|
|
}
|
|
t.Fatalf("disk did not reach %q within %v (last read: %q)", want, timeout, readOrErr(path))
|
|
}
|
|
|
|
func readOrErr(path string) string {
|
|
b, err := os.ReadFile(path)
|
|
if err != nil {
|
|
return err.Error()
|
|
}
|
|
return string(b)
|
|
}
|
|
|
|
func newSerializationHarness(t *testing.T, name string, seed string, delay time.Duration) (*slowCountingFS, *e2e.Harness, string) {
|
|
t.Helper()
|
|
dir := t.TempDir()
|
|
diskPath := filepath.Join(dir, name)
|
|
if err := os.WriteFile(diskPath, []byte(seed), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
fs := &slowCountingFS{inner: real.NewRealFileSystem(dir), delay: delay}
|
|
h := e2e.NewHarness(e2e.WithFileSystem(fs, "/"))
|
|
h.Run()
|
|
t.Cleanup(h.Cleanup)
|
|
loadRealFile(t, h, name)
|
|
return fs, h, diskPath
|
|
}
|
|
|
|
// TestWriteSerialization_ConcurrentSavesSerialize drives two autosaves whose
|
|
// 2 s saves would overlap (edit 2 lands 0.5 s after save 1 starts) and
|
|
// proves the protocol never runs two same-file saves concurrently: save 2's
|
|
// token is deferred while save 1 is in flight, and the result handler
|
|
// re-issues it with the newer content. On the pre-fix code the two saves ran
|
|
// concurrently on the same deterministic temp file (peak 2, possible byte
|
|
// mixture on disk).
|
|
func TestWriteSerialization_ConcurrentSavesSerialize(t *testing.T) {
|
|
fs, h, diskPath := newSerializationHarness(t, "ser.txt", "original content\n", 2*time.Second)
|
|
|
|
// Edit 1: autosave debounce fires ~1 s later -> save W1 (in flight
|
|
// ~1.0 s .. ~3.0 s).
|
|
if err := h.WithState(func(st *editor.State) {
|
|
st.Editor.CursorPosition = 0
|
|
editor.HandleInsert("A")
|
|
}); err != nil {
|
|
t.Fatalf("edit 1: %v", err)
|
|
}
|
|
time.Sleep(1500 * time.Millisecond) // W1 is in flight now
|
|
|
|
// Edit 2 while W1 is in flight: its autosave token fires ~2.5 s, inside
|
|
// W1's window -> must be deferred, not dispatched concurrently.
|
|
if err := h.WithState(func(st *editor.State) {
|
|
st.Editor.CursorPosition = 1
|
|
editor.HandleInsert("B")
|
|
}); err != nil {
|
|
t.Fatalf("edit 2: %v", err)
|
|
}
|
|
|
|
const model = "ABoriginal content\n"
|
|
waitForDisk(t, diskPath, model, 20*time.Second)
|
|
|
|
if peak := fs.peak(); peak > 1 {
|
|
t.Fatalf("peak concurrent same-file saves = %d, want <= 1", peak)
|
|
}
|
|
}
|
|
|
|
// TestWriteSerialization_FlushDuringInFlight drives the GoToBrowser/Shutdown
|
|
// path (FlushAll) while a worker save is in flight: the flush must defer
|
|
// (no concurrent save), and the in-flight save's completion must re-issue the
|
|
// latest content so "last rename wins" coincides with "newest snapshot wins".
|
|
func TestWriteSerialization_FlushDuringInFlight(t *testing.T) {
|
|
fs, h, diskPath := newSerializationHarness(t, "flushser.txt", "base\n", 2*time.Second)
|
|
|
|
// Edit -> autosave token ~1 s -> save W1 (in flight ~1.0 s .. ~3.0 s).
|
|
if err := h.WithState(func(st *editor.State) {
|
|
st.Editor.CursorPosition = 0
|
|
editor.HandleInsert("X")
|
|
}); err != nil {
|
|
t.Fatalf("edit: %v", err)
|
|
}
|
|
time.Sleep(1500 * time.Millisecond) // W1 is in flight now
|
|
|
|
// Flush while W1 is in flight.
|
|
if err := h.Flush(); err != nil {
|
|
t.Fatalf("flush: %v", err)
|
|
}
|
|
if cur := fs.current(); cur != 1 {
|
|
t.Fatalf("Flush during in-flight save: concurrent saves = %d, want 1 (the in-flight one only)", cur)
|
|
}
|
|
|
|
const model = "Xbase\n"
|
|
waitForDisk(t, diskPath, model, 20*time.Second)
|
|
|
|
if peak := fs.peak(); peak > 1 {
|
|
t.Fatalf("peak concurrent same-file saves = %d, want <= 1", peak)
|
|
}
|
|
}
|