cavaliergopher / grab

A download manager package for Go
BSD 3-Clause "New" or "Revised" License
1.38k stars 151 forks source link

Data race #26

Closed HeavyHorst closed 6 years ago

HeavyHorst commented 6 years ago

I just ran the example on the start page with the race detector.

Downloading http://www.golang-book.com/public/pdf/gobook.pdf...
  200 OK
==================
WARNING: DATA RACE
Read at 0x00c420136298 by main goroutine:
  github.com/cavaliercoder/grab.(*Response).BytesComplete()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/response.go:133 +0x43
  main.main()
      /home/rkaufmann/Downloads/grab.go:30 +0x442

Previous write at 0x00c420136298 by goroutine 14:
  [failed to restore the stack]

Goroutine 14 (running) created at:
  github.com/cavaliercoder/grab.(*Client).Do()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/client.go:81 +0x451
  main.main()
      /home/rkaufmann/Downloads/grab.go:18 +0x325
==================
==================
WARNING: DATA RACE
Write at 0x00c420076180 by main goroutine:
  sync/atomic.CompareAndSwapInt32()
      /usr/local/go/src/runtime/race_amd64.s:293 +0xb
  sync.(*Mutex).Lock()
      /usr/local/go/src/sync/mutex.go:74 +0x4d
  github.com/cavaliercoder/grab.(*transfer).N()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/transfer.go:74 +0x4a
  github.com/cavaliercoder/grab.(*Response).BytesComplete()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/response.go:133 +0x58
  main.main()
      /home/rkaufmann/Downloads/grab.go:30 +0x442

Previous write at 0x00c420076180 by goroutine 14:
  [failed to restore the stack]

Goroutine 14 (running) created at:
  github.com/cavaliercoder/grab.(*Client).Do()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/client.go:81 +0x451
  main.main()
      /home/rkaufmann/Downloads/grab.go:18 +0x325
==================
  transferred 1156706 / 0 bytes 2893557 bps (39.98%)
  transferred 2617418 / 0 bytes 2893557 bps (90.46%)
Download saved to ./gobook.pdf
Found 2 data race(s)
exit status 66
cavaliercoder commented 6 years ago

Thanks for logging this. Unfortunately, I can't replicate the race detector output. Can you please what Go compiler version you are using and how you built the example to generate this output?

cavaliercoder commented 6 years ago

Though, I think I've found the culprit. I'm not protecting Response.transfer. I'll try undo this mistake.

HeavyHorst commented 6 years ago

Thanks for taking a look! At the moment I can't reproduce the second race either.

uname -a                                                                                                                                                                                                                
Linux chefkaufmann 4.15.13-300.fc27.x86_64 #1 SMP Mon Mar 26 19:06:57 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

go version
go version go1.10.1 linux/amd64
go build -race grab.go (grab.go is the example from the Readme)
./grab
Downloading http://www.golang-book.com/public/pdf/gobook.pdf...
  200 OK
==================
WARNING: DATA RACE
Read at 0x00c420110298 by main goroutine:
  github.com/cavaliercoder/grab.(*Response).BytesComplete()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/response.go:133 +0x43
  main.main()
      /home/rkaufmann/Downloads/grab.go:30 +0x442

Previous write at 0x00c420110298 by goroutine 14:
  [failed to restore the stack]

Goroutine 14 (running) created at:
  github.com/cavaliercoder/grab.(*Client).Do()
      /home/rkaufmann/go/src/github.com/cavaliercoder/grab/client.go:81 +0x451
  main.main()
      /home/rkaufmann/Downloads/grab.go:18 +0x325
==================
  transferred 1216238 / 2893557 bytes (42.03%)
  transferred 2674046 / 2893557 bytes (92.41%)
Download saved to ./gobook.pdf
Found 1 data race(s)
cmaglie commented 6 years ago

Though, I think I've found the culprit. I'm not protecting Response.transfer. I'll try undo this mistake.

This protection of Response.transfer should be fixed by #28 (but I don't know if it's the reason of the reported data race).