Merge pull request #28 from pjbgf/fix-go-git-data-race

Fix data race when running `go-git` tests
go-git · Dec 19, 2022 · 4e5a841 · 4e5a841
2 parents 7ab80d7 + 38b02ce
commit 4e5a841
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 44 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -1,18 +1,19 @@
 on: [push, pull_request]
 name: Test
+permissions: {}
 jobs:
   test:
     strategy:
       matrix:
-        go-version: [1.14.x, 1.15.x, 1.16.x]
+        go-version: [1.18.x,1.19.x]
         platform: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.platform }}
     steps:
     - name: Install Go
-      uses: actions/setup-go@v1
+      uses: actions/setup-go@v3
       with:
         go-version: ${{ matrix.go-version }}
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
     - name: Test
-      run: go test ./...
+      run: make test
diff --git a/.github/workflows/test_js.yml b/.github/workflows/test_js.yml
@@ -1,24 +1,25 @@
 on: [push, pull_request]
-name: Test
+name: Test JS
+permissions: {}
 jobs:
   test:
     strategy:
       matrix:
-        go-version: [1.14.x, 1.15.x, 1.16.x]
+        go-version: [1.18.x,1.19.x]
     runs-on: ubuntu-latest
     steps:
     - name: Install Go
-      uses: actions/setup-go@v1
+      uses: actions/setup-go@v3
       with:
         go-version: ${{ matrix.go-version }}
 
     - name: Install wasmbrowsertest
       run: |
-        go get github.com/agnivade/wasmbrowsertest
+        go install github.com/agnivade/wasmbrowsertest@latest
         mv $HOME/go/bin/wasmbrowsertest $HOME/go/bin/go_js_wasm_exec
 
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: Test
       run: go test -exec="$HOME/go/bin/go_js_wasm_exec" ./...

diff --git a/Makefile b/Makefile
@@ -0,0 +1,11 @@
+# Go parameters
+GOCMD = go
+GOTEST = $(GOCMD) test 
+
+.PHONY: test
+test:
+	$(GOTEST) -race ./...
+
+test-coverage:
+	echo "" > $(COVERAGE_REPORT); \
+	$(GOTEST) -coverprofile=$(COVERAGE_REPORT) -coverpkg=./... -covermode=$(COVERAGE_MODE) ./...
diff --git a/go.mod b/go.mod
@@ -3,8 +3,8 @@ module github.com/go-git/go-billy/v5
 require (
 	github.com/kr/text v0.2.0 // indirect
 	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
-	golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527
-	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f
+	golang.org/x/sys v0.3.0
+	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
 )
 
 go 1.13
diff --git a/go.sum b/go.sum
@@ -1,6 +1,8 @@
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/go-git/go-billy v1.0.0 h1:bXR6Zu3opPSg0R4dDxqaLglY4rxw7ja7wS16qSpOKL4=
 github.com/go-git/go-billy v3.1.0+incompatible h1:dwrJ8G2Jt1srYgIJs+lRjA36qBY68O2Lg5idKG8ef5M=
+github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
+github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
@@ -10,5 +12,11 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 h1:uYVVQ9WP/Ds2ROhcaGPeIdVq0RIXVLwsHlnvJ+cT1So=
 golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
+golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=
+golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
diff --git a/memfs/storage.go b/memfs/storage.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sync"
 )
 
 type storage struct {
@@ -174,6 +175,8 @@ func clean(path string) string {
 type content struct {
 	name  string
 	bytes []byte
+
+	m sync.RWMutex
 }
 
 func (c *content) WriteAt(p []byte, off int64) (int, error) {
@@ -185,6 +188,7 @@ func (c *content) WriteAt(p []byte, off int64) (int, error) {
 		}
 	}
 
+	c.m.Lock()
 	prev := len(c.bytes)
 
 	diff := int(off) - prev
@@ -196,6 +200,7 @@ func (c *content) WriteAt(p []byte, off int64) (int, error) {
 	if len(c.bytes) < prev {
 		c.bytes = c.bytes[:prev]
 	}
+	c.m.Unlock()
 
 	return len(p), nil
 }
@@ -209,8 +214,10 @@ func (c *content) ReadAt(b []byte, off int64) (n int, err error) {
 		}
 	}
 
+	c.m.RLock()
 	size := int64(len(c.bytes))
 	if off >= size {
+		c.m.RUnlock()
 		return 0, io.EOF
 	}
 
@@ -220,10 +227,12 @@ func (c *content) ReadAt(b []byte, off int64) (n int, err error) {
 	}
 
 	btr := c.bytes[off : off+l]
+	n = copy(b, btr)
+
 	if len(btr) < len(b) {
 		err = io.EOF
 	}
-	n = copy(b, btr)
+	c.m.RUnlock()
 
 	return
 }
