-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compress/zlib: Writer appears to ignore underlying writer errors at times. #16749
Labels
Milestone
Comments
It's a regression from 53984e5. I'll send out a fix soon. This is probably go1.7.1 worthy. |
CL https://golang.org/cl/27200 mentions this issue. |
gopherbot
pushed a commit
that referenced
this issue
Sep 7, 2016
…ersistent For persistent error handling, the methods of huffmanBitWriter have to be consistent about how they check errors. It must either consistently check error *before* every operation OR immediately *after* every operation. Since most of the current logic uses the previous approach, we apply the same style of error checking to writeBits and all calls to Write such that they only operate if w.err is already nil going into them. The error handling approach is brittle and easily broken by future commits to the code. In the near future, we should switch the logic to use panic at the lowest levels and a recover at the edge of the public API to ensure that errors are always persistent. Fixes #16749 Change-Id: Ie1d83e4ed8842f6911a31e23311cd3cbf38abe8c Reviewed-on: https://go-review.googlesource.com/27200 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-on: https://go-review.googlesource.com/28634 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Please answer these questions before submitting your issue. Thanks!
go version
)?go version go1.7 darwin/amd64
go env
)?GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/simon.fell/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
It seems like there's a condition where the zlib Writer doesn't notice that the Writer its writing to returned an error, the test below fails for me on go 1.7 on OSX, passes on go 1.6 linux and go 1.6.3 on osx. The number of writes to zlib seems to be significant in reproducing the error.
The test has a writer that returns an error for the 4th write to the writer, however neither the call the zlib.Writer.Write or to zlib.Writer.Close surface the error.
When running the test, it'll fail with
--- FAIL: Test_WriteErrorCheck (0.00s)
compress_test.go:54: Write count=1, len=2
compress_test.go:54: Write count=2, len=240
compress_test.go:54: Write count=3, len=240
compress_test.go:54: Write count=4, len=240
compress_test.go:54: Write count=5, len=240
compress_test.go:54: Write count=6, len=240
compress_test.go:54: Write count=7, len=240
compress_test.go:54: Write count=8, len=240
compress_test.go:54: Write count=9, len=240
compress_test.go:54: Write count=10, len=240
compress_test.go:54: Write count=11, len=240
compress_test.go:54: Write count=12, len=240
compress_test.go:54: Write count=13, len=240
compress_test.go:54: Write count=14, len=240
compress_test.go:54: Write count=15, len=240
compress_test.go:54: Write count=16, len=240
compress_test.go:54: Write count=17, len=149
compress_test.go:54: Write count=18, len=4
compress_test.go:54: Write count=19, len=4
compress_test.go:38: Underlying writer returned error, that wasn't surfaced to zlib caller [total num writes=2000]
compress_test.go:54: Write count=1, len=2
compress_test.go:54: Write count=2, len=240
compress_test.go:54: Write count=3, len=240
compress_test.go:54: Write count=4, len=240
compress_test.go:54: Write count=5, len=240
compress_test.go:54: Write count=6, len=240
compress_test.go:54: Write count=7, len=240
compress_test.go:54: Write count=8, len=134
compress_test.go:54: Write count=9, len=4
compress_test.go:54: Write count=10, len=4
compress_test.go:38: Underlying writer returned error, that wasn't surfaced to zlib caller [total num writes=2000]
FAIL
FAIL _/Users/simon.fell/code/zlibtest 0.008s
I thought this may of been related to the compression algo for zlib.BestSpeed, but also seem same issue with DefaultCompression as well.
Expect to see the test pass [as it does on go 1.6]
test fails, no error is surfaces from the zlib Writer.
package main
import (
"compress/zlib"
crand "crypto/rand"
"errors"
"testing"
)
var errWriteFailed = errors.New("I/O Error")
func Test_WriteErrorCheck(t *testing.T) {
src := make([]byte, 2000)
crand.Read(src)
writes := make([][]byte, 2000)
for i := range writes {
writes[i] = src[i/2 : i/2+10]
}
checkWriteErrorSurfaced(t, zlib.BestSpeed, writes)
checkWriteErrorSurfaced(t, zlib.DefaultCompression, writes)
}
func checkWriteErrorSurfaced(t *testing.T, compLevel int, writes [][]byte) bool {
w := &errorWriter{t: t}
c, err := zlib.NewWriterLevel(w, compLevel)
if err != nil {
t.Fatalf("Unable to construct zlib.Writer: %v", err)
}
var errw error
for _, w := range writes {
_, errw = c.Write(w)
if errw != nil {
break
}
}
errc := c.Close()
if errc == nil && errw == nil {
t.Errorf("Underlying writer returned error, that wasn't surfaced to zlib caller [total num writes=%d]", len(writes))
return false
}
t.Logf("%v", errw)
t.Logf("%v", errc)
return true
}
// errorWriter is an io.Writer that returns an error for the 4th write
type errorWriter struct {
t *testing.T
writeCount int
}
func (w *errorWriter) Write(d []byte) (int, error) {
w.writeCount++
w.t.Logf("Write count=%d, len=%d", w.writeCount, len(d))
if w.writeCount == 4 {
return 0, errWriteFailed
}
return len(d), nil
}
The text was updated successfully, but these errors were encountered: