panic on bodyclose #608
Comments
|
I'm hitting this same issue with |
Merged
|
I created a patch but I'm not sure if it covers your case, could you try to isolate the code that produces the panic? |
|
No, sorry, I've seen it just once, no idea how to reproduce. |
|
You have to find the "body-close" pattern in your code base and try to change your code to find the lines that produce the panic when they are analyzed by the linter. |
Closed
|
I'm getting this same nil pointer dereference. My stack trace is below. I've also noted that #622 is a duplicate of this, and I've added a comment there to try to point everyone here. OSX version 10.14.5 /usr/local/bin/golangci-lint run -c .golangci.yml ./...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x1598484]
goroutine 1835 [running]:
go/types.(*object).Name(...)
/home/travis/.gimme/versions/go1.12.5.linux.amd64/src/go/types/object.go:134
github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose.(*runner).isCloseCall(0xc00009e2c0, 0x1be0ea0, 0xc00798b700, 0xc0068132c0)
/home/travis/gopath/src/github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose/bodyclose.go:230 +0x234
github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose.(*runner).isopen(0xc00009e2c0, 0xc014b1f760, 0x9, 0xc01169dd00)
/home/travis/gopath/src/github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose/bodyclose.go:175 +0x2e9
github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose.(*runner).run(0xc00009e2c0, 0xc0000cbd60, 0x10, 0x191fec0, 0x4af2d33feb6c8001, 0xc012290720)
/home/travis/gopath/src/github.com/golangci/golangci-lint/vendor/github.com/timakin/bodyclose/passes/bodyclose/bodyclose.go:100 +0x641
github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker.(*action).execOnce(0xc00041d720)
/home/travis/gopath/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker/checker.go:382 +0x68a
sync.(*Once).Do(0xc00041d720, 0xc000a83790)
/home/travis/.gimme/versions/go1.12.5.linux.amd64/src/sync/once.go:44 +0xb3
github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker.(*action).exec(0xc00041d720)
/home/travis/gopath/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker/checker.go:303 +0x50
github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker.execAll.func1(0xc00041d720)
/home/travis/gopath/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker/checker.go:291 +0x34
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker.execAll
/home/travis/gopath/src/github.com/golangci/golangci-lint/pkg/golinters/goanalysis/checker/checker.go:297 +0x11b
Process finished with exit code 2 |
|
I've found the cause of the problem. It crashed because I hadn't closed a response body from the server, which I had copied out as an DetailsI had first read the body using the following function (taken directly from the go standard library): // ReadBody safely reads the HTTP request or response body, returning a byte
// slice of the body contents, a copy of the body (to add back into the body),
// and an error, if any.
func ReadBody(body io.ReadCloser) (contents []byte, copy io.ReadCloser, err error) {
if body == http.NoBody {
// No copying needed. Preserve the magic sentinel meaning of NoBody.
return nil, http.NoBody, nil
}
var buf bytes.Buffer
if _, err = buf.ReadFrom(body); err != nil {
return nil, body, err
}
if err = body.Close(); err != nil {
return nil, body, err
}
return buf.Bytes(), ioutil.NopCloser(bytes.NewReader(buf.Bytes())), nil
}I then used it like so, which turns the body into a var respBody []byte
respBody, resp.Body, err = network.ReadBody(resp.Body)So, the problem might be that it hits a |
|
Possibly fixed by timakin/bodyclose#17 I was able to reproduce the issue with this code package main
import (
"io"
"io/ioutil"
"log"
"net/http"
)
var c http.Client
func main() {
req, _ := http.NewRequest(http.MethodGet, "/", nil)
resp, _ := c.Do(req)
readAndClose(resp.Body)
}
// readAndClose read the body to EOF and closes it
func readAndClose(b io.ReadCloser) {
if b == nil {
return
}
if _, err := io.Copy(ioutil.Discard, b); err != nil {
log.Printf("read response body: %v", err)
}
if err := b.Close(); err != nil {
log.Printf("close response body: %v", err)
}
}And then running System infogo version go1.12.6 linux/amd64 |
|
Can still repro this on d2b1eea revision (latest) ): on: |
|
We are getting sporadic failures as well. I think you just need to pull timakin/bodyclose#17 into your forked version. |
|
Any chance of cutting a new release soon? We're hitting this fairly frequently. |
|
This is also blocking us updating, which is a problem as we've moved to 1.13 so are hitting the go mod issues. |
|
@bwplotka It's another bug in bodyclose - timakin/bodyclose#20. |
jirfag
added a commit
that referenced
this issue
Sep 10, 2019
go/analysis panics were propagated to main and crashed golangci-lint. Just log them, as with other linters. Found in #608.
jirfag
added a commit
that referenced
this issue
Sep 23, 2019
Until timakin/bodyclose#20 merged use forked version.
|
Unfortunately this doesn't fix all the issues, with 1.19.0 we still get a panic: |
|
Raised a new issue to track this here: #733 |
korjavin
added a commit
to korjavin/golangci-lint-1
that referenced
this issue
Sep 28, 2019
* tag 'v1.19.1': Use separate go.mod/go.sum to manage tool deps. (golangci#736) Use stretchr/testify to mock log. Fix golangci#733: update forked bodyclose Cleanup obsolete go.mod/go.sum diff. (golangci#729) dev: update deprecated parts of .goreleaser.yml dev: trigger CI build on tag push Update minimum Go version to 1.12. Enable consistent GOPROXY Go 1.12/1.13 behavior. Fix golangci#608: use forked bodyclose Drop memory usage of go/analysis linters 5x Manage build tools via go.mod. Update whitespace Reduce memory usage of go/analysis
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This happens just once on CI and wasn't reproduce on rebuilding same commit on CI.
The text was updated successfully, but these errors were encountered: