go-mssqldb: Reset/handle/mark bad connections

Is your feature request related to a problem? Please describe. I’m hitting following panic:

panic: driver.ErrBadConn in checkBadConn. This should not happen.
goroutine 117 [running]:
github.com/denisenkom/go-mssqldb.(*Conn).checkBadConn(0xc00005b2c0, 0xe90100, 0xc0000282e0, 0xe90100, 0xc0000282e0)
    /go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20190806190131-db2462fef53b/mssql.go:184 +0x1ca
github.com/denisenkom/go-mssqldb.(*Conn).begin(0xc00005b2c0, 0xea1900, 0xc0003107e0, 0x409900, 0xc13cc0, 0xcbeb80, 0x536401, 0x7f00bc513260)
    /go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20190806190131-db2462fef53b/mssql.go:288 +0xbc
github.com/denisenkom/go-mssqldb.(*Conn).BeginTx(0xc00005b2c0, 0xea1900, 0xc0003107e0, 0x0, 0xc00005b200, 0x1, 0x12, 0x8bb2c97000, 0x0)
    /go/pkg/mod/github.com/denisenkom/go-mssqldb@v0.0.0-20190806190131-db2462fef53b/mssql.go:946 +0x7e
database/sql.ctxDriverBegin(0xea1900, 0xc0003107e0, 0x0, 0xe9d600, 0xc00005b2c0, 0xc0003f13d0, 0x42ab2f, 0xc000000008, 0xc00005b7c0)
    /usr/local/go/src/database/sql/ctxutil.go:106 +0x9e
database/sql.(*DB).beginDC.func1()
    /usr/local/go/src/database/sql/sql.go:1653 +0x6b
database/sql.withLock(0xe97480, 0xc000634c00, 0xc0003f1450)
    /usr/local/go/src/database/sql/sql.go:3097 +0x63
database/sql.(*DB).beginDC(0xc0003fc0c0, 0xea1900, 0xc0003107e0, 0xc000634c00, 0xc000662060, 0x0, 0x0, 0x0, 0x0)
    /usr/local/go/src/database/sql/sql.go:1652 +0xac
database/sql.(*DB).begin(0xc0003fc0c0, 0xea1900, 0xc0003107e0, 0x0, 0x7f00bc7abf01, 0x1000, 0x20300000000000, 0x8000)
    /usr/local/go/src/database/sql/sql.go:1646 +0xd3
database/sql.(*DB).BeginTx(0xc0003fc0c0, 0xea1900, 0xc0003107e0, 0x0, 0x8000000000000107, 0x1, 0x0)
    /usr/local/go/src/database/sql/sql.go:1624 +0x89
(...cut...)

Describe the solution you’d like https://github.com/denisenkom/go-mssqldb/blob/44cdfe8d8ba9a7322b38cd8fbb0f2cb445e65a69/mssql.go#L184

By following comments on the code above I discovered that it might be somehow fixed by using ResetSessioner, to handle bad connections? Simple search on code base didn’t show up any related phrases so I’m assuming this is not in use currently?

About this issue

Original URL
State: open
Created 5 years ago
Comments: 15 (3 by maintainers)

Commits related to this issue

Issue #536 - prevent errors returned from mssql.Conn#processBeginResponse(context.Context) from being checked twice (#583) Co-authored-by: Severinas Monkevicius <severinas_monkevicius@trimble.com> — committed to denisenkom/go-mssqldb by sevasm 4 years ago
Issue #536 - prevent errors returned from mssql.Conn#processBeginResponse(context.Context) from being checked twice (#583) Co-authored-by: Severinas Monkevicius <severinas_monkevicius@trimble.com> — committed to xhit/go-mssqldb by sevasm 4 years ago
Prevent errors returned from Stmt.processExec from being checked twice (#536) — committed to nwwdles/go-mssqldb by nwwdles 3 years ago
Prevent errors returned from Stmt.processExec from being checked twice (#536) (#662) — committed to denisenkom/go-mssqldb by nwwdles 3 years ago

Most upvoted comments

Hi all, after hitting this same issue and some investigation I’ve found the following:

The issue occurs when sql.DB#BeginTx(context.Context, *sql.TxOptions) is called after SQL server connection is closed by, for example, shutting down SQL server by stopping its Docker container. See sevasm/go-mssqldb-panic for an example program that can reproduce the issue reliably.

The issue cannot be reproduced by closing the underlying transport of the connection as is done in a number of tests, e.g. TestCommitTranError (conn.sess.buf.transport.Close()).

After closing the underlying transport and calling sql.DB#BeginTx(context.Context, *sql.TxOptions), #sendBeginXact(*tdsBuffer, []headerStruct, isoLevel, string, bool) is eventually called, and due to the closed transport returns an error.

However, when the connection is closed by stopping the SQLServer container, this method instead returns nil, indicating no error. Instead, an error is later received by mssql.Conn#simpleProcessResp(context.Context) on line 214. There, checkBadConn is called on the error and the “checked” error is returned. Eventually it propagates up to mssql.Conn#begin(context.Context, isoLevel) and is checked for a second time on line 288, causing the panic as it’s already “checked” and is already a driver.ErrBadConn as indicated by the error message.

Because of how mssql.Conn#simpleProcessResp(context.Context) and mssql.Conn#processBeginResponse(context.Context) are implemented, I believe mssql.Conn#begin(context.Context, isoLevel) can only ever receive errors that are already “checked”, therefore I have locally fixed the error by replacing line 288 of mssql.go with the following:

return nil, err

instead of

return nil, c.checkBadConn(err)

I can confirm that the panic no longer occurs after making this change, and all tests in the repository pass.

sevasm on Jun 18, 2020

@jozuenoon sorry for the delay, we have implemented SessionResetter, which is supported in go 1.10 and above. They decided to change the name which is why you didn’t find it. I’m still looking into how the panic is being hit but I hope this helps.

chris-rossi on Nov 19, 2019

Hi @jozuenoon, I’ll look further into this.

chris-rossi on Nov 8, 2019