Bug: Data race while downloading files using concurrency

[criscojocaru]

I came across data races while trying to download files using concurrent reads. I followed this tutorial https://sftptogo.com/blog/go-sftp/ while writing the code. I tested it using -race on different sftp servers and the warning was there. Files sizes were over 300 MB (downloading worked, the data race was just a warning). I also tried to download files of 4GB and I received fatal error because of the data race. I looked over the implementation of putChannel and getChannel and I do not understand how the data race can happen, as there are locks on these methods.

WARNING: DATA RACE Read at 0x00c00011a090 by goroutine 16: runtime.mapaccess2_fast32() /usr/local/Cellar/go/1.16.5/libexec/src/runtime/map_fast32.go:52 +0x0 github.com/pkg/sftp.(clientConn).getChannel() github.com/pkg/sftp@v1.13.4/conn.go:119 +0xf7 github.com/pkg/sftp.(clientConn).recv() github.com/pkg/sftp@v1.13.4/conn.go:87 +0x1cb github.com/pkg/sftp.(*clientConn).loop() github.com/pkg/sftp@v1.13.4/conn.go:66 +0x76

Previous write at 0x00c00011a090 by goroutine 33: runtime.mapassign_fast32() /usr/local/Cellar/go/1.16.5/libexec/src/runtime/map_fast32.go:92 +0x0 github.com/pkg/sftp.(clientConn).putChannel() github.com/pkg/sftp@v1.13.4/conn.go:111 +0x11b github.com/pkg/sftp.(clientConn).dispatchRequest() github.com/pkg/sftp@v1.13.4/conn.go:152 +0x72 github.com/pkg/sftp.(*File).WriteTo.func2() github.com/pkg/sftp@v1.13.4/client.go:1293 +0x2f3

Goroutine 16 (running) created at: github.com/pkg/sftp.NewClientPipe() github.com/pkg/sftp@v1.13.4/client.go:237 +0x457 github.com/pkg/sftp.NewClient() github.com/pkg/sftp@v1.13.4/client.go:197 +0x25c main.main() sftp_test/main.go:77 +0x44b

Goroutine 33 (running) created at: github.com/pkg/sftp.(*File).WriteTo() github.com/pkg/sftp@v1.13.4/client.go:1273 +0x5ce main.downloadFile() main.go:192 +0x67b main.main() main.go:97 +0x804

[puellanivis]

Huh… weird.

[puellanivis]

Found it. In the article they have:

func downloadFile(sc sftp.Client, remoteFile, localFile string) (err error) {

You cannot use sftp.Client as a non-pointer. It must be used as a pointer. I’m curious why this isn’t being caught by a go vet… that should note that you’re copying a mutex, so that the two goroutines are locking different mutexes. 🤔

[criscojocaru]

Thank you sooooo much! That was indeed the problem. I didn't know about go vet.

[puellanivis]

Yeah, most people don’t know about go vet anymore, because go test runs it automagically. Unfortunately, it appears that go test only calls a subset of vet commands, which does not include copylocks the vet test that would have caught this problem. 😢