From 529988d62c1ffc3e5332231fc3e977858e5a2351 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 11 Feb 2020 17:49:52 -0800 Subject: [PATCH] os: seek should invalidate any cached directory reads When we seek on the underlying FD, discard any directory entries we've already read and cached. This makes sure we won't return the same entry twice. We already fixed this for Darwin in CL 209961. Fixes #37161 Change-Id: I20e1ac8d751443135e67fb4c43c18d69befb643b Reviewed-on: https://go-review.googlesource.com/c/go/+/219143 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Emmanuel Odeke Reviewed-by: Brad Fitzpatrick --- src/os/dir_darwin.go | 10 ---------- src/os/dir_unix.go | 2 -- src/os/file_unix.go | 7 ++++++- src/os/os_test.go | 31 +++++++++++++++++++++++++++++++ src/os/testdata/issue37161/a | 1 + src/os/testdata/issue37161/b | 1 + src/os/testdata/issue37161/c | 1 + 7 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 src/os/testdata/issue37161/a create mode 100644 src/os/testdata/issue37161/b create mode 100644 src/os/testdata/issue37161/c diff --git a/src/os/dir_darwin.go b/src/os/dir_darwin.go index a274dd1268..2f9ba78d68 100644 --- a/src/os/dir_darwin.go +++ b/src/os/dir_darwin.go @@ -24,16 +24,6 @@ func (d *dirInfo) close() { d.dir = 0 } -func (f *File) seekInvalidate() { - if f.dirinfo == nil { - return - } - // Free cached dirinfo, so we allocate a new one if we - // access this file as a directory again. See #35767. - f.dirinfo.close() - f.dirinfo = nil -} - func (f *File) readdirnames(n int) (names []string, err error) { if f.dirinfo == nil { dir, call, errno := f.pfd.OpenDir() diff --git a/src/os/dir_unix.go b/src/os/dir_unix.go index 2856a2dc0f..e0c4989756 100644 --- a/src/os/dir_unix.go +++ b/src/os/dir_unix.go @@ -26,8 +26,6 @@ const ( func (d *dirInfo) close() {} -func (f *File) seekInvalidate() {} - func (f *File) readdirnames(n int) (names []string, err error) { // If this file has no dirinfo, create one. if f.dirinfo == nil { diff --git a/src/os/file_unix.go b/src/os/file_unix.go index 6945937fd6..32e4442e5d 100644 --- a/src/os/file_unix.go +++ b/src/os/file_unix.go @@ -295,7 +295,12 @@ func (f *File) pwrite(b []byte, off int64) (n int, err error) { // relative to the current offset, and 2 means relative to the end. // It returns the new offset and an error, if any. func (f *File) seek(offset int64, whence int) (ret int64, err error) { - f.seekInvalidate() + if f.dirinfo != nil { + // Free cached dirinfo, so we allocate a new one if we + // access this file as a directory again. See #35767 and #37161. + f.dirinfo.close() + f.dirinfo = nil + } ret, err = f.pfd.Seek(offset, whence) runtime.KeepAlive(f) return ret, err diff --git a/src/os/os_test.go b/src/os/os_test.go index cc03b91d72..44e1434dbe 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -2496,3 +2496,34 @@ func TestDirSeek(t *testing.T) { } } } + +func TestReaddirSmallSeek(t *testing.T) { + // See issue 37161. Read only one entry from a directory, + // seek to the beginning, and read again. We should not see + // duplicate entries. + if runtime.GOOS == "windows" { + testenv.SkipFlaky(t, 36019) + } + wd, err := Getwd() + if err != nil { + t.Fatal(err) + } + df, err := Open(filepath.Join(wd, "testdata", "issue37161")) + if err != nil { + t.Fatal(err) + } + names1, err := df.Readdirnames(1) + if err != nil { + t.Fatal(err) + } + if _, err = df.Seek(0, 0); err != nil { + t.Fatal(err) + } + names2, err := df.Readdirnames(0) + if err != nil { + t.Fatal(err) + } + if len(names2) != 3 { + t.Fatalf("first names: %v, second names: %v", names1, names2) + } +} diff --git a/src/os/testdata/issue37161/a b/src/os/testdata/issue37161/a new file mode 100644 index 0000000000..7898192261 --- /dev/null +++ b/src/os/testdata/issue37161/a @@ -0,0 +1 @@ +a diff --git a/src/os/testdata/issue37161/b b/src/os/testdata/issue37161/b new file mode 100644 index 0000000000..6178079822 --- /dev/null +++ b/src/os/testdata/issue37161/b @@ -0,0 +1 @@ +b diff --git a/src/os/testdata/issue37161/c b/src/os/testdata/issue37161/c new file mode 100644 index 0000000000..f2ad6c76f0 --- /dev/null +++ b/src/os/testdata/issue37161/c @@ -0,0 +1 @@ +c -- 2.50.0