From 286ae6a1b4deeea8677614e15c37dde4e1796ff8 Mon Sep 17 00:00:00 2001 From: Lorenz Brun Date: Tue, 20 Sep 2022 00:31:56 +0200 Subject: [PATCH] vfs: fix directory cache serving stale data The VFS directory cache layer didn't update directory entry properties if they are reused after cache invalidation. Update them unconditionally as newDir sets them to the same value and setting a pointer is cheaper in both LoC as well as CPU cycles than a branch. Also add a test exercising this behavior. Fixes #6335 --- vfs/dir.go | 1 + vfs/dir_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/vfs/dir.go b/vfs/dir.go index c72494eee..a4a31c4b6 100644 --- a/vfs/dir.go +++ b/vfs/dir.go @@ -760,6 +760,7 @@ func (d *Dir) _readDirFromEntries(entries fs.DirEntries, dirTree dirtree.DirTree dir := node.(*Dir) dir.mu.Lock() dir.modTime = item.ModTime(context.TODO()) + dir.entry = item if dirTree != nil { err = dir._readDirFromDirTree(dirTree, when) if err != nil { diff --git a/vfs/dir_test.go b/vfs/dir_test.go index 555cacf1f..69e45c269 100644 --- a/vfs/dir_test.go +++ b/vfs/dir_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "runtime" "sort" "testing" "time" @@ -655,3 +656,34 @@ func TestDirFileOpen(t *testing.T) { require.NoError(t, err) assert.Equal(t, int64(12), fi.Size()) } + +func TestDirEntryModTimeInvalidation(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("dirent modtime is unreliable on Windows filesystems") + } + r, vfs := newTestVFS(t) + + // Needs to be less than 2x the wait time below, othewrwise the entry + // gets cleared out before it had a chance to be updated. + vfs.Opt.DirCacheTime = fs.Duration(50 * time.Millisecond) + + r.WriteObject(context.Background(), "dir/file1", "file1 contents", t1) + + node, err := vfs.Stat("dir") + require.NoError(t, err) + modTime1 := node.(*Dir).DirEntry().ModTime(context.Background()) + + // Wait some time, then write another file which must update ModTime of + // the directory. + time.Sleep(75 * time.Millisecond) + r.WriteObject(context.Background(), "dir/file2", "file2 contents", t2) + + node2, err := vfs.Stat("dir") + require.NoError(t, err) + modTime2 := node2.(*Dir).DirEntry().ModTime(context.Background()) + + // ModTime of directory must be different after second file was written. + if modTime1.Equal(modTime2) { + t.Error("ModTime not invalidated") + } +}