-
Notifications
You must be signed in to change notification settings - Fork 1.5k
storage: nil pointer crash in writer goroutine after CloseWithError #4167
Description
Client
Storage
Code
Extracted/adapted from a more complex reproduction, but more or less:
for {
c := storage.NewClient(...)
w := c.NewWriter(...)
w.CloseWithError(io.EOF)
c.Close(...)
}Expected behavior
The write is aborted.
Actual behavior
The process crashes:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x375c91a]
...
cloud.google.com/go/storage.(*Writer).open.func1(0xc03023bb00, 0xc00dea2380, 0xc0381d70e0, 0xc00cac1840, 0x1, 0x1)
/go/src/.../vendor/cloud.google.com/go/storage/writer.go:131 +0xfa fp=0xc0040f0fb0 sp=0xc0040f0e30 pc=0x375c91a
runtime.goexit()
/usr/local/go/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0040f0fb8 sp=0xc0040f0fb0 pc=0x4c4ae1
created by cloud.google.com/go/storage.(*Writer).open
/go/src/.../vendor/cloud.google.com/go/storage/writer.go:118 +0x412
Additional Information
It looks like Writer spawns this goroutine to handle the upload side of the pipe. Inside this goroutine it is reading fields of storage.Client such as Client.raw on the line mentioned in the above crash stack trace:
google-cloud-go/storage/writer.go
Line 131 in 825ddd6
| call := w.o.c.raw.Objects.Insert(w.o.bucket, rawObj). |
Looking at Client.Close(), it nils out raw when the client is closed:
google-cloud-go/storage/storage.go
Line 167 in 825ddd6
| c.raw = nil |
So it looks like the caller has called Client.Close() while while the writer goroutine was still running. However reviewing the calling the code in my application, the the Writer is always closed -- via either Close or a CloseWithError in a defer -- before the function that created it returns to the function which created and would close the Client.
So it appears that the writer was somehow still running after it was closed, which meant it was then poised to crash when the client was closed out from under it?
Looking at Writer.Close(), it blocks on the w.donec channel, which is closed by the spawned goroutine when it exits, thus ensuring that that goroutine has exited before Close returns:
google-cloud-go/storage/writer.go
Line 230 in 825ddd6
| <-w.donec |
However Writer.CloseWithError() does not block on that donec -- it simply calls CloseWithError() on the underlying io.Pipe and returns but it does not actually wait for the writer goroutine to notice the closed pipe and exit:
google-cloud-go/storage/writer.go
Line 261 in 825ddd6
| return w.pw.CloseWithError(err) |