diff --git a/.gitea/workflows/test-pr.yaml b/.gitea/workflows/test-pr.yaml index bb925aa..00ab8f2 100644 --- a/.gitea/workflows/test-pr.yaml +++ b/.gitea/workflows/test-pr.yaml @@ -4,6 +4,9 @@ on: push: branches: - main + paths-ignore: + - '**.md' + - 'CONTRIBUTING.md' jobs: check-and-test: diff --git a/vfs/disk/disk.go b/vfs/disk/disk.go index 8dd163c..35e4cba 100644 --- a/vfs/disk/disk.go +++ b/vfs/disk/disk.go @@ -248,15 +248,25 @@ func (d *DiskFS) calculateSizeAndPopulateIndex() { // insertBatch populates info/LRU under lock for a bounded batch (follows maxEvictBatch pattern for short critical sections). // Size is incremented here only for files actually added (prevents double-count vs. concurrent Create during window). +// Fail-closed: re-stat each path under d.mu and skip if the file is gone. Create does not wait on +// initDone, so a file the scanner observed can be Evict/Delete'd (info + os.Remove) before this +// insert runs. Inserting without a live-file check would resurrect the key in d.info and make +// Stat succeed while os.Stat fails. func (d *DiskFS) insertBatch(batch []discoveredFile) { d.mu.Lock() for _, df := range batch { - if _, exists := d.info[df.key]; !exists { - fi := vfs.NewFileInfoFromOS(df.osInfo, df.key) - d.info[df.key] = fi - d.LRU.Add(df.key, fi) - d.size += df.size + if _, exists := d.info[df.key]; exists { + continue } + path := d.pathForKey(df.key) + st, err := os.Stat(path) + if err != nil { + continue + } + fi := vfs.NewFileInfoFromOS(st, df.key) + d.info[df.key] = fi + d.LRU.Add(df.key, fi) + d.size += st.Size() } d.mu.Unlock() } diff --git a/vfs/disk/disk_test.go b/vfs/disk/disk_test.go index 287eb51..f77fc7a 100644 --- a/vfs/disk/disk_test.go +++ b/vfs/disk/disk_test.go @@ -371,7 +371,8 @@ func testKey(i int) string { // artifacts for victims are immediately gone (no resurrection via lazy discovery in Stat/Open), // and that recreating the same key produces independent content that is not subject to any // stale eviction unlinks. This exercises the coordinated WLock remove path for DiskFS. -// Uses tolerant checks suitable for raw DiskFS lazy discovery + bg size. +// Create does not wait on initDone, so this also covers insertBatch racing with eviction: +// gone files must not be re-indexed (Stat present / disk missing). func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { t.Parallel() td := t.TempDir() @@ -400,47 +401,21 @@ func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { _ = d.EvictBySize(1024*1024, true) } - // Consistency check: never have a key absent from Stat but with a file on disk (would indicate - // either resurrection risk or orphan). If Stat succeeds, file should exist. - // A few retries tolerate the documented lazy discovery + eviction coordination windows under - // artificial "force massive eviction then immediate audit" load (especially visible under -race). - for attempt := 0; attempt < 3; attempt++ { - bad := false - for _, k := range created { - p := d.pathForKey(k) - _, statErr := d.Stat(k) - _, diskErr := os.Stat(p) - if statErr != nil { - if !os.IsNotExist(diskErr) { - bad = true - } - } else { - if diskErr != nil { - bad = true - } - } - } - if !bad { - break - } - if attempt < 2 { - time.Sleep(10 * time.Millisecond) - } else { - // On final attempt, report the last observed state for the keys - for _, k := range created { - p := d.pathForKey(k) - _, statErr := d.Stat(k) - _, diskErr := os.Stat(p) - if statErr != nil { - if !os.IsNotExist(diskErr) { - t.Errorf("key %s absent via Stat but file lingers on disk at %s (resurrection risk)", k, p) - } - } else { - if diskErr != nil { - t.Errorf("key %s present via Stat but missing on disk: %v", k, diskErr) - } - } + // Drain bg population so insertBatch cannot still be in flight when we audit. + _ = d.Size() + + // Consistency: Stat success iff the file exists on disk. insertBatch must not resurrect + // keys whose backing files were already evicted. + for _, k := range created { + p := d.pathForKey(k) + _, statErr := d.Stat(k) + _, diskErr := os.Stat(p) + if statErr != nil { + if !os.IsNotExist(diskErr) { + t.Errorf("key %s absent via Stat but file lingers on disk at %s (resurrection risk)", k, p) } + } else if diskErr != nil { + t.Errorf("key %s present via Stat but missing on disk: %v", k, diskErr) } } @@ -464,6 +439,64 @@ func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { } } +// TestDiskFS_InsertBatchSkipsGoneFiles is the fail-closed contract for bg/lazy index +// insert: a discoveredFile whose path was removed (evicted) must not be re-inserted +// into d.info. That resurrection is what made Stat succeed while os.Stat failed. +func TestDiskFS_InsertBatchSkipsGoneFiles(t *testing.T) { + t.Parallel() + td := t.TempDir() + d, err := New(td, 10*1024*1024, nil) + if err != nil { + t.Fatal(err) + } + _ = d.Size() // finish constructor scan so it cannot also index these keys + + liveKey := "live" + goneKey := "gone" + writeKey := func(key, body string) os.FileInfo { + t.Helper() + p := d.pathForKey(key) + if err := os.MkdirAll(filepath.Dir(p), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0600); err != nil { + t.Fatal(err) + } + st, err := os.Stat(p) + if err != nil { + t.Fatal(err) + } + return st + } + liveInfo := writeKey(liveKey, "still-here") + goneInfo := writeKey(goneKey, "about-to-vanish") + if err := os.Remove(d.pathForKey(goneKey)); err != nil { + t.Fatal(err) + } + + d.insertBatch([]discoveredFile{ + {key: liveKey, size: liveInfo.Size(), osInfo: liveInfo}, + {key: goneKey, size: goneInfo.Size(), osInfo: goneInfo}, + }) + + d.mu.RLock() + _, liveExists := d.info[liveKey] + _, goneExists := d.info[goneKey] + d.mu.RUnlock() + if !liveExists { + t.Errorf("insertBatch skipped live key %s", liveKey) + } + if goneExists { + t.Errorf("insertBatch resurrected gone key %s", goneKey) + } + if _, err := d.Stat(goneKey); err == nil { + t.Errorf("Stat succeeded for gone key %s", goneKey) + } + if _, err := os.Stat(d.pathForKey(liveKey)); err != nil { + t.Errorf("live key %s missing on disk: %v", liveKey, err) + } +} + // TestDiskFS_EvictBoundedLargeN exercises the maxEvictBatch early-break logic (Idea #2) // under a map size >> batch limit. Forces repeated eviction rounds via GC-style pressure // and asserts progress + consistency (no resurrection/orphans). Covers bounded collection