Skip to content

storage: nil pointer crash in writer goroutine after CloseWithError #4167

@dt

Description

@dt

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:

call := w.o.c.raw.Objects.Insert(w.o.bucket, rawObj).

Looking at Client.Close(), it nils out raw when the client is closed:

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:

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:

return w.pw.CloseWithError(err)

Metadata

Metadata

Assignees

Labels

api: storageIssues related to the Cloud Storage API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions