ipfs get race conditions

[chriscool]

Some tests in t0090-get.sh have been failing randomly for some time.

And when building ipfs with -race and trying the following (which is part of t0090-get.sh):

echo "Hello Worlds!" >data &&
HASH=`ipfs add -q data` &&
ipfs get "$HASH" >actual

one get a lot of debug output and Found 8 data race(s).

[whyrusleeping]

👍 always good to flush these out.

[jbenet]

if still have the outputs of the data races, it would be useful to post them here to make sure we account for them all, and can discuss them. (sometimes it can be hard to get the same races to appear across platforms)

[chriscool]

The file with the -race output is 2411 lines long and GitHub only supports image file uploads, so I added it to ipfs:

http://gateway.ipfs.io/ipfs/Qmagf4SGLwjDUgQ4TEvDGPKJiSmN1DHwGfxwXj3T8bsvac

I hope it won't disappear if I disconnect.

[jbenet]

i just pinned it on a server i run. we need cube (our s3 pinner) soon

[chriscool]

@jbenet thanks, it looks like the problems are in unixfs/tar/reader.go
cc @whyrusleeping @mappum as they worked on it.

[whyrusleeping]

@chriscool git is lying to you. I've nothing to do with that code (but thats not to say i cant look at it)

[jbenet]

@whyrusleeping hmmmmmmm

> git praise unixfs/tar/reader.go | grep Jeromy | wc -l
50
> git praise unixfs/tar/reader.go |  wc
210
> echo "print 50.0 / 210" | python
0.238095238095

[whyrusleeping]

I must have written that in my sleep, because i seriously have no recollection of writing any of that...

[chriscool]

I just tried again to see if the races are still there and it looks like there are fewer of them.
I ran:

make GOFLAGS=-race verbose=t t0090-get.sh

and I got output like that:

expecting success: 
     ipfs get "$HASH2" -a -C >actual

216 B 0                                                                                                                                           ==================
WARNING: DATA RACE
Read by goroutine 17:
  github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb.(*ProgressBar).writer()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb/pb.go:328 +0x3b

Previous write by main goroutine:
  github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb.(*ProgressBar).Finish()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb/pb.go:189 +0x3b
  github.com/ipfs/go-ipfs/core/commands.func·039()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/core/commands/get.go:120 +0x95d
  main.callCommand()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:344 +0xc0d
  main.(*cmdInvocation).Run()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:177 +0x28b
  main.main()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:145 +0x88c

Goroutine 17 (running) created at:
  github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb.(*ProgressBar).Start()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/cheggaaa/pb/pb.go:101 +0x143
  github.com/ipfs/go-ipfs/core/commands.func·039()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/core/commands/get.go:111 +0x845
  main.callCommand()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:344 +0xc0d
  main.(*cmdInvocation).Run()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:177 +0x28b
  main.main()
      /home/christian/gocode/src/github.com/ipfs/go-ipfs/cmd/ipfs/main.go:145 +0x88c
==================
Found 1 data race(s)

[chriscool]

Ok, the data race above seems to be one in pb, so I opened an issue there: cheggaaa/pb#40

[chriscool]

Well the race has been fixed in the last versions of pb, so the good news is that we just have to upgrade to the latest version to fix this race.

[chriscool]

I will left it to someone used to vendoring dependencies, as it looks like I haven't figured out exactly how to do it yet and it 's late now. Thanks in advance!

[cryptix]

I can do some godeps housecleaning, no problem.

[chriscool]

@cryptix that would be great if you could do it, thanks!

[jbenet]

@cryptix did you fix this one (cheggaaa/pb#40) already?

[cryptix]

Hrmpf, I hoped it would be fixed by upgrading to master but I just ran the tests with race like @chriscool did and got this.

After looking at it a bit, I guess this could be on our side now, though...

[chriscool]

Yeah, upgrading cheggaaa/pb to master doesn't fix all the races, but it fixes many of them.
The one you got is definitely on our side and is also there now before upgrading.

[cryptix]

Okay, cool. Thanks for confirming that there are less of them now. :)

[chriscool]

Thanks for upgrading cheggaaa/pb, I had not seen that you did it!

[jbenet]

not sure if this will be reverted by the recent #1323 -- something went wrong.

[whyrusleeping]

I think this is fixed? can anyone confirm? @cryptix @chriscool

[chriscool]

I confirm that it is fixed.

To check by yourself you just need to run make GOFLAGS=-race verbose=t deps t0090-get.sh in test/sharness.

[whyrusleeping]

awesome, thanks!