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
database/sql: segfault in connection pool when the driver frequently fails to connect #24445
Comments
Thanks for the report. I'll look into it. |
Change https://golang.org/cl/102477 mentions this issue: |
I'm having trouble reproducing, but here is what I have confirmed:
I can confirm so far when I insert a call to |
@andybons the cl needs a +2, but the linked cl would be a good candidate for a 1.10.1. I'm not sure it it a regression from 1.9. |
Thanks for the fix. I've been running my program with the The panic happened two times on different computers, and it took 3~4 days of continuously failing connection attempts. |
/cc @bradfitz |
@gopherbot please backport to 1.10 with CL https://golang.org/cl/102477 |
Backport issue(s) opened: #25235 (for 1.10). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
@kardianos once the CL lands, feel free to cherry-pick the change to release-branch.go1.10 per the instructions above. The release team will then determine whether it makes it in. Thanks! |
Change https://golang.org/cl/146778 mentions this issue: |
…before use The connRequest may return a nil conn value. However in a rare case that is difficult to test for it was being passed to DB.putConn without a nil check. This was an error as this made no sense if the driverConn is nil. This also caused a panic in putConn. A test for this would be nice, but didn't find a sane way to test for this condition. Updates #24445 Fixes #25235 Change-Id: I827316e856788a5a3ced913f129bb5869b7bcf68 Reviewed-on: https://go-review.googlesource.com/102477 Run-TryBot: Daniel Theophanes <kardianos@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> (cherry picked from commit b98ffdf) Reviewed-on: https://go-review.googlesource.com/c/146778 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Katie Hockman <katie@golang.org>
What version of Go are you using (
go version
)?go version go1.10 linux/amd64
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?What did you do?
All line numbers are from go1.10 release (https://github.com/golang/go/blob/bf86aec25972f3a100c3aa58a6abcbcc35bdea49/src/database/sql/sql.go)
Goroutine 1:
DB.conn() is called. Passes the first context check at line 1021~1026.
go/src/database/sql/sql.go
Line 1026 in bf86aec
Goroutine 2:
Cancel the context that was passed to DB.conn() in Goroutine 1.
Goroutine 1:
Block on db.mu.Lock() on sql.go:1067
go/src/database/sql/sql.go
Line 1067 in bf86aec
Goroutine 3:
DB.openNewConnection() calls db.putConnDBLocked(nil, err).
The comment for DB.putConnDBLocked() says
If err != nil, the value of dc is ignored
, so this is valid.Goroutine 1:
In sql.go:1072 it receives connRequest{conn: nil, err: err} sent by Goroutine 3.
Calls db.putConn(nil, err).
db.putConn accesses
dc.inUse
where dc is nil, and panics.The comment for DB.putConn() says
err is optionally the last error that occurred on this connection
,so there should be a check before calling db.putConn(), but there aren't.
What did you expect to see?
It should return an error, ret.err or maybe ctx.Err().
What did you see instead?
The text was updated successfully, but these errors were encountered: