Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Race detected during PlainClone #351

Closed
kimh opened this issue Apr 20, 2017 · 3 comments
Closed

Race detected during PlainClone #351

kimh opened this issue Apr 20, 2017 · 3 comments
Assignees
@smola
Labels
bug

Comments

@kimh
Copy link

kimh commented Apr 20, 2017 •
edited
Loading

Go version: go version go1.8 darwin/amd64
go-git version: fresh install with go get gopkg.in/src-d/go-git.v4/...

I tested my code that calls PlainClone with -race option and race is detected.

This gist will demonstrate the issue.

Download the gist, replace TestSSHKey with your own ssh key and run it with go test -race will make the following error. Interesting thing is that race is not always detected depending on which repository to clone.

➜  go-git-race go test -race
Counting objects: 778, done.
Compressing objects: 100% (21/21), done.
==================
WARNING: DATA RACE
Read at 0x00c420239570 by goroutine 6:
  sync/atomic.LoadInt64()
      /usr/local/go/src/runtime/race_amd64.s:211 +0xb
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*syncedReader).Write.func1()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:180 +0x7e
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*syncedReader).Write()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:187 +0xe6
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*PackWriter).Write()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:95 +0x70
  io.copyBuffer()
      /usr/local/go/src/io/io.go:392 +0x27d
  io.Copy()
      /usr/local/go/src/io/io.go:360 +0x7e
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.UpdateObjectStorage()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/common.go:33 +0x168
  gopkg.in/src-d/go-git%2ev4.(*Remote).fetchPack()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/remote.go:226 +0x41e
  gopkg.in/src-d/go-git%2ev4.(*Remote).fetch()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/remote.go:167 +0x58c
  gopkg.in/src-d/go-git%2ev4.(*Repository).clone()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/repository.go:333 +0x3af
  gopkg.in/src-d/go-git%2ev4.PlainClone()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/repository.go:227 +0xbd
  _/Users/kimh/git/go-git-race.Clone()
      /Users/kimh/git/go-git-race/clone_test.go:68 +0x43f
  _/Users/kimh/git/go-git-race.TestClone()
      /Users/kimh/git/go-git-race/clone_test.go:78 +0x33
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:657 +0x107

Previous write at 0x00c420239570 by goroutine 17:
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*syncedReader).Seek()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:239 +0x144
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*bufferedSeeker).Seek()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/scanner.go:380 +0x1ee
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*teeReader).Seek()
      <autogenerated>:28 +0x7c
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Scanner).Seek()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/scanner.go:315 +0x118
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).DecodeObjectAt.func1()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:319 +0x5a
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).DecodeObjectAt()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:325 +0x141
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).recallByOffset()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:399 +0x223
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).fillOFSDeltaObjectContent()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:375 +0x3e9
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).decodeByHeader()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:279 +0x5ab
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).DecodeObject()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:214 +0xaa
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).decodeObjects()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:155 +0x4d
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).doDecode()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:145 +0x17c
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).Decode()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:129 +0xd5
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*PackWriter).buildIndex()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:64 +0x590

Goroutine 6 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:697 +0x543
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:882 +0xaa
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:657 +0x107
  testing.runTests()
      /usr/local/go/src/testing/testing.go:888 +0x4e0
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:822 +0x1c3
  main.main()
      _/Users/kimh/git/go-git-race/_test/_testmain.go:42 +0x20f

Goroutine 17 (running) created at:
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.newPackWrite()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/writers.go:52 +0x507
  gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit.(*DotGit).NewObjectPack()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit/dotgit.go:111 +0x59
  gopkg.in/src-d/go-git.v4/storage/filesystem.(*ObjectStorage).PackfileWriter()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/storage/filesystem/object.go:62 +0x50
  gopkg.in/src-d/go-git.v4/storage/filesystem.(*Storage).PackfileWriter()
      <autogenerated>:5 +0x58
  gopkg.in/src-d/go-git.v4/plumbing/format/packfile.UpdateObjectStorage()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/common.go:27 +0xc7
  gopkg.in/src-d/go-git%2ev4.(*Remote).fetchPack()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/remote.go:226 +0x41e
  gopkg.in/src-d/go-git%2ev4.(*Remote).fetch()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/remote.go:167 +0x58c
  gopkg.in/src-d/go-git%2ev4.(*Repository).clone()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/repository.go:333 +0x3af
  gopkg.in/src-d/go-git%2ev4.PlainClone()
      /Users/kimh/golang/src/gopkg.in/src-d/go-git.v4/repository.go:227 +0xbd
  _/Users/kimh/git/go-git-race.Clone()
      /Users/kimh/git/go-git-race/clone_test.go:68 +0x43f
  _/Users/kimh/git/go-git-race.TestClone()
      /Users/kimh/git/go-git-race/clone_test.go:78 +0x33
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:657 +0x107
==================
Total 778 (delta 12), reused 0 (delta 0), pack-reused 757
--- FAIL: TestClone (2.27s)
        testing.go:610: race detected during execution of test
FAIL
exit status 1
FAIL    _/Users/kimh/git/go-git-race    2.293s
@smola smola added the bug label Apr 26, 2017
@smola
Copy link
Collaborator

smola commented Apr 26, 2017

@kimh Thanks for reporting this issue. We'll look into it.

@smola
Copy link
Collaborator

smola commented Apr 27, 2017

I can reproduce in master with the following reduced test:

package git

import (
	"io/ioutil"
	"os"
	"testing"
)

// Race can be reproduced with this repo
var Repo = "git://github.com/kimh/trusty-test.git"

// But not with this repo
//var Repo = "git://github.com/octocat/hello-worId.git"

func TestCloneRace(t *testing.T) {
	url := Repo
	directory, _ := ioutil.TempDir("", "")
	defer os.RemoveAll(directory)

	_, err := PlainClone(directory, false, &CloneOptions{
		URL: url,
		Progress: os.Stdout,
	})

	if err != nil {
		t.Fatalf("error: %s", err)
	}
}

running with go test -race -v gopkg.in/src-d/go-git.v4 -run ^TestCloneRace$

@smola
Copy link
Collaborator

smola commented Jun 27, 2017

Fixed by #453

@smola smola assigned smola and unassigned mcuadros Jun 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@smola smola

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@smola @kimh @mcuadros