diff --git a/util/walk_test.go b/util/walk_test.go
@@ -20,6 +20,7 @@ type WalkSuite struct{}
 func TestWalk(t *testing.T) { TestingT(t) }
 
 var _ = Suite(&WalkSuite{})
+var targetSubfolder = filepath.FromSlash("path/to/some/subfolder")
 
 func (s *WalkSuite) TestWalkCanSkipTopDirectory(c *C) {
 	filesystem := memfs.New()
@@ -52,13 +53,13 @@ func (s *WalkSuite) TestWalkOnExistingFolder(c *C) {
 		return nil
 	}), IsNil)
 	c.Assert(discoveredPaths, Contains, "path")
-	c.Assert(discoveredPaths, Contains, "path/to")
-	c.Assert(discoveredPaths, Contains, "path/to/some")
-	c.Assert(discoveredPaths, Contains, "path/to/some/file")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that/contain")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder/that/contain/file")
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that/contain"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
 }
 
 func (s *WalkSuite) TestWalkCanSkipFolder(c *C) {
@@ -68,19 +69,19 @@ func (s *WalkSuite) TestWalkCanSkipFolder(c *C) {
 	discoveredPaths := []string{}
 	c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
 		discoveredPaths = append(discoveredPaths, path)
-		if path == "path/to/some/subfolder" {
+		if path == targetSubfolder {
 			return filepath.SkipDir
 		}
 		return nil
 	}), IsNil)
 	c.Assert(discoveredPaths, Contains, "path")
-	c.Assert(discoveredPaths, Contains, "path/to")
-	c.Assert(discoveredPaths, Contains, "path/to/some")
-	c.Assert(discoveredPaths, Contains, "path/to/some/file")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
 }
 
 func (s *WalkSuite) TestWalkStopsOnError(c *C) {
@@ -90,27 +91,27 @@ func (s *WalkSuite) TestWalkStopsOnError(c *C) {
 	discoveredPaths := []string{}
 	c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
 		discoveredPaths = append(discoveredPaths, path)
-		if path == "path/to/some/subfolder" {
+		if path == targetSubfolder {
 			return errors.New("uncaught error")
 		}
 		return nil
 	}), NotNil)
 	c.Assert(discoveredPaths, Contains, "path")
-	c.Assert(discoveredPaths, Contains, "path/to")
-	c.Assert(discoveredPaths, Contains, "path/to/some")
-	c.Assert(discoveredPaths, Contains, "path/to/some/file")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
 }
 
 func (s *WalkSuite) TestWalkForwardsStatErrors(c *C) {
 	memFilesystem := memfs.New()
 	filesystem := &fnFs{
 		Filesystem: memFilesystem,
 		lstat: func(path string) (os.FileInfo, error) {
-			if path == "path/to/some/subfolder" {
+			if path == targetSubfolder {
 				return nil, errors.New("uncaught error")
 			}
 			return memFilesystem.Lstat(path)
@@ -122,19 +123,19 @@ func (s *WalkSuite) TestWalkForwardsStatErrors(c *C) {
 	discoveredPaths := []string{}
 	c.Assert(util.Walk(filesystem, "path", func(path string, info os.FileInfo, err error) error {
 		discoveredPaths = append(discoveredPaths, path)
-		if path == "path/to/some/subfolder" {
+		if path == targetSubfolder {
 			c.Assert(err, NotNil)
 		}
 		return err
 	}), NotNil)
 	c.Assert(discoveredPaths, Contains, "path")
-	c.Assert(discoveredPaths, Contains, "path/to")
-	c.Assert(discoveredPaths, Contains, "path/to/some")
-	c.Assert(discoveredPaths, Contains, "path/to/some/file")
-	c.Assert(discoveredPaths, Contains, "path/to/some/subfolder")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain")
-	c.Assert(discoveredPaths, NotContain, "path/to/some/subfolder/that/contain/file")
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/file"))
+	c.Assert(discoveredPaths, Contains, filepath.FromSlash("path/to/some/subfolder"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain"))
+	c.Assert(discoveredPaths, NotContain, filepath.FromSlash("path/to/some/subfolder/that/contain/file"))
 }
 
 func createFile(c *C, filesystem billy.Filesystem, path string) {