From 4ce35d3d17b8a1483d42dc46d010067c4fcce8a8 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Sat, 17 Oct 2020 15:31:40 -0600 Subject: [PATCH] Fix race in filesystem_test.go --- server/filesystem/disk_space.go | 3 +- server/filesystem/filesystem_test.go | 48 +++++++++++++++------------- server/filesystem/stat_arm.go | 2 +- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/server/filesystem/disk_space.go b/server/filesystem/disk_space.go index 3c7ef79..9efd2cc 100644 --- a/server/filesystem/disk_space.go +++ b/server/filesystem/disk_space.go @@ -209,7 +209,8 @@ func (fs *Filesystem) hasSpaceFor(size int64) error { // Updates the disk usage for the Filesystem instance. func (fs *Filesystem) addDisk(i int64) int64 { - var size = atomic.LoadInt64(&fs.diskUsed) + size := atomic.LoadInt64(&fs.diskUsed) + // Sorry go gods. This is ugly but the best approach I can come up with for right // now without completely re-evaluating the logic we use for determining disk space. // diff --git a/server/filesystem/filesystem_test.go b/server/filesystem/filesystem_test.go index 0ee8931..c472dcd 100644 --- a/server/filesystem/filesystem_test.go +++ b/server/filesystem/filesystem_test.go @@ -9,6 +9,7 @@ import ( "math/rand" "os" "path/filepath" + "sync/atomic" "testing" "unicode/utf8" ) @@ -329,7 +330,7 @@ func TestFilesystem_Readfile(t *testing.T) { g.AfterEach(func() { buf.Truncate(0) - fs.diskUsed = 0 + atomic.StoreInt64(&fs.diskUsed, 0) rfs.reset() }) }) @@ -347,7 +348,7 @@ func TestFilesystem_Writefile(t *testing.T) { g.It("can create a new file", func() { r := bytes.NewReader([]byte("test file content")) - g.Assert(fs.diskUsed).Equal(int64(0)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0)) err := fs.Writefile("test.txt", r) g.Assert(err).IsNil() @@ -355,7 +356,7 @@ func TestFilesystem_Writefile(t *testing.T) { err = fs.Readfile("test.txt", buf) g.Assert(err).IsNil() g.Assert(buf.String()).Equal("test file content") - g.Assert(fs.diskUsed).Equal(r.Size()) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(r.Size()) }) g.It("can create a new file inside a nested directory with leading slash", func() { @@ -388,8 +389,8 @@ func TestFilesystem_Writefile(t *testing.T) { g.Assert(errors.Is(err, ErrBadPathResolution)).IsTrue() }) - g.It("cannot write a file that exceedes the disk limits", func() { - fs.diskLimit = 1024 + g.It("cannot write a file that exceeds the disk limits", func() { + atomic.StoreInt64(&fs.diskLimit, 1024) b := make([]byte, 1025) _, err := rand.Read(b) @@ -403,7 +404,7 @@ func TestFilesystem_Writefile(t *testing.T) { }) g.It("updates the total space used when a file is appended to", func() { - fs.diskUsed = 100 + atomic.StoreInt64(&fs.diskUsed, 100) b := make([]byte, 100) _, _ = rand.Read(b) @@ -411,7 +412,7 @@ func TestFilesystem_Writefile(t *testing.T) { r := bytes.NewReader(b) err := fs.Writefile("test.txt", r) g.Assert(err).IsNil() - g.Assert(fs.diskUsed).Equal(int64(200)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(200)) // If we write less data than already exists, we should expect the total // disk used to be decremented. @@ -421,7 +422,7 @@ func TestFilesystem_Writefile(t *testing.T) { r = bytes.NewReader(b) err = fs.Writefile("test.txt", r) g.Assert(err).IsNil() - g.Assert(fs.diskUsed).Equal(int64(150)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(150)) }) g.It("truncates the file when writing new contents", func() { @@ -441,8 +442,9 @@ func TestFilesystem_Writefile(t *testing.T) { g.AfterEach(func() { buf.Truncate(0) rfs.reset() - fs.diskUsed = 0 - fs.diskLimit = 0 + + atomic.StoreInt64(&fs.diskUsed, 0) + atomic.StoreInt64(&fs.diskLimit, 0) }) }) } @@ -481,7 +483,7 @@ func TestFilesystem_CreateDirectory(t *testing.T) { g.It("should not increment the disk usage", func() { err := fs.CreateDirectory("test", "/") g.Assert(err).IsNil() - g.Assert(fs.diskUsed).Equal(int64(0)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0)) }) g.AfterEach(func() { @@ -597,7 +599,7 @@ func TestFilesystem_Copy(t *testing.T) { panic(err) } - fs.diskUsed = int64(utf8.RuneCountInString("test content")) + atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content"))) }) g.It("should return an error if the source does not exist", func() { @@ -640,7 +642,7 @@ func TestFilesystem_Copy(t *testing.T) { }) g.It("should return an error if there is not space to copy the file", func() { - fs.diskLimit = 2 + atomic.StoreInt64(&fs.diskLimit, 2) err := fs.Copy("source.txt") g.Assert(err).IsNotNil() @@ -672,7 +674,7 @@ func TestFilesystem_Copy(t *testing.T) { g.Assert(err).IsNil() } - g.Assert(fs.diskUsed).Equal(int64(utf8.RuneCountInString("test content")) * 3) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(utf8.RuneCountInString("test content")) * 3) }) g.It("should create a copy inside of a directory", func() { @@ -694,8 +696,9 @@ func TestFilesystem_Copy(t *testing.T) { g.AfterEach(func() { rfs.reset() - fs.diskUsed = 0 - fs.diskLimit = 0 + + atomic.StoreInt64(&fs.diskUsed, 0) + atomic.StoreInt64(&fs.diskLimit, 0) }) }) } @@ -710,7 +713,7 @@ func TestFilesystem_Delete(t *testing.T) { panic(err) } - fs.diskUsed = int64(utf8.RuneCountInString("test content")) + atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content"))) }) g.It("does not delete files outside the root directory", func() { @@ -744,7 +747,7 @@ func TestFilesystem_Delete(t *testing.T) { g.Assert(err).IsNotNil() g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue() - g.Assert(fs.diskUsed).Equal(int64(0)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0)) }) g.It("deletes all items inside a directory if the directory is deleted", func() { @@ -762,11 +765,11 @@ func TestFilesystem_Delete(t *testing.T) { g.Assert(err).IsNil() } - fs.diskUsed = int64(utf8.RuneCountInString("test content") * 3) + atomic.StoreInt64(&fs.diskUsed, int64(utf8.RuneCountInString("test content")*3)) err = fs.Delete("foo") g.Assert(err).IsNil() - g.Assert(fs.diskUsed).Equal(int64(0)) + g.Assert(atomic.LoadInt64(&fs.diskUsed)).Equal(int64(0)) for _, s := range sources { _, err = rfs.StatServerFile(s) @@ -777,8 +780,9 @@ func TestFilesystem_Delete(t *testing.T) { g.AfterEach(func() { rfs.reset() - fs.diskUsed = 0 - fs.diskLimit = 0 + + atomic.StoreInt64(&fs.diskUsed, 0) + atomic.StoreInt64(&fs.diskLimit, 0) }) }) } diff --git a/server/filesystem/stat_arm.go b/server/filesystem/stat_arm.go index 2e2918b..21e75e3 100644 --- a/server/filesystem/stat_arm.go +++ b/server/filesystem/stat_arm.go @@ -10,4 +10,4 @@ func (s *Stat) CTime() time.Time { st := s.Info.Sys().(*syscall.Stat_t) return time.Unix(int64(st.Ctim.Sec), int64(st.Ctim.Nsec)) -} \ No newline at end of file +}