-
Notifications
You must be signed in to change notification settings - Fork 18.9k
Description
Description
There is a race condition in pkg/archive when using cmd.Start for pigz and xz. The *bufio.Reader could be returned to the pool while the command is still writing to it. The command is wrapped in a CommandContext where the process will be killed when the context is cancelled, however this is not instantaneous, so there's a brief window while the command could still be running but the *bufio.Reader was already returned to the pool.
wrapReadCloser calls cancel(), and then readBuf.Close() which eventually returns the buffer to the pool:
Lines 179 to 180 in 1d19062
| cancel() | |
| return readBuf.Close() |
However, because cmdStream runs cmd.Wait in a go routine that we never wait for to finish, it is not safe to return the reader to the pool yet. We need to ensure we wait for cmd.Wait to finish!
Steps to reproduce the issue:
I have written a reproducer at https://github.com/stbenjam/docker-race-reproducer, where you can see the behavior. Check out the repo and run go run main.go.
Describe the results you received:
In the worst case, a panic:
Waiting...
Done
Done
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x4b20b0]
goroutine 34 [running]:
bufio.(*Reader).fill(0xc0000fe300)
/usr/lib/golang/src/bufio/bufio.go:100 +0xe0
bufio.(*Reader).WriteTo(0xc0000fe300, 0x554bc0, 0xc000118068, 0x7fb786381fb0, 0xc0000fe300, 0x4eb001)
/usr/lib/golang/src/bufio/bufio.go:486 +0x259
io.copyBuffer(0x554bc0, 0xc000118068, 0x554a40, 0xc0000fe300, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc00010c060)
/usr/lib/golang/src/io/io.go:384 +0x352
io.Copy(0x554bc0, 0xc000118068, 0x554a40, 0xc0000fe300, 0x0, 0xc00003c7b8, 0x4ebb89)
/usr/lib/golang/src/io/io.go:364 +0x5a
os/exec.(*Cmd).stdin.func1(0x0, 0x0)
/usr/lib/golang/src/os/exec/exec.go:234 +0x55
os/exec.(*Cmd).Start.func1(0xc000110160, 0xc00011e120)
/usr/lib/golang/src/os/exec/exec.go:400 +0x27
created by os/exec.(*Cmd).Start
/usr/lib/golang/src/os/exec/exec.go:399 +0x5af
exit status 2
Describe the results you expected:
No race condition.
Here's the output of go run -race:
=================
WARNING: DATA RACE
Read at 0x00c000076088 by goroutine 16:
bufio.(*Reader).writeBuf()
/usr/lib/golang/src/bufio/bufio.go:510 +0x50
bufio.(*Reader).WriteTo()
/usr/lib/golang/src/bufio/bufio.go:468 +0x6a
io.copyBuffer()
/usr/lib/golang/src/io/io.go:384 +0x46a
io.Copy()
/usr/lib/golang/src/io/io.go:364 +0x74
os/exec.(*Cmd).stdin.func1()
/usr/lib/golang/src/os/exec/exec.go:234 +0x8a
os/exec.(*Cmd).Start.func1()
/usr/lib/golang/src/os/exec/exec.go:400 +0x34
Previous write at 0x00c000076088 by goroutine 6:
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/pools.(*BufioReaderPool).Put()
/usr/lib/golang/src/bufio/bufio.go:75 +0xaf
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/pools.(*BufioReaderPool).NewReadCloserWrapper.func1()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/pools/pools.go:93 +0x98
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/ioutils.(*ReadCloserWrapper).Close()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/ioutils/readers.go:20 +0x4c
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive.wrapReadCloser.func1()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive/archive.go:180 +0x67
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/ioutils.(*ReadCloserWrapper).Close()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/ioutils/readers.go:20 +0x4c
main.decompress()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/main.go:33 +0xdb
Goroutine 16 (running) created at:
os/exec.(*Cmd).Start()
/usr/lib/golang/src/os/exec/exec.go:399 +0x9bf
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive.cmdStream()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive/archive.go:1217 +0x33b
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive.gzDecompress()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive/archive.go:174 +0x17a
github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive.DecompressStream()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/vendor/github.com/docker/docker/pkg/archive/archive.go:207 +0x572
main.decompress()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/main.go:29 +0x46
Goroutine 6 (finished) created at:
main.main()
/home/stbenjam/go/src/github.com/stbenjam/docker-race-reproducer/main.go:21 +0xb6
==================