From ceef78ce4493f8d07f0ad2622a7d52ca98fb24f3 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") + } +